Be less strict in the documentation for using PCRE2_SUBSTITUTE_MATCHED

[IsaacOscar]

I was reading the documentation of pcre2_substitute in pcre2api

The relevant parts being (on both the website and my systems man pages, which is v10.45)

One such option is PCRE2_SUBSTITUTE_MATCHED. When this is set, an external match_data block must be provided, and it must have already been used for an external call to pcre2_match() with the same pattern and subject arguments.

The contents of the externally supplied match data block are not changed when PCRE2_SUBSTITUTE_MATCHED is set.

This is a bit limiting for several reasons:

1. It requires that the subject arguments (I assume the PCRE2_SPTR subject, PCRE2_SIZE length, and PCRE2_SIZE startoffset) be the same as the preceding call to match. I am suggesting that this be relaxed, to only required that all the ranges in the ovector are within the bounds of the subject; I don't think there needs to be any additional requirement on startoffset (other than it being within the bounds of subject of course).

2. It isn't clear whether you can call pcre_substitute with PCRE2_SUBSTITUTE_MATCHED multiple times with the same match data. The second paragraph quoted above suggests that this is ok. I suggest clarifying that this is ok.

3. Similarly, its not clear if a call to pcre2_substitute without PCRE2_SUBSTITUTE_MATCHED counts as a call to pcre2_match. I also suggest allowing this.

I looked at the source code for my version of PCRE2 (the latest stable, 10.45). It seems that the above suggested changes are entirely consistent with the current implementation. I.e. I am asking only that the documentation to be updated so that one can rely on this behaviour in future releases.

My primary motivation for the above is a bit strange: I'm trying to make safe Rust wrapper over pcre2_substitute:

1. I want to be able to check the result of a match before deciding to do a substitute (i.e. something like pcre2_match(....); if some_condition(match_data) { pcre2_substitute(...) }).
2. I don't want to pay the performance cost of having to re-do the pcre2_match everytime you want to do a substitute
3. I don't want to pay the performance cost of unconditionally doing a pcre2_substitute, and then delete the substitution result (which will need to be dynamically allocated) if I decide not to do the substitution.
4. I want to minimise unsafe code: I don't want higher level code to get into undefined behaviour because it called pcre2_substitute with a different subject string.
5. I don't want to have to dynamically allocate a copy of the subject string and keep it around to prevent the above undefined behaviour.
6. I want to be able able to reuse match data blocks for multiple calls to pcre2_match and pcre2_substitute, thus saving memory and allocation time. I can use the Rust type system here to ensure that the match data is not used between the call to pcre2_match and pcre2_substitute.
7. Trying to store a reference to the subject string so I can safely ensure the same one is passed to pcre2_substitute is a nightmare to do in generic Rust code as it needs to keep track of lifetimes (whereas the case with match data above is easy, as it doesn't "borrow" anything)..
8. Modifying the code of pcre2_substitute so it uses the subject stored in the match data, won't help me as getting Rusts's type system to ensure that the subject hasn't been freed or mutated between the two calls is particularly difficult.

So basically, if you do change #1 above, I can just store the length of the subject string with the match data, and require the user to pass the subject string when doing a substitute. I then just throw an error if the new subject is shorter than the original one. (I can similarly easily store the start offset).

Other use cases of course are:

1. Doing some modification on the subject string between the calls to pcre2_match and pcre2_substitute, e.g. uppercasing it.
2. Generating different replacement strings on the same subject (this requires points 1. and 2. above), e.g. you could use substitute once to title-case a string, and another to lowercase it.

I've also attached a simple test program that demonstrates that suggestions 1. and 2. currently work as expected (I also ran it through valgrind and there were no errors, despite me having deleted the origin subject string I used when calling pcre2_match).
Bassically, my program does:

1. A search with regex (l*)o(.*)l on the string hello world, starting at position 3.
2. A substitution on the same string and starting offset with replacement\n\t$_\n\t$|$&|$'\n\t($1)\n\t($2)\n`.
3. I print the result of the above, yielding:

hel
        hello world
        hel|lo worl|d
        (l)
        ( wor)
d

I.e. the first line is the text before the match (which is not replaced), the second line is the entire input string, the third is the text preceding the match, a |, the match, a |, and the text following it. The next two lines are the two captured groups in (...), and the final line is the text after the match (which is not replaced).
4. Free the original subject string, and delete the pointer to it in the match data (so I can be sure pcre2_substitute wont access it in the next step).
5. Do step #2 above with the same replacement string, but subject totally different string and start offset 1.
6. Print the result, which yields:

tot
        totally different string
        tot|ally di|fferent string
        (a)
        (ly d)
fferent string

Note that if the start offset given to pcre2_substitute is larger than the start of the first match recorded by pcre2_match, pcre2_substitute returns PCRE2_ERROR_BADSUBSPATTERN (i.e. match with end before start or start moved backwards is not supported). However, using any other value for the start offset doesn't effect the result (so the error seems pointless: the start offset is only relevant for when pcre2_substitute internally calls pcre2_match, but it doesn't do that when PCRE2_SUBSTITUTE_MATCHED is set, and PCRE2_SUBSTITUTE_GLOBAL is not).

[NWilson]

\1. It requires that the subject arguments (I assume the PCRE2_SPTR subject, PCRE2_SIZE length, and PCRE2_SIZE startoffset) be the same as the preceding call to match. I am suggesting that this be relaxed, to only required that all the ranges in the ovector are within the bounds of the subject; I don't think there needs to be any additional requirement on startoffset (other than it being within the bounds of subject of course).

I think you are correct. I will try and check that this is the case, and relax the documentation to match, if there are use cases where someone would want to match one subject string, and then reuse the match block to substitute in another subject.

\2. It isn't clear whether you can call pcre_substitute with PCRE2_SUBSTITUTE_MATCHED multiple times with the same match data. The second paragraph quoted above suggests that this is ok. I suggest clarifying that this is ok.

It is OK. The fact that the match block is read but not written implies that it can be reused.

\3. Similarly, its not clear if a call to pcre2_substitute without PCRE2_SUBSTITUTE_MATCHED counts as a call to pcre2_match. I also suggest allowing this.

Is it allowed. pcre2_substitute() can be used in one of three ways:

• With PCRE2_SUBSTITUTE_MATCHED: no internal call to pcre2_match
• With PCRE2_SUBSTITUTE_GLOBAL: a loop that internally calls pcre2_match to find all matches
• Otherwise, performs a single internal call to pcre2_match

My primary motivation for the above is a bit strange: I'm trying to make safe Rust wrapper over pcre2_substitute:

I think I see. You want your Rust wrapper to be at the same abstraction level as PCRE2 (ie function-for-function), but you want to make the API clean from undefined behaviours where possible. In general, that's going to be hard, because PCRE2 has quite a few places (sadly) where it will blindly trust the input you provided matches the documented restrictions.

In this case... your solution isn't the only one, I think. You're assuming that the behaviour of PCRE2 won't change, but that you can make your wrapper work (and be compliant to the documentation) if we relax the documented requirements.

Even better though, in my opinion, would be to simply change the behaviour of pcre2_substitute(), so that it checks and returns a sensible error if mismatched subject pointers are passed in to pcre2_match and pcre2_substitute.

This would benefit all users, not just those using the Rust wrapper.

I am very keen to gradually improve PCRE2 safety, where instances of missing or loose argument validation are discovered.

\1. Doing some modification on the subject string between the calls to pcre2_match and pcre2_substitute, e.g. uppercasing it.

In general, uppercasing will not preserve offsets. There are many upper/lowercase pairs in Unicode where the two codepoints don't have the same byte length in UTF-8. That's not even going into the complex cases where characters are split into multiple codepoints when case converting.

2. Generating different replacement strings on the same subject (this requires points 1. and 2. above), e.g. you could use substitute once to title-case a string, and another to lowercase it.

This is already valid.

Note that if the start offset given to pcre2_substitute is larger than the start of the first match recorded by pcre2_match, pcre2_substitute returns PCRE2_ERROR_BADSUBSPATTERN (i.e. match with end before start or start moved backwards is not supported). However, using any other value for the start offset doesn't effect the result (so the error seems pointless: the start offset is only relevant for when pcre2_substitute internally calls pcre2_match, but it doesn't do that when PCRE2_SUBSTITUTE_MATCHED is set, and PCRE2_SUBSTITUTE_GLOBAL is not).

I think you're right. It should be possible to ignore the initial passed-in value of start_offset when PCRE2_SUBSTITUTE_MATCHED is set. The code does actually use it, by writing the fragment from 0 to start_offset and then start_offset to ovector[0] in two separate copies, but that chunking is not visible to clients.

---

Would you be happy if I changed pcre2_substitute() so that it returns PCRE2_ERROR_BADSUBSSUBJECT (a new error) when you use PCRE2_SUBSTITUTE_MATCHED but don't pass in an exactly-matching subject, start_offset, and subject length? Is that too strict for you? I can also check that the subject length hasn't changed (to pick up the case where, totally improbably, the string was deallocated, and a new one allocated at the same address).

We obviously can't check whether the contents have changed. That's up to the caller.

---

Finally, note that just having a valid length doesn't mean the ovector is compatible with a subject.

Rust programs probably want to ensure that strings are always valid UTF-8. Two strings of length 5 bytes could be a 2-byte then a 3-byte character, or a 3-byte then a 2-byte character. If you have offset ovector[0] = 2 then it's saying, "at offset 2 in the subject there is a character". If it's slicing a character (rather than pointing to a character start) then pcre2_substitute won't produce valid UTF-8 output, because it trusts the input ovector.

So just a little point if that's a concern to you. Checking everything is within bounds isn't enough to ensure the output is valid UTF string.

[IsaacOscar]

Thanks so much for the reply!

It is OK. The fact that the match block is read but not written implies that it can be reused.
Is it allowed. pcre2_substitute() can be used in one of three ways:

• With PCRE2_SUBSTITUTE_MATCHED: no internal call to pcre2_match
• With PCRE2_SUBSTITUTE_GLOBAL: a loop that internally calls pcre2_match to find all matches
• Otherwise, performs a single internal call to pcre2_match

Thanks for the clarification, the documentation should be more clear. To quote the documentation again (emphasis added):

... an external match_data block must be provided, and it must have already been used for an external call to pcre2_match() ...

To me, a prior call to, pcre2_subsitute does not satisfy that. (As pcre2_match() is called internally).

My primary motivation for the above is a bit strange: I'm trying to make safe Rust wrapper over pcre2_substitute:

I think I see. You want your Rust wrapper to be at the same abstraction level as PCRE2 (ie function-for-function), but you want to make the API clean from undefined behaviours where possible. In general, that's going to be hard, because PCRE2 has quite a few places (sadly) where it will blindly trust the input you provided matches the documented restrictions.

Well the Rust code isn't exposing all of PCRE2's features (e.g. it doesn't let you set arbitrary options, only options that appear to be safe).

In this case... your solution isn't the only one, I think. You're assuming that the behaviour of PCRE2 won't change, but that you can make your wrapper work (and be compliant to the documentation) if we relax the documented requirements.

Even better though, in my opinion, would be to simply change the behaviour of pcre2_substitute(), so that it checks and returns a sensible error if mismatched subject pointers are passed in to pcre2_match and pcre2_substitute.

This would benefit all users, not just those using the Rust wrapper.

I am very keen to gradually improve PCRE2 safety, where instances of missing or loose argument validation are discovered.
Of course getting a sensible error would be better, I was just trying to minimise the amount of changes I was asking for.

\1. Doing some modification on the subject string between the calls to pcre2_match and pcre2_substitute, e.g. uppercasing it.

In general, uppercasing will not preserve offsets. There are many upper/lowercase pairs in Unicode where the two codepoints don't have the same byte length in UTF-8. That's not even going into the complex cases where characters are split into multiple codepoints when case converting.
Naturally Unicode is never simple. It would still be useful if the user of the API new what kind of input they were providing. As long as its not unsafe when the characters change width.

2. Generating different replacement strings on the same subject (this requires points 1. and 2. above), e.g. you could use substitute once to title-case a string, and another to lowercase it.

This is already valid.
I wasn't sure about that, I just wanted to double check.

Note that if the start offset given to pcre2_substitute is larger than the start of the first match recorded by pcre2_match, pcre2_substitute returns PCRE2_ERROR_BADSUBSPATTERN (i.e. match with end before start or start moved backwards is not supported). However, using any other value for the start offset doesn't effect the result (so the error seems pointless: the start offset is only relevant for when pcre2_substitute internally calls pcre2_match, but it doesn't do that when PCRE2_SUBSTITUTE_MATCHED is set, and PCRE2_SUBSTITUTE_GLOBAL is not).

I think you're right. It should be possible to ignore the initial passed-in value of start_offset when PCRE2_SUBSTITUTE_MATCHED is set. The code does actually use it, by writing the fragment from 0 to start_offset and then start_offset to ovector[0] in two separate copies, but that chunking is not visible to clients.

Would you be happy if I changed pcre2_substitute() so that it returns PCRE2_ERROR_BADSUBSSUBJECT (a new error) when you use PCRE2_SUBSTITUTE_MATCHED but don't pass in an exactly-matching subject, start_offset, and subject length? Is that too strict for you? I can also check that the subject length hasn't changed (to pick up the case where, totally improbably, the string was deallocated, and a new one allocated at the same address).

We obviously can't check whether the contents have changed. That's up to the caller.
For my use case, that's fine. I am expecting uses of the API to always use the same subject, I just don't want any undefined behaviour if they don't. (My API already ensures that the length and start offsets are the same, it's just checking the contents of the subject would be a much bigger performance cost).

I'm not sure what's more useful though: allowing people to use pcre2_substitute in more ways, or allowing the pcre2_substitute implementation to change more. So I see several options:

1. Allow pcre2_substitute to be used with different subject/subject-length/start-offset, and only throw an error if it actually needs to know characters that are outside the bounds of the new subject string (e.g. if some capture groups are out bounds in the new string, but you aren't referencing those in the replacement string, this would be allow)
2. Same as above, except always give an error if the new subject's length is shorter.
3. You're suggestion: also give an error if the subject pointer is different, or the new length is longer.
4. Make pcre2_substitute's behaviour unspecified (but not undefined) if the subject/subject-length/start-offset is different from the call to pcre2_match (e.g. this could allow a hypothetical optimisation where pcre2_substitute reads a cached copy of the contents of small capture groups, but reads the contents of larger groups from the new subject).

Personally I think 2. makes the most sense. The additional restriction in 3. that the pointers be the same seems rather arbitrary though as there's no requirement that the contents are the same. (But I guess it does allow pcre2_match to save pointers to particular parts of the string, instead of just offsets).

For my use cases, I just need it to not be undefined behaviour if the subject's contents has changed (as I can easily check the pointer's value, length, and start offsets in Rust, with minimal performance overhead).

Finally, note that just having a valid length doesn't mean the ovector is compatible with a subject.

Rust programs probably want to ensure that strings are always valid UTF-8. Two strings of length 5 bytes could be a 2-byte then a 3-byte character, or a 3-byte then a 2-byte character. If you have offset ovector[0] = 2 then it's saying, "at offset 2 in the subject there is a character". If it's slicing a character (rather than pointing to a character start) then pcre2_substitute won't produce valid UTF-8 output, because it trusts the input ovector.

So just a little point if that's a concern to you. Checking everything is within bounds isn't enough to ensure the output is valid UTF string.

Oh woops, I thought compiling the pattern with PCRE2_UTF | PCRE2_NEVER_BACKSLASH_C would be enough to ensure UTF-8 output.
The Rust API is using byte strings though (not making any assumptions as to encodings), and duplicating the entire API to also work with Rust's UTF-8 string types seems unnecessary.
I will have to make a note in the documentation that the substitution might not preserve UTF-8 validity. (It would be nice if the documentation made it clear when PCRE2 can output invalid UTF-8, including bad ovector ranges).

Perhaps pcre2_substitute can take an additional flag, say PCRE2_SUBSTITUTE_MATCHED_UTF_CHECK to check that the bounds in the ovector are all on UTF-8 character boundaries. User's of the API can just do this themselves, but pcre2_substitute could choose to not check ovector ranges that aren't actually being substituted in (I often have regexes with capturing groups that I never actually reference in the replacement string, so this could be faster than making user's do the check).

Also, is there any other way to get invalid UTF-8/16/32 output from PCRE2, when using options | PCRE2_UTF | PCRE2_NEVER_BACKSLASH_C & ~(PCRE2_MATCH_INVALID_UTF | PCRE2_NO_UTF_CHECK)?

I'm happy to submit a pull request with documentation changes to clarify things more, I just want to make sure I get all the details right.

[NWilson]

Thanks for the clarification, the documentation should be more clear. To quote the documentation again (emphasis added):

... an external match_data block must be provided, and it must have already been used for an external call to pcre2_match() ...

To me, a prior call to, pcre2_subsitute does not satisfy that. (As pcre2_match() is called internally).

I see now what you're saying. I'm sorry, I'd misunderstood. You want to know if calling pcre2_substitute without PCRE2_SUBSTITUTE_MATCHED but with a passed-in match block will populate the match block so that it's suitable for use in another call to pcre2_substitute with PCRE2_SUBSTITUTE_MATCHED.

I guess that would work...? But it's an odd requirement (to me). The code would be just as performant, and clearer, if the caller simply did the match, then called pcre2_substitute twice with PCRE2_SUBSTITUTE_MATCHED.

Naturally Unicode is never simple. It would still be useful if the user of the API new what kind of input they were providing. As long as its not unsafe when the characters change width.

...

For my use case, that's fine. I am expecting uses of the API to always use the same subject, I just don't want any undefined behaviour if they don't. (My API already ensures that the length and start offsets are the same, it's just checking the contents of the subject would be a much bigger performance cost).

It depends what you mean by "unsafe". If you use the match options PCRE2_UTF | PCRE2_NEVER_BACKSLASH_C, then you will get a match error if either of the pattern or subject strings is not valid UTF. And, the ovector offsets are guaranteed not to slice codepoints in half.

If you then pass in a valid UTF-8 replacement string to pcre2_substitute, then we also guarantee that the output of the substitution is valid UTF-8. Everything is safe.

However, if you mutate the subject string in between the calls to match and substitute, in such a way that the ovector's offsets point to bytes which are not the start of UTF-8 characters, then output of pcre2_substitute will not be valid UTF-8. All the behaviour is defined, but you won't get a "valid string" as the substitute output, even if all the input strings are valid UTF-8 (because you used subject strings which were valid UTF-8, but with mismatched contents in between the calls to match and substitute).

Is this unsafe for you? Well, if your string type asserts that the contents are valid UTF-8, then you may want to find a way to "pin" the string in between calls to match & substitute.

1. Allow pcre2_substitute to be used with different subject/subject-length/start-offset, and only throw an error if it actually needs to know characters that are outside the bounds of the new subject string (e.g. if some capture groups are out bounds in the new string, but you aren't referencing those in the replacement string, this would be allow)
2. Same as above, except always give an error if the new subject's length is shorter.
3. You're suggestion: also give an error if the subject pointer is different, or the new length is longer.
4. Make pcre2_substitute's behaviour unspecified (but not undefined) if the subject/subject-length/start-offset is different from the call to pcre2_match (e.g. this could allow a hypothetical optimisation where pcre2_substitute reads a cached copy of the contents of small capture groups, but reads the contents of larger groups from the new subject).

Personally I think 2. makes the most sense. The additional restriction in 3. that the pointers be the same seems rather arbitrary though as there's no requirement that the contents are the same. (But I guess it does allow pcre2_match to save pointers to particular parts of the string, instead of just offsets).

For my use cases, I just need it to not be undefined behaviour if the subject's contents has changed (as I can easily check the pointer's value, length, and start offsets in Rust, with minimal performance overhead).

I agree, asserting the subject pointer is the same seems a little strict. However, it matches how the API is intended to be used. For UTF-8, mutating a string in general does not preserve the byte-offsets of characters (eg changing an a to à alters the length). So, the cases where you can actually have a different subject string are very limited.

I feel like being strict here is helpful to users: we make it sufficiently clear that we stop users from shooting themselves in the foot, for example, assuming that they can just uppercase an arbitrary string in between match & substitute.

Oh woops, I thought compiling the pattern with PCRE2_UTF | PCRE2_NEVER_BACKSLASH_C would be enough to ensure UTF-8 output. The Rust API is using byte strings though (not making any assumptions as to encodings), and duplicating the entire API to also work with Rust's UTF-8 string types seems unnecessary. I will have to make a note in the documentation that the substitution might not preserve UTF-8 validity. (It would be nice if the documentation made it clear when PCRE2 can output invalid UTF-8, including bad ovector ranges).

That's fair. As long as you provide valid input (with valid ovector ranges), you shouldn't be able to get invalid output.

Perhaps pcre2_substitute can take an additional flag, say PCRE2_SUBSTITUTE_MATCHED_UTF_CHECK to check that the bounds in the ovector are all on UTF-8 character boundaries. User's of the API can just do this themselves, but pcre2_substitute could choose to not check ovector ranges that aren't actually being substituted in (I often have regexes with capturing groups that I never actually reference in the replacement string, so this could be faster than making user's do the check).

If we do the strict check, that the subject pointer and length are the same between match & substitute, then this would perhaps make sense too, without needing any additional option. Then we would be certain that UTF-8 characters are not being sliced in half.

Also, is there any other way to get invalid UTF-8/16/32 output from PCRE2, when using options | PCRE2_UTF | PCRE2_NEVER_BACKSLASH_C & ~(PCRE2_MATCH_INVALID_UTF | PCRE2_NO_UTF_CHECK)?

I think that's correct, yes. Valid UTF-8 input, with no \C, implies valid output.

I'm happy to submit a pull request with documentation changes to clarify things more, I just want to make sure I get all the details right.

We'd be grateful for any contributions. Thank you for raising this issue and discussing it with us.

[IsaacOscar]

I guess that would work...? But it's an odd requirement (to me). The code would be just as performant, and clearer, if the caller simply did the match, then called pcre2_substitute twice with PCRE2_SUBSTITUTE_MATCHED.
Yes, it wasn't something I was wanting to do, just curious if it was allowed. So yeah, no need to clarify that this is ok then.

It depends what you mean by "unsafe". If you use the match options PCRE2_UTF | PCRE2_NEVER_BACKSLASH_C, then you will get a match error if either of the pattern or subject strings is not valid UTF. And, the ovector offsets are guaranteed not to slice codepoints in half.

In Rust, when the output types are just raw bytes (as in my code, Rust's &[u8] and Vec<u8> types), invalid UTF-8 is not unsafe, what would be unsafe however is if pcre2_substitute tried to read or write to the old subject, as Rust will assume that the old pointer can safety be freed, or passed to another thread which might then modify it. It would also be unsafe if it tried to read past the end of the new subject, as that data might not be allocated, or might be in use by another thread.

If you then pass in a valid UTF-8 replacement string to pcre2_substitute, then we also guarantee that the output of the substitution is valid UTF-8. Everything is safe.

However, if you mutate the subject string in between the calls to match and substitute, in such a way that the ovector's offsets point to bytes which are not the start of UTF-8 characters, then output of pcre2_substitute will not be valid UTF-8. All the behaviour is defined, but you won't get a "valid string" as the substitute output, even if all the input strings are valid UTF-8 (because you used subject strings which were valid UTF-8, but with mismatched contents in between the calls to match and substitute).

Is this unsafe for you? Well, if your string type asserts that the contents are valid UTF-8, then you may want to find a way to "pin" the string in between calls to match & substitute.

It absolutely is unsafe in Rust if the output type is a UTF-8 string (i.e. Rust's normal &str and String types). Rust is very challenging sometimes when dealing with references to data, I actually think doing a runtime check that the ovector doesn't point inside multi-byte-characters is likely the best option here. It's certainly the easiest option that doesn't require dynamically allocating a copy of the entire subject (which seems like it would be slower if the subject was very large, but only a small part of it was captured by the regex?).
In fact I attempted to make my API try and save a reference to the original subject together with the match data, but I had great difficulty as the match-data was being used for calls to pcre2_match and prec2_subsitute with different subject strings, and they didn't all "live" for the same amount of time...

I agree, asserting the subject pointer is the same seems a little strict. However, it matches how the API is intended to be used. For UTF-8, mutating a string in general does not preserve the byte-offsets of characters (eg changing an a to à alters the length). So, the cases where you can actually have a different subject string are very limited.

I feel like being strict here is helpful to users: we make it sufficiently clear that we stop users from shooting themselves in the foot, for example, assuming that they can just uppercase an arbitrary string in between match & substitute.

Yeah I think I agree now, you can always extend the behaviour in the future if people need it to be more flexible (whereas making it less flexible would break existing code).

I'm happy to submit a pull request with documentation changes to clarify things more, I just want to make sure I get all the details right.

We'd be grateful for any contributions. Thank you for raising this issue and discussing it with us.

Thanks for listening, I was worried my use case would be a bit too strange.

I'll work on a pull request in the next day or so then, covering roughly the following:

1. Clarifying the documentation as to when PCRE2 can give invalid UTF-8 output
2. Clarifying the documentation that pcre2_subsitute with PCRE2_SUBSTITUTE_MATCHED can be used multiple times with the same match block.
3. Implementing a check (and new error code) that the new subject pointer, subject length, and start offset are identical (but not the contents), and updating the documentation accordingly.
4. Implementing an additional check, if PCRE2_UTF is on, but not PCRE2_NO_UTF_CHECK, that the ranges in the ovector fall on valid UTF boundaries. (but only when pcre2_substitute is actually reading the range in question?) I'll add a new error for this case and update the documentation.

Obviously I'll put stuff in separate commits and make sure there are appropriate test cases (the best way to write tests is to modify testdata right?).

Any other ideas I should try and implement?

[NWilson]

I'll work on a pull request in the next day or so then, covering roughly the following:

1. Clarifying the documentation as to when PCRE2 can give invalid UTF-8 output
2. Clarifying the documentation that pcre2_subsitute with PCRE2_SUBSTITUTE_MATCHED can be used multiple times with the same match block.
3. Implementing a check (and new error code) that the new subject pointer, subject length, and start offset are identical (but not the contents), and updating the documentation accordingly.

Note that the match block already stores these values, so implementing the check should be simple. It's also OK to add new fields to the match block if there's a need for them.

5. Implementing an additional check, if PCRE2_UTF is on, but not PCRE2_NO_UTF_CHECK, that the ranges in the ovector fall on valid UTF boundaries.

Don't worry about PCRE2_NO_UTF_CHECK. It's just a very corner-case match-time optimisation, and doesn't need to be consulted at all by pcre2_substitute().

but only when pcre2_substitute is actually reading the range in question

Just do the check unconditionally. It's cheap, and I don't like it when an API only validates its input sometimes - users write test cases that don't cover the times when the validation kicks in, and then some customer hits the error case that the developer hadn't anticipated. Better to make sure that the dev sees the validation.

Obviously I'll put stuff in separate commits and make sure there are appropriate test cases (the best way to write tests is to modify testdata right?).

👍

Amazing! That sounds very helpful and thorough, it's much appreciated.