[RFC] Add RFC 3986 and WHATWG compliant URL parsing support #14461
Conversation
| @@ -3715,7 +3715,6 @@ function uniqid(string $prefix = "", bool $more_entropy = false): string {} | |||
|
|
|||
| /** | |||
| * @return int|string|array<string, int|string>|null|false | |||
| * @compile-time-eval | |||
There was a problem hiding this comment.
Removed for benchmarking purposes
ext/url/php_url.c
Outdated
|
|
||
| static void cleanup_parser(void) | ||
| { | ||
| if (++URL_G(urls) % 500 == 0) { |
There was a problem hiding this comment.
Approach is copy-pasted from lexbor/lexbor#206
823e4c6 to
e65deb7
Compare
0ccffa0 to
04d4e6e
Compare
d0b9a3d to
b9293a2
Compare
20f2844 to
0d2d2c3
Compare
| @@ -141,6 +141,7 @@ static void user_shutdown_function_dtor(zval *zv); | |||
| static void user_tick_function_dtor(user_tick_function_entry *tick_function_entry); | |||
|
|
|||
| static const zend_module_dep standard_deps[] = { /* {{{ */ | |||
| ZEND_MOD_REQUIRED("uri") | |||
There was a problem hiding this comment.
If uri is required, it should not require the --enable-uri flag to build it. It should always be built, like ext/random.
ext/soap/tests/bugs/bug38067.phpt
Outdated
| @@ -32,13 +32,13 @@ class TestSoapClient extends SoapClient { | |||
|
|
|||
| $client = new TestSoapClient(__DIR__.'/bug38067.wsdl', | |||
| array('encoding' => 'ISO-8859-1')); | |||
| $str = 'test: �'; | |||
| $str = 'test: �'; | |||
There was a problem hiding this comment.
Unintended change? Did you editor treat this file as UTF-8 instead of ISO-8859-1?
There was a problem hiding this comment.
Yes, it's surely an editor issue.
| @@ -604,7 +604,7 @@ public function __getLastRequestHeaders(): ?string {} | |||
| public function __getLastResponseHeaders(): ?string {} | |||
|
|
|||
| /** @tentative-return-type */ | |||
| public function __doRequest(string $request, string $location, string $action, int $version, bool $oneWay = false): ?string {} | |||
| public function __doRequest(string $request, string $location, string $action, int $version, bool $oneWay = false, ?string $uriParserClass = null): ?string {} | |||
There was a problem hiding this comment.
Given the number of tests that you changed, is this a breaking change? If so, it needs to be noted in the RFC.
There was a problem hiding this comment.
yes, that's a good idea
| [Enable URI support])]) | ||
|
|
||
| if test "$PHP_URI" != "no"; then | ||
| PHP_LEXBOR_CFLAGS="-I$abs_srcdir/ext/dom/lexbor -DLEXBOR_STATIC -I$abs_srcdir/ext/uri/uriparser/include" |
There was a problem hiding this comment.
This effectively makes ext/dom a dependency of ext/uri. Should Lexbor be moved somewhere else?
There was a problem hiding this comment.
It's just a WIP behavior from the early days of the PR. Once I manage to make the RFC approved, I'll create a lexbor "extension" (similarly to mysqlnd).
| PHP_ADD_EXTENSION_DEP(uri, dom) | ||
| PHP_NEW_EXTENSION(uri, [php_uri.c php_uri_common.c php_lexbor.c php_uriparser.c $URIPARSER_SOURCES], $ext_shared,,$PHP_LEXBOR_CFLAGS) |
There was a problem hiding this comment.
| PHP_ADD_EXTENSION_DEP(uri, dom) | |
| PHP_NEW_EXTENSION(uri, [php_uri.c php_uri_common.c php_lexbor.c php_uriparser.c $URIPARSER_SOURCES], $ext_shared,,$PHP_LEXBOR_CFLAGS) | |
| PHP_NEW_EXTENSION(uri, [php_uri.c php_uri_common.c php_lexbor.c php_uriparser.c $URIPARSER_SOURCES], $ext_shared,,$PHP_LEXBOR_CFLAGS) | |
| PHP_ADD_EXTENSION_DEP(uri, dom) |
For this macro to be effective, it should be called after the PHP_NEW_EXTENSION.
There was a problem hiding this comment.
Thank you! I'll take care of this once the RFC is accepted :)
2nd take after the failed experiment with #11315
RFC: https://wiki.php.net/rfc/url_parsing_api