Forbid \C in UTF-8 patterns #21139
Open
Forbid \C in UTF-8 patterns #21139
+22
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
devnexen
reviewed
Feb 5, 2026
| coptions |= PCRE2_UCP; | ||
| #endif | ||
| /* The \C escape sequence is unsafe in PCRE2_UTF mode */ | ||
| coptions |= PCRE2_NEVER_BACKSLASH_C; |
Member
There was a problem hiding this comment.
would need to document it somehow (UPGRADING ?) wdyt ?
| } | ||
| pcre2_get_error_message(errnumber, error, sizeof(error)); | ||
| if (errnumber == PCRE2_ERROR_BACKSLASH_C_CALLER_DISABLED) { | ||
| strlcpy((char*)error, "using \\C is incompatible with the 'u' modifier", sizeof(error)); |
Member
There was a problem hiding this comment.
yes the default message is a bit vague in this context, makes sense.
Member
|
I know enough pcre2 to see it s LGTM but cc @ndossche just in case :) |
ndossche
approved these changes
Feb 5, 2026
Comment on lines
+792
to
797
| if (errnumber == PCRE2_ERROR_BACKSLASH_C_CALLER_DISABLED) { | ||
| strlcpy((char*)error, "using \\C is incompatible with the 'u' modifier", sizeof(error)); | ||
| } else { | ||
| pcre2_get_error_message(errnumber, error, sizeof(error)); | ||
| } | ||
| php_error_docref(NULL,E_WARNING, "Compilation failed: %s at offset %zu", error, erroffset); |
Member
There was a problem hiding this comment.
I think the strlcpy call is ugly, but I won't object. Why not smth like this (and possibly rename error to error_buf):
Suggested change
| if (errnumber == PCRE2_ERROR_BACKSLASH_C_CALLER_DISABLED) { | |
| strlcpy((char*)error, "using \\C is incompatible with the 'u' modifier", sizeof(error)); | |
| } else { | |
| pcre2_get_error_message(errnumber, error, sizeof(error)); | |
| } | |
| php_error_docref(NULL,E_WARNING, "Compilation failed: %s at offset %zu", error, erroffset); | |
| const char *err_msg = (const char *) error; | |
| if (errnumber == PCRE2_ERROR_BACKSLASH_C_CALLER_DISABLED) { | |
| err_msg = "using \\C is incompatible with the 'u' modifier"; | |
| } else { | |
| pcre2_get_error_message(errnumber, error, sizeof(error)); | |
| } | |
| php_error_docref(NULL,E_WARNING, "Compilation failed: %s at offset %zu", err_msg, erroffset); |
Member
There was a problem hiding this comment.
on that, I was surprised to see pcre2 provides a strcpy-like only for their own buffer type.
Member
Author
Member
|
Since it can otherwise induce some potentially unsafe behaviour, I think 8.4 is reasonable. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Possible fix for GH-21134.
\Cresults in undefined, potentially unsafe behavior in UTF-8 mode, so error out in case the escape sequence is used in UTF-8 mode.