Skip to content

Reject malformed Unicode escape sequences - #1176

Merged
nikic merged 1 commit into
nikic:masterfrom
ydah:fix-invalid-unicode-escapes
Oct 5, 2026
Merged

nikic merged 1 commit into
nikic:masterfrom
ydah:fix-invalid-unicode-escapes

Conversation

@ydah

@ydah ydah commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

String_ only matches complete hexadecimal Unicode escapes, leaving malformed sequences as literal text instead of reporting an error:

String_::parse('"\u{zz}"'); // Before: literal \u{zz}
String_::parse('"\u{}"');   // Before: literal \u{}
// After: both throw PhpParser\Error("Invalid UTF-8 codepoint escape sequence")

Also match the \u{ prefix and reject it when no complete hexadecimal escape was captured. This catches invalid digits, empty escapes, and missing closing braces.

<?php
"\u{41";
-----
Invalid UTF-8 codepoint escape sequence on line 2

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth testing "\u{" as well, as the minimal invalid sequence?

On the other hand, I'd drop the tests with variables below -- at least it's not obvious to me what extra coverage they are adding.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Added a test for \u{ and removed the cases with variables as suggested.

@ydah
ydah force-pushed the fix-invalid-unicode-escapes branch from 457d901 to cbd65c1 Compare October 5, 2026 06:15
@ydah
ydah requested a review from nikic October 5, 2026 06:15
@nikic
nikic merged commit 8d2b672 into nikic:master Oct 5, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants