Skip to content

Fix GH-20366 ext/uri: Do not throw ValueError on null-byte #20489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 19, 2025
Merged

Fix GH-20366 ext/uri: Do not throw ValueError on null-byte #20489

merged 2 commits into from
Nov 19, 2025
+393 −78

Conversation

@kocsismate
Copy link
Member

@kocsismate kocsismate commented Nov 14, 2025 •
edited
Loading

Fixes #20366

We still need to verify if ext/uri can handle null bytes properly.

@kocsismate kocsismate requested a review from TimWolla as a code owner November 14, 2025 20:34
@kocsismate kocsismate requested review from ndossche and removed request for TimWolla November 14, 2025 20:34
@DanielEScherzer
Copy link
Member

DanielEScherzer commented Nov 14, 2025

Is this for master or 8.5? And can it wait for 8.5.1?

@kocsismate kocsismate changed the base branch from master to PHP-8.5 November 14, 2025 20:36
@kocsismate
Copy link
Member Author

kocsismate commented Nov 14, 2025 •
edited
Loading

Is this for master or 8.5? And can it wait for 8.5.1?

Sorry! As always: I didn't set the target... It's a bug fix which fixes null-byte handling for WHATWG. It is completely broken in the RC-s right now.

I think it's not a deal-breaker for 8.5.0... It seems like an edge case for me. @DanielEScherzer

@kocsismate kocsismate requested a review from TimWolla November 14, 2025 20:40
ndossche
ndossche approved these changes Nov 16, 2025
View reviewed changes
Copy link
Member

@ndossche ndossche left a comment

Choose a reason for hiding this comment

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

I think this is right

TimWolla
TimWolla approved these changes Nov 18, 2025
View reviewed changes
Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

LGTM, but I would prefer the hexadecimal \x00 instead of the octal \0 in tests, because the latter can accidentally capture additional characters afterwards if they happen to be numeric.

nyamsprod added a commit to thephpleague/uri-src that referenced this pull request Nov 18, 2025
@nyamsprod
Backport PHP fix php/php-src#20489
9b527d2
nyamsprod added a commit to thephpleague/uri-polyfill that referenced this pull request Nov 18, 2025
@nyamsprod
Backport PHP fix php/php-src#20489
18519e1
@DanielEScherzer
Copy link
Member

DanielEScherzer commented Nov 18, 2025

Okay, this isn't in 8.5.0, to RM objections to the bug fix for 8.5.1

@kocsismate kocsismate force-pushed the fix-20366-null-byte-handling branch from 5b94e0c to 139983f Compare November 19, 2025 19:39
@kocsismate kocsismate merged commit 9743977 into php:PHP-8.5 Nov 19, 2025
9 of 10 checks passed
@kocsismate kocsismate deleted the fix-20366-null-byte-handling branch November 19, 2025 19:41
@kocsismate
Copy link
Member Author

kocsismate commented Nov 19, 2025

LGTM, but I would prefer the hexadecimal \x00 instead of the octal \0 in tests, because the latter can accidentally capture additional characters afterwards if they happen to be numeric.

I can workaround this in a followup test improvement.

kocsismate added a commit that referenced this pull request Nov 19, 2025
@kocsismate
Merge branch 'PHP-8.5'
15ca366 
* PHP-8.5:
  Fix GH-20366 ext/uri: Do not throw ValueError on null-byte (#20489)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TimWolla TimWolla TimWolla approved these changes

@ndossche ndossche ndossche approved these changes

@DanielEScherzer DanielEScherzer DanielEScherzer approved these changes

@edorian edorian Awaiting requested review from edorian

Assignees

No one assigned

Labels

Extension: uri

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ext/uri incorrectly throws ValueError when encountering null byte

4 participants

@kocsismate @DanielEScherzer @TimWolla @ndossche