Implement "Followup improvements for ext/uri" RFC - WHATWG URL building by kocsismate · Pull Request #22268 · php/php-src

kocsismate · 2026-06-10T15:10:46Z

RFC: https://wiki.php.net/rfc/uri_followup#uri_building

kocsismate · 2026-06-25T08:18:47Z

+		goto failure;
+	}
+
+	if (lexbor_base_url != NULL) {


What's a big shame is that apparently it's not possible to properly use the builder with a base URL :(

if we try to add the base URL after the input URL is built, then legitimate relative URLs are rejected (e.g. /foo + https://example.com), because /foo is not a valid URL on its own

If we try to build the input URL with the base URL in the same time, then the parsing algorithm must be used (currently, only setters are used with a hack on line 821). Then the complication is to find some URL component that is suitable for parsing:

for special, full URLs: scheme + host is needed at least (e.g. https://example.com)

for non-special full URLs: scheme is needed at least (e.g. https://)

for relative URLs: the path is needed at least (e.g. /foo/bar)

And in the 2nd case, the question arises if it's ok to parse only the minimally required components and then set the rest of the components, or the whole input URL must be built and parsed all at once.

I have limited knowledge of lexbor, but can we build a string out of the components we have in the builder, and then call the parser with that? Assuming that we can build the string unambiguously, any errors from missing components would be reported by the parser.

thanks for the idea! Unfortunately, - as far as I can see the situation - the main blocker is achiveving unambiguous recomposition. E.g.

$builder = new Uri\WhatWg\UrlBuilder(); $builder->setScheme("git"); $builder->setPath("//refs/heads/main"); $builder->build();

This would be recomposed as git://refs/heads/main, however it should rather be git:////refs/heads/main. So overall, we would end up reimplementing the WHATWG URL spec to prevent some cases. Since it's very long specification with a lot of special cases, I wouldn't even dare to attempt this. :(

Just brainstorming, but would that work?

base = parse base url

url = clone base

for each component in builder:

lxb_url_parse_basic(..., url, base, ..., override_state, ...) (with override_state the state corresponding to this component, e.g. LXB_URL_STATE_PATH_START_STATE for the path)

kocsismate · 2026-07-01T08:38:08Z

May I have a review soon so that this can potentially be included into alpha 2 at least? :)

kocsismate · 2026-07-01T08:55:27Z

+	 * The URL is initialized as LXB_URL_SCHEMEL_TYPE__UNDEF but this would prevent the scheme to be updated
+	 * in case of non-special schemes due to https://github.com/php/php-src/blob/27d7b799c0a13578ee0506b428b8ddc209ffb010/ext/lexbor/lexbor/url/url.c#L1402
+	 */
+	if (!php_uri_parser_whatwg_is_special_scheme(scheme)) {


I'll try to check if it's something that we can omit somehow...

I didn't find any way how this workaround could be eliminated...

TimWolla

Had a very superficial first look.

TimWolla

Looked at some tests and some of the C files. Not yet through the PR, but you can likely already make some changes in response to this review.

TimWolla

Went through all tests now. Didn't deeply look at the C code yet, but I'm not super stoked about needing to reimplement the component validation ourselves.

TimWolla · 2026-08-01T13:43:02Z

+$builder->setHost("example.com");
+$builder->setFragment("\tfoo");
+$errors = [];
+$url = $builder->build(errors: $errors);


It seems this is the only test testing $errors.

arnaud-lb

I've only done a partial review yet

…set() Found in php#22268 (comment)

…set() (#23144) Originally found in #22268 (comment)

@kocsismate

Added lxb_url_parse_host_ipv6() — a public entry point to the IPv6 parser from the WHATWG specification: https://url.spec.whatwg.org/#concept-ipv6-parser The address is accepted both with and without the surrounding square brackets: "::1" and "[::1]" give the same result. #402 The API was requested in #402 for use by php/php-src#22268. Suggested-by: Máté Kocsis (@kocsismate)

- URL: added public IPv6 parser. (lexbor/lexbor@a7dbebbe) - URL: added public percent-encoder API. (lexbor/lexbor@2b24b565)

RFC: https://wiki.php.net/rfc/uri_followup#uri_building

kocsismate requested a review from TimWolla as a code owner June 10, 2026 15:10

github-actions Bot added ABI break Extension: uri labels Jun 10, 2026

kocsismate changed the title ~~IImplement "Followup improvements for ext/uri" RFC - WHATWG URL building~~ Implement "Followup improvements for ext/uri" RFC - WHATWG URL building Jun 10, 2026

kocsismate marked this pull request as draft June 10, 2026 16:12

kocsismate force-pushed the uri-followup4 branch from 97cb0bf to 0655d8c Compare June 25, 2026 07:53

kocsismate commented Jun 25, 2026

View reviewed changes

kocsismate marked this pull request as ready for review June 25, 2026 08:19

kocsismate requested a review from ndossche June 25, 2026 13:32

kocsismate force-pushed the uri-followup4 branch from 0655d8c to af73d4a Compare June 27, 2026 21:30

kocsismate commented Jul 1, 2026

View reviewed changes

Comment thread ext/uri/php_uri.c Outdated

kocsismate commented Jul 1, 2026

View reviewed changes

ndossche requested changes Jul 1, 2026

View reviewed changes

Comment thread ext/uri/php_uri.c

Comment thread ext/uri/uri_parser_whatwg.c Outdated

Comment thread ext/uri/uri_parser_whatwg.c Outdated

TimWolla reviewed Jul 1, 2026

View reviewed changes

Comment thread ext/uri/uri_parser_whatwg.c Outdated

Comment thread ext/uri/uri_parser_whatwg.c Outdated

kocsismate commented Jul 3, 2026

View reviewed changes

Comment thread ext/uri/uri_parser_whatwg.c

TimWolla self-requested a review July 17, 2026 18:25

TimWolla reviewed Jul 21, 2026

View reviewed changes

kocsismate force-pushed the uri-followup4 branch from 4a39e3f to 8c50acd Compare July 24, 2026 12:45

TimWolla reviewed Aug 1, 2026

View reviewed changes

arnaud-lb reviewed Aug 3, 2026

View reviewed changes

Comment thread ext/uri/php_uri.c Outdated

kocsismate force-pushed the uri-followup4 branch from 9e71a5c to 1fa2162 Compare August 7, 2026 06:53

kocsismate mentioned this pull request Aug 7, 2026

Expose lxb_url_ipv6_parse() lexbor/lexbor#402

Closed

kocsismate added a commit to kocsismate/php-src that referenced this pull request Aug 8, 2026

Fix typed reference property writes during Uri\Rfc3986\UriBuilder::re…

c45020b

…set() Found in php#22268 (comment)

kocsismate mentioned this pull request Aug 8, 2026

Fix typed reference property writes during Uri\Rfc3986\UriBuilder::reset() #23144

Merged

kocsismate added a commit that referenced this pull request Aug 10, 2026

Fix typed reference property writes during Uri\Rfc3986\UriBuilder::re…

e63e2b7

…set() (#23144) Originally found in #22268 (comment)

kocsismate added 3 commits August 14, 2026 13:48

lexbor: Add new URL patches

89e4f75

- URL: added public IPv6 parser. (lexbor/lexbor@a7dbebbe) - URL: added public percent-encoder API. (lexbor/lexbor@2b24b565)

Implement "Followup improvements for ext/uri" RFC - WHATWG URL building

01de061

RFC: https://wiki.php.net/rfc/uri_followup#uri_building

Address review comments

0d2f70d

kocsismate added 5 commits August 14, 2026 14:02

Address review feedback + fixes

a72769f

Fix test name

0af9ce1

zval *errors to HashTable *errors

1d822bf

Fix code review suggestions again

4618287

Migrate to the new lxb_url_parse_host_ipv6() API

3733c48

kocsismate force-pushed the uri-followup4 branch from 1fa2162 to 3733c48 Compare August 14, 2026 12:20

Uh oh!

Conversation

kocsismate commented Jun 10, 2026

Uh oh!

kocsismate Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arnaud-lb Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

kocsismate Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

arnaud-lb Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

kocsismate commented Jul 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kocsismate Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

kocsismate Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimWolla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimWolla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimWolla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimWolla Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arnaud-lb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kocsismate Jun 25, 2026 •

edited

Loading