Skip to content

ext/standard: Minor NUL byte rejection fixes - #25171

Merged
LamentXU123 merged 2 commits into
php:masterfrom
LamentXU123:fix/nul-string-apis
Nov 4, 2026
Merged

LamentXU123 merged 2 commits into
php:masterfrom
LamentXU123:fix/nul-string-apis

Conversation

@LamentXU123

Copy link
Copy Markdown
Member

This reject NUL bytes in parameters in quoted_printable_decode and version_compare function

@iliaal iliaal left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The version_compare() part looks g2g, php_version_compare() works on C strings so rejecting is the only sane option. I'd split it into its own PR so it can land while quoted_printable_decode() gets reworked.

For quoted_printable_decode() I'd rather fix the truncation than reject. The decoder already passes through everything else that isn't valid QP, =00 decodes to a NUL anyway, and the convert.quoted-printable-decode filter keeps a raw NUL: "abc\0d=41f" gives abc\0dAf from the filter and abc from the function. Walking the input by ZSTR_LEN() instead of stopping at the terminator would make the two agree.

@LamentXU123

Copy link
Copy Markdown
Member Author

I'd split it into its own PR so it can land while quoted_printable_decode() gets reworked.

Don't worry, I will rebase & merge so it will appears in different commit.

@LamentXU123

Copy link
Copy Markdown
Member Author

The mac test failure seems to be unrelated.

@LamentXU123
LamentXU123 requested a review from iliaal November 4, 2026 07:05
@LamentXU123
LamentXU123 merged commit 8ba59c8 into php:master Nov 4, 2026
33 of 34 checks passed
@TimWolla

TimWolla commented Nov 4, 2026

Copy link
Copy Markdown
Member

php_version_compare() works on C strings so rejecting is the only sane option.

I'd argue the sane option would be making it not take C strings instead. Yes, version numbers are not typically expected to hold NUL bytes, but rejecting them for no good reason (other than laziness) does not really help the developer experience either.

@LamentXU123

Copy link
Copy Markdown
Member Author

php_version_compare() works on C strings so rejecting is the only sane option.

I'd argue the sane option would be making it not take C strings instead. Yes, version numbers are not typically expected to hold NUL bytes, but rejecting them for no good reason (other than laziness) does not really help the developer experience either.

Why do you think we should process a version number with NUL bytes anyways?

FWIW, I reject them because NUL truncation is more like a C side stuff leaking to PHP, and most of the time doesn't make sense. "1.0.0\0RC1" is not a valid version number anyways and will cause confusion in whether we see it as "1.0.0" or "1.0.0RC1", in this case I'd suggest to reject it as what we done all the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants