Repository navigation
ext/uri incorrectly throws ValueError when encountering null byte #20366
Description
Description
The following code:
Resulted in this output:
Currently the tests returns a `ValueError`
But I expected this output instead:
Uri\WhatWg\InvalidUrlException: The specified URI is malformed; Invalid URL (DomainInvalidCodePoint)
To me both are acceptable, I would lean toward the second options which is what the WHATWG URL standard would expect but I understand the current output. I just want to know if there is a clear guideline when to throw a ValueError vs InvalidUrlException because from a DX usage it may mean that withers can only throw ValueError ? 🤔
PHP Version
using php master branch (but this is irrelevant)
Operating System
No response
Activity
devnexen commentedon Nov 2, 2025
it seems to me the parameter is of type Z_PARAM_PATH_STR thus with the null byte in the middle of your test case, it already takes precedence though I guess (as not even reaching the lexbor parser).
nyamsprod commentedon Nov 2, 2025
OK so following your reasoning, the
nullbyte character will never reach the lexbor parser soInvalidUrlExceptioncan never be thrown.devnexen commentedon Nov 2, 2025
IMHO, those are not bugs at all.
nyamsprod commentedon Nov 2, 2025
I am not saying it is a bug I just need to understand when one is thrown vs the other to improve my polyfill. Currently the polyfill never throw the
ValueErrorand everything is validated by the WHATWG parser which means I need to add a specific fix for the null byte in order to match PHP implementation.devnexen commentedon Nov 4, 2025
yes so to rephrase it, it is expected outcome in my view. Hope it helps.
nyamsprod commentedon Nov 4, 2025
@devnexen I have added the following
prior to any validation by the userland URI parser.
I hope this is the only gatekeeper to look for 🤔
devnexen commentedon Nov 4, 2025
I ll let @TimWolla respond on this particular point ;)
TimWolla commentedon Nov 4, 2025
Yes. That's Z_PARAM_PATH_STR vs Z_PARAM_STR internally.
As for this issue, I'd be inclined to just pass along NUL bytes as-is to the parsers for consistent error handling, but I didn't yet have time to check if all the methods are fully NUL-safe (and I would need to check with Mate as well).
nyamsprod commentedon Nov 4, 2025
@TimWolla thanks for the comment. So if you end up passing it along I will be able to remove my current "patch" then. Let me know if this is the case.
devnexen commentedon Nov 4, 2025
@TimWolla I think your point is valid, might be a tougher decision in another hand. i.e. most of other php extensions treat nul bytes from input prevently. But again, making it a thinner wrapper around lexbor is a valid POV.
kocsismate commentedon Nov 13, 2025
I used
Z_PARAM_PATH_STReverywhere in ext/uri when a string is expected for safety reasons. Back then, I didn't know that the\0may be successfully passed to WHATWG URIs: https://timothygu.me/urltester/#input=https%3A%2F%2Fexample.com%2Ffo%5C0o (see the "your browser" column), it gets percent-encoded... (I only realized it a few days ago)So it seems we will have to verify if all functions nul-byte safe - I'm not really knowledgable though in this regard. :(
kocsismate commentedon Nov 14, 2025
(I'm working on a fix right now)
kocsismate commentedon Nov 14, 2025
Fix is here with a lot of tests: #20366 We should still verify if null-byte handling is properly implemented.
[-]ext-uri when to throw ValueError vs InvalidUriException[/-][+]ext/uri incorrectly throws ValueError when encountering null byte[/+]Fix phpGH-20366 Do not throw ValueError on null-bytes
Fix GH-20366 ext/uri: Do not throw ValueError on null-byte (#20489)
Merge branch 'PHP-8.5'