Implement "Followup improvements for ext/uri" RFC - URL building with base URL - #23526
Implement "Followup improvements for ext/uri" RFC - URL building with base URL#23526kocsismate wants to merge 1 commit into
Conversation
… base URL RFC: https://wiki.php.net/rfc/uri_followup#uri_building Add support for passing a non-null $baseUrl parameter for Uri\WhatWg\UrlBuilder::build().
| } | ||
| } | ||
|
|
||
| ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser_whatwg_resolve_reference_from_zval( |
There was a problem hiding this comment.
Overall, this code below is pretty much hacky, works coincidentally, and to be honest I wish it wouldn't exist. 😅 On the other hand, it gets the job done! So thank you @arnaud-lb for the suggestion, your idea works indeed :) I would have never thought about it.
Due to the above mentioned implementation difficulties though, IMO we should prioritize if the feature is worth more or the sanity of our code ^^
| try { | ||
| $builder->build(new Uri\WhatWg\Url("scheme:opaque-path")); | ||
| } catch (Throwable $e) { | ||
| echo $e::class, ": ", $e->getMessage(), PHP_EOL; |
There was a problem hiding this comment.
| echo $e::class, ": ", $e->getMessage(), PHP_EOL; | |
| echo $e::class, ': ', $e->getMessage(), "\n"; |
Allow me the nit. 😅
| string(4) "user" | ||
| ["password"]=> | ||
| string(4) "pass" |
There was a problem hiding this comment.
Is this right? In #22268 (comment) you mention parsing rules. Resolving //example.net:124/foo/bar/baz drops user:pass. Are they here intentionally preserved? How is the builder generally supposed to work? From the RFC, your comments, and this test here it's not really clear to me what's actually intended behaviour. Like, generally, should this resolve the builder components against the base like parsing or replace the defined components of the base and keep the rest?
3 participants
RFC: https://wiki.php.net/rfc/uri_followup#uri_building
Add support for passing a non-null $baseUrl parameter for Uri\WhatWg\UrlBuilder::build().
This PR is not complete yet (tests are missing).