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

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

Open
kocsismate wants to merge 1 commit into
php:masterfrom
kocsismate:uri-followup4
Open

Implement "Followup improvements for ext/uri" RFC - WHATWG URL building#22268
kocsismate wants to merge 1 commit into
php:masterfrom
kocsismate:uri-followup4

Conversation

@kocsismate

Copy link
Copy Markdown
Member

@kocsismate kocsismate requested a review from TimWolla as a code owner June 10, 2026 15:10
@kocsismate 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 kocsismate marked this pull request as draft June 10, 2026 16:12

@kocsismate kocsismate Jun 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kocsismate kocsismate marked this pull request as ready for review June 25, 2026 08:19
@kocsismate kocsismate requested a review from ndossche June 25, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant