Skip to content

Conversation

@jnoordsij
Copy link
Contributor

@jnoordsij jnoordsij commented Dec 15, 2025

Including polyfill for prior PHP versions without native support

See also https://wiki.php.net/rfc/url_parsing_api

Note this should result in generally more safe and consistent code, given the RFC goals. See also https://thephp.foundation/blog/2025/10/10/php-85-uri-extension/.

Implementation note: Illuminate\Foundation\resources\server.php has been intentionally skipped due to the 'minimal' nature of this file.

I'll later try and fix the 'Most complex example with read and write subarrays all in string' parsing or leave out the configuration url parse class for now.

@jnoordsij jnoordsij force-pushed the new-uri-handler branch 3 times, most recently from 37b8c0e to d726cb5 Compare December 15, 2025 11:35
Including polyfill for prior PHP versions without native support

See also https://wiki.php.net/rfc/url_parsing_api
@jnoordsij jnoordsij marked this pull request as draft December 15, 2025 11:36
@GrahamCampbell
Copy link
Collaborator

I think this would be best targetted at Laravel 13. People may also find it unexpected to be forced into RFC 3986, and expect parsing to be more forgiving.

@nyamsprod
Copy link

nyamsprod commented Dec 18, 2025

@jnoordsij thanks for the bug report in the puri-olyfill. While I am a proponent advocate of compliance everywhere it is not always the best route. I did take a quick glance at this PR and I believe you need to be aware of some caveats with the PHP native implementations.

Uri\Rfc3986\Uri:

This Uri object does not support i18n: So if for whatever reason a host like bébé.be is fed to your code, Uri\Rfc3986\Uri will throw an exception, parse_url will not. It may parse it wrongly too bug it will parse it 😄 . The Uri object also always expect well encoded components. If the component is not correctly encoded (ie no UTF-8 like characters are permitted anywhere) an exception will be thrown.

if you opt to use Uri\Whatwg\Url

Since Laravel is a web first framework , this is also a valid option. But again, there are some limitations. While the Url object supports i18n, it always requires absolute URI (aka scheme MUST be present always) this also is not the case for parse_url.

So while I understand the need of using modern PHP I would be cautious during the PR review to take into account these informations. One does not move away from parse_url that easy, hence why during the RFC period noone suggested its deprecation.

@jnoordsij
Copy link
Contributor Author

@nyamsprod thanks for sharing your expertise on the matter. As @GrahamCampbell already rightfully pointed out, the fact that this refactoring does actually go paired with a change in behavior does probably warrant targeting the next major release.

I've already been seeing this in tests of the db url parser, which requires some more changes or potentially should be skipped entirely for this reason. So I'll likely follow up by introducing some more incremental changes and some more thoughts/reasoning on wether or not refactoring is warranted on a per-case basis.

@lucasmichot
Copy link
Contributor

Why not simply use Illuminate\Support\Uri ?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants