Repository navigation
ext/standard: Minor NUL byte rejection fixes - #25171
Conversation
There was a problem hiding this comment.
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.
Don't worry, I will rebase & merge so it will appears in different commit. |
d82f06d to
445282f
Compare
|
The mac test failure seems to be unrelated. |
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. |
This reject NUL bytes in parameters in quoted_printable_decode and version_compare function