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)

Very clever idea again, but I'm not exactly sure that "inverting" parsing would work correctly either..

let base be https://example.com:80/foo

let url be scheme://example.net/bar

Then this should be scheme://example.net/bar. However, it would be scheme://example.net:80/bar. I guess, the algorithm could be improved by discarding base if url has a scheme.

@lexborisov do you maybe have some insights whether the above idea would work to support base URLs with the WHATWG URL Builder that was proposed in https://wiki.php.net/rfc/uri_followup#builder ? The complexity is that we store the components separately, and we need to build them together (via the setters) + use the base URL - but the setters obviously don't support the base URL.

The build() is implemented here: https://github.com/php/php-src/pull/22268/changes#diff-1ac5fecaced76e4592af01f30d246ce17edfd60c29ebe0e01d57b2988ec10fbdR892

@arnaud-lb can we merge what we have so far? and then I can go back to this question a bit later? I'm worried a little bit, because quite a few of the URI followup RFC haven't been merged yet :(

Hi @kocsismate

I've started looking into this thread, but I'll need some time. I have a lot on my plate right now.
At first glance, it seems like this can be resolved with minimal effort.

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)

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

arnaud-lb

This looks good to me (except a few nits), but we should throw when a $baseUrl parameter is passed to ::build() until this is implemented.

arnaud-lb · 2026-08-18T11:18:58Z

+		if (php_uri_whatwg_is_ascii_tab_or_newline(uc)) {
+			continue;
+		}


This is consistent with Url::withScheme(), but also non-standard as per https://url.spec.whatwg.org/#url-writing:

A URL-scheme string must be one ASCII alpha, followed by zero or more of ASCII alphanumeric, U+002B (+), U+002D (-), and U+002E (.).

arnaud-lb · 2026-08-18T11:42:18Z

+        if (php_uri_whatwg_is_ascii_tab_or_newline(uc)) {
+            continue;


Nit: This can skip tabs and newlines in the middle of the scheme

Yes, this is intentional because of step 3 in https://url.spec.whatwg.org/#url-parsing:

Remove all ASCII tab or newline from input.

Or do you think this step shouldn't apply for the Builder?

There's also a special case for leading/trailing C0 or space characters:

If url is not given:

Set url to a new URL

If input contains any leading or trailing C0 control or space, invalid-URL-unit validation error.

Remove any leading and trailing C0 control or space from input.

The url parameter is given when we are using the setters. However, IMO the Builder should rather adhere to parsing rules (#22268 (comment)), so this section could apply to our use-case.

But at last, I didn't implement the Remove any leading and trailing C0 control or space from input., since we don't really know where the end of the URL is. Although, the scheme validator could ignore leading CO control or space characters at least...

But I don't really know what the best solution could be :(.

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 force-pushed the uri-followup4 branch from 1fa2162 to 3733c48 Compare August 14, 2026 12:20

kocsismate added 2 commits August 15, 2026 13:07

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

27da2ed

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

Address review comments

ce23fe3

kocsismate added 5 commits August 15, 2026 13:07

Address review feedback + fixes

367b190

Fix test name

a9a3b19

zval *errors to HashTable *errors

ad98393

Fix code review suggestions again

b8d9786

Migrate to the new lxb_url_parse_host_ipv6() API

a7145f3

kocsismate force-pushed the uri-followup4 branch from 3733c48 to a7145f3 Compare August 15, 2026 11:08

arnaud-lb approved these changes Aug 18, 2026

View reviewed changes

Some code review fixes

24d382a

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kocsismate Aug 15, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kocsismate commented Jul 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

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!

arnaud-lb left a comment • 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kocsismate Jun 25, 2026 •

edited

Loading

kocsismate Aug 15, 2026 •

edited

Loading

arnaud-lb left a comment •

edited

Loading