ext/standard: Reject null bytes in parse_str()#21942
Conversation
|
@LamentXU123 Can you make sure to use the PR title as the commit title for single-commit PRs in the future? That makes merging much easier, because GitHub implicitly uses the commit data for single-commit PRs. |
Weird behavior....
Uh got it :) |
|
|
||
| ZEND_PARSE_PARAMETERS_START(2, 2) | ||
| Z_PARAM_STRING(arg, arglen) | ||
| Z_PARAM_PATH(arg, arglen) |
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
There was a problem hiding this comment.
the implementation of parse_str is based on C str, and therefore if the parsed string contains NUL bytes it will truncates the trailing part. Now, we have already done the same fix of getenv and putenv. From this fix we can throw a ValueError to users instead of silently truncate the input.
There was a problem hiding this comment.
I see, it would be great to add a comment here, that the limitation is due php_default_treat_data not supporting an explicit input string length.
There was a problem hiding this comment.
Uh yes.That'd improve readability. Humm, we have lots of similar fix out there so adding comments to this case alone seems not to be effective... Could you please send a PR after this is merged to add this line of comment to every similar fix we have done before (i.e. reject NUL bytes because it silently truncates stuff)? Thank in advance!
resulted in
reproduce: https://3v4l.org/UF6Pu#v
This might be the last important occurrence in the standard extension :)