From 6be0e0299b42bbc5f984782aaf303f24ada5bb01 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Mon, 21 Sep 2026 15:55:38 +0800 Subject: [PATCH 1/2] ext/uri: fixed url component delimiter handling --- ...gment_success_hashmark_only_with_base.phpt | 39 +++++++++++++++++++ ..._success_question_mark_only_with_base.phpt | 39 +++++++++++++++++++ ext/uri/uri_parser_whatwg.c | 32 ++++++++------- 3 files changed, 96 insertions(+), 14 deletions(-) create mode 100644 ext/uri/tests/whatwg/builder/fragment_success_hashmark_only_with_base.phpt create mode 100644 ext/uri/tests/whatwg/builder/query_success_question_mark_only_with_base.phpt diff --git a/ext/uri/tests/whatwg/builder/fragment_success_hashmark_only_with_base.phpt b/ext/uri/tests/whatwg/builder/fragment_success_hashmark_only_with_base.phpt new file mode 100644 index 000000000000..c73538afe96e --- /dev/null +++ b/ext/uri/tests/whatwg/builder/fragment_success_hashmark_only_with_base.phpt @@ -0,0 +1,39 @@ +--TEST-- +Test Uri\WhatWg\UrlBuilder::setFragment() - success - hashmark only with base URL +--FILE-- +setFragment('#') + ->build($base); + +var_dump($url->toAsciiString()); +var_dump($url); +var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString()))); +var_dump($url->equals(new Uri\WhatWg\Url('#', $base), Uri\UriComparisonMode::IncludeFragment)); + +?> +--EXPECTF-- +string(39) "https://example.com/base/path?oldQuery#" +object(Uri\WhatWg\Url)#%d (%d) { + ["scheme"]=> + string(5) "https" + ["username"]=> + NULL + ["password"]=> + NULL + ["host"]=> + string(11) "example.com" + ["port"]=> + NULL + ["path"]=> + string(10) "/base/path" + ["query"]=> + string(8) "oldQuery" + ["fragment"]=> + string(0) "" +} +bool(true) +bool(true) diff --git a/ext/uri/tests/whatwg/builder/query_success_question_mark_only_with_base.phpt b/ext/uri/tests/whatwg/builder/query_success_question_mark_only_with_base.phpt new file mode 100644 index 000000000000..c42ae389fac0 --- /dev/null +++ b/ext/uri/tests/whatwg/builder/query_success_question_mark_only_with_base.phpt @@ -0,0 +1,39 @@ +--TEST-- +Test Uri\WhatWg\UrlBuilder::setQuery() - success - question mark only with base URL +--FILE-- +setQuery('?') + ->build($base); + +var_dump($url->toAsciiString()); +var_dump($url); +var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString()))); +var_dump($url->equals(new Uri\WhatWg\Url('?', $base), Uri\UriComparisonMode::IncludeFragment)); + +?> +--EXPECTF-- +string(30) "https://example.com/base/path?" +object(Uri\WhatWg\Url)#%d (%d) { + ["scheme"]=> + string(5) "https" + ["username"]=> + NULL + ["password"]=> + NULL + ["host"]=> + string(11) "example.com" + ["port"]=> + NULL + ["path"]=> + string(10) "/base/path" + ["query"]=> + string(0) "" + ["fragment"]=> + NULL +} +bool(true) +bool(true) diff --git a/ext/uri/uri_parser_whatwg.c b/ext/uri/uri_parser_whatwg.c index 48ffc767207d..47b12661bd2d 100644 --- a/ext/uri/uri_parser_whatwg.c +++ b/ext/uri/uri_parser_whatwg.c @@ -1137,26 +1137,30 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser if (Z_TYPE_P(query) == IS_STRING) { lxb_url_query_set_null(lexbor_url); - lxb_url_parser_clean(&lexbor_parser); - status = lxb_url_parse_basic(&lexbor_parser, lexbor_url, lexbor_base_url, - (lxb_char_t *) Z_STRVAL_P(query), Z_STRLEN_P(query), - LXB_URL_STATE_QUERY_STATE, LXB_ENCODING_AUTO - ); - php_uri_parser_whatwg_build_errors_and_throw(status, "query", Z_ARRVAL(errors)); - if (status != LXB_STATUS_OK) { + zend_result result; + if (Z_STRLEN_P(query) == 0) { + lexbor_str_init(&lexbor_url->query, lexbor_url->mraw, 1); + result = SUCCESS; + } else { + result = php_uri_parser_whatwg_query_write(lexbor_url, query, NULL); + } + php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors)); + if (result == FAILURE) { goto failure; } } if (Z_TYPE_P(fragment) == IS_STRING) { lxb_url_fragment_set_null(lexbor_url); - lxb_url_parser_clean(&lexbor_parser); - status = lxb_url_parse_basic(&lexbor_parser, lexbor_url, lexbor_base_url, - (lxb_char_t *) Z_STRVAL_P(fragment), Z_STRLEN_P(fragment), - LXB_URL_STATE_FRAGMENT_STATE, LXB_ENCODING_AUTO - ); - php_uri_parser_whatwg_build_errors_and_throw(status, "fragment", Z_ARRVAL(errors)); - if (status != LXB_STATUS_OK) { + zend_result result; + if (Z_STRLEN_P(fragment) == 0) { + lexbor_str_init(&lexbor_url->fragment, lexbor_url->mraw, 1); + result = SUCCESS; + } else { + result = php_uri_parser_whatwg_fragment_write(lexbor_url, fragment, NULL); + } + php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors)); + if (result == FAILURE) { goto failure; } } From dfa46bb2a775a880efc5d0c22444475dc61ab5ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Tue, 20 Oct 2026 13:05:18 +0200 Subject: [PATCH 2/2] Address review comments --- .../fragment_success_empty_string.phpt | 4 +- .../fragment_success_empty_with_base.phpt | 4 +- ...agment_success_empty_with_opaque_base.phpt | 4 +- .../fragment_success_hashmark_only.phpt | 37 +++++++++++++ ...s_opaque_spaces_with_empty_components.phpt | 48 +++++++++++++++++ .../builder/query_success_empty_string.phpt | 4 +- .../query_success_empty_with_base.phpt | 4 +- .../query_success_question_mark_only.phpt | 37 +++++++++++++ ext/uri/uri_parser_whatwg.c | 52 +++++-------------- 9 files changed, 144 insertions(+), 50 deletions(-) create mode 100644 ext/uri/tests/whatwg/builder/fragment_success_hashmark_only.phpt create mode 100644 ext/uri/tests/whatwg/builder/path_success_opaque_spaces_with_empty_components.phpt create mode 100644 ext/uri/tests/whatwg/builder/query_success_question_mark_only.phpt diff --git a/ext/uri/tests/whatwg/builder/fragment_success_empty_string.phpt b/ext/uri/tests/whatwg/builder/fragment_success_empty_string.phpt index 2fe13ffd7020..3d7be9923ca8 100644 --- a/ext/uri/tests/whatwg/builder/fragment_success_empty_string.phpt +++ b/ext/uri/tests/whatwg/builder/fragment_success_empty_string.phpt @@ -16,7 +16,7 @@ var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString()))); ?> --EXPECTF-- -string(21) "https://example.com/#" +string(20) "https://example.com/" object(Uri\WhatWg\Url)#%d (%d) { ["scheme"]=> string(5) "https" @@ -33,6 +33,6 @@ object(Uri\WhatWg\Url)#%d (%d) { ["query"]=> NULL ["fragment"]=> - string(0) "" + NULL } bool(true) diff --git a/ext/uri/tests/whatwg/builder/fragment_success_empty_with_base.phpt b/ext/uri/tests/whatwg/builder/fragment_success_empty_with_base.phpt index 04f92a8f7764..a18ef25fb886 100644 --- a/ext/uri/tests/whatwg/builder/fragment_success_empty_with_base.phpt +++ b/ext/uri/tests/whatwg/builder/fragment_success_empty_with_base.phpt @@ -1,5 +1,5 @@ --TEST-- -Test Uri\WhatWg\UrlBuilder::build() - success - replaces the base fragment with an empty fragment +Test Uri\WhatWg\UrlBuilder::build() - success - clears the base fragment with an empty string --FILE-- equals(new Uri\WhatWg\Url($url->toAsciiString()))); ?> --EXPECT-- -string(26) "https://example.com/a?old#" +string(25) "https://example.com/a?old" bool(true) diff --git a/ext/uri/tests/whatwg/builder/fragment_success_empty_with_opaque_base.phpt b/ext/uri/tests/whatwg/builder/fragment_success_empty_with_opaque_base.phpt index bcdb4d277b97..fe256b5565f3 100644 --- a/ext/uri/tests/whatwg/builder/fragment_success_empty_with_opaque_base.phpt +++ b/ext/uri/tests/whatwg/builder/fragment_success_empty_with_opaque_base.phpt @@ -1,5 +1,5 @@ --TEST-- -Test Uri\WhatWg\UrlBuilder::build() - success - resolves an empty fragment against an opaque base +Test Uri\WhatWg\UrlBuilder::build() - success - clears the fragment of an opaque base with an empty string --FILE-- toAsciiString()); ?> --EXPECT-- -string(15) "foo:opaque?old#" +string(14) "foo:opaque?old" enum(Uri\WhatWg\UrlValidationErrorType::InvalidUrlUnit) bool(true) string(18) "foo:opaque?old#old" diff --git a/ext/uri/tests/whatwg/builder/fragment_success_hashmark_only.phpt b/ext/uri/tests/whatwg/builder/fragment_success_hashmark_only.phpt new file mode 100644 index 000000000000..34c60b37d9d3 --- /dev/null +++ b/ext/uri/tests/whatwg/builder/fragment_success_hashmark_only.phpt @@ -0,0 +1,37 @@ +--TEST-- +Test Uri\WhatWg\UrlBuilder::setFragment() - success - hashmark only +--FILE-- +setScheme("https"); +$builder->setHost("example.com"); +$builder->setFragment("#"); +$url = $builder->build(); + +var_dump($url->toAsciiString()); +var_dump($url); +var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString()), Uri\UriComparisonMode::IncludeFragment)); + +?> +--EXPECTF-- +string(21) "https://example.com/#" +object(Uri\WhatWg\Url)#%d (%d) { + ["scheme"]=> + string(5) "https" + ["username"]=> + NULL + ["password"]=> + NULL + ["host"]=> + string(11) "example.com" + ["port"]=> + NULL + ["path"]=> + string(1) "/" + ["query"]=> + NULL + ["fragment"]=> + string(0) "" +} +bool(true) diff --git a/ext/uri/tests/whatwg/builder/path_success_opaque_spaces_with_empty_components.phpt b/ext/uri/tests/whatwg/builder/path_success_opaque_spaces_with_empty_components.phpt new file mode 100644 index 000000000000..25bcdc19affd --- /dev/null +++ b/ext/uri/tests/whatwg/builder/path_success_opaque_spaces_with_empty_components.phpt @@ -0,0 +1,48 @@ +--TEST-- +Test Uri\WhatWg\UrlBuilder::setPath() - success - trailing spaces with empty query and fragment inputs +--FILE-- +setScheme("foo"); +$builder->setPath("abc "); +$builder->setQuery(""); +$builder->setFragment(""); +$url = $builder->build(softErrors: $softErrors); + +var_dump($url->toAsciiString()); +var_dump($url); +var_dump($softErrors); + +?> +--EXPECTF-- +string(8) "foo:abc " +object(Uri\WhatWg\Url)#%d (%d) { + ["scheme"]=> + string(3) "foo" + ["username"]=> + NULL + ["password"]=> + NULL + ["host"]=> + NULL + ["port"]=> + NULL + ["path"]=> + string(4) "abc " + ["query"]=> + NULL + ["fragment"]=> + NULL +} +array(1) { + [0]=> + object(Uri\WhatWg\UrlValidationError)#%d (%d) { + ["context"]=> + string(1) " " + ["type"]=> + enum(Uri\WhatWg\UrlValidationErrorType::InvalidUrlUnit) + ["failure"]=> + bool(false) + } +} diff --git a/ext/uri/tests/whatwg/builder/query_success_empty_string.phpt b/ext/uri/tests/whatwg/builder/query_success_empty_string.phpt index 01cdb62a11cb..a2d4a8a74484 100644 --- a/ext/uri/tests/whatwg/builder/query_success_empty_string.phpt +++ b/ext/uri/tests/whatwg/builder/query_success_empty_string.phpt @@ -16,7 +16,7 @@ var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString()))); ?> --EXPECTF-- -string(21) "https://example.com/?" +string(20) "https://example.com/" object(Uri\WhatWg\Url)#%d (%d) { ["scheme"]=> string(5) "https" @@ -31,7 +31,7 @@ object(Uri\WhatWg\Url)#%d (%d) { ["path"]=> string(1) "/" ["query"]=> - string(0) "" + NULL ["fragment"]=> NULL } diff --git a/ext/uri/tests/whatwg/builder/query_success_empty_with_base.phpt b/ext/uri/tests/whatwg/builder/query_success_empty_with_base.phpt index e2e41cb7a127..fa9a374bf022 100644 --- a/ext/uri/tests/whatwg/builder/query_success_empty_with_base.phpt +++ b/ext/uri/tests/whatwg/builder/query_success_empty_with_base.phpt @@ -1,5 +1,5 @@ --TEST-- -Test Uri\WhatWg\UrlBuilder::build() - success - replaces the base query with an empty query +Test Uri\WhatWg\UrlBuilder::build() - success - clears the base query with an empty string --FILE-- equals(new Uri\WhatWg\Url($url->toAsciiString()))); ?> --EXPECT-- -string(22) "https://example.com/a?" +string(21) "https://example.com/a" bool(true) diff --git a/ext/uri/tests/whatwg/builder/query_success_question_mark_only.phpt b/ext/uri/tests/whatwg/builder/query_success_question_mark_only.phpt new file mode 100644 index 000000000000..e0217d4ce79a --- /dev/null +++ b/ext/uri/tests/whatwg/builder/query_success_question_mark_only.phpt @@ -0,0 +1,37 @@ +--TEST-- +Test Uri\WhatWg\UrlBuilder::setQuery() - success - question mark only +--FILE-- +setScheme("https"); +$builder->setHost("example.com"); +$builder->setQuery("?"); +$url = $builder->build(); + +var_dump($url->toAsciiString()); +var_dump($url); +var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString()))); + +?> +--EXPECTF-- +string(21) "https://example.com/?" +object(Uri\WhatWg\Url)#%d (%d) { + ["scheme"]=> + string(5) "https" + ["username"]=> + NULL + ["password"]=> + NULL + ["host"]=> + string(11) "example.com" + ["port"]=> + NULL + ["path"]=> + string(1) "/" + ["query"]=> + string(0) "" + ["fragment"]=> + NULL +} +bool(true) diff --git a/ext/uri/uri_parser_whatwg.c b/ext/uri/uri_parser_whatwg.c index 47b12661bd2d..079bbb84e7bb 100644 --- a/ext/uri/uri_parser_whatwg.c +++ b/ext/uri/uri_parser_whatwg.c @@ -1136,14 +1136,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser } if (Z_TYPE_P(query) == IS_STRING) { - lxb_url_query_set_null(lexbor_url); - zend_result result; - if (Z_STRLEN_P(query) == 0) { - lexbor_str_init(&lexbor_url->query, lexbor_url->mraw, 1); - result = SUCCESS; - } else { - result = php_uri_parser_whatwg_query_write(lexbor_url, query, NULL); - } + zend_result result = php_uri_parser_whatwg_query_write(lexbor_url, query, NULL); php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors)); if (result == FAILURE) { goto failure; @@ -1151,14 +1144,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser } if (Z_TYPE_P(fragment) == IS_STRING) { - lxb_url_fragment_set_null(lexbor_url); - zend_result result; - if (Z_STRLEN_P(fragment) == 0) { - lexbor_str_init(&lexbor_url->fragment, lexbor_url->mraw, 1); - result = SUCCESS; - } else { - result = php_uri_parser_whatwg_fragment_write(lexbor_url, fragment, NULL); - } + zend_result result = php_uri_parser_whatwg_fragment_write(lexbor_url, fragment, NULL); php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors)); if (result == FAILURE) { goto failure; @@ -1206,9 +1192,9 @@ ZEND_ATTRIBUTE_NONNULL static zend_result php_uri_parser_whatwg_build_path( /* The opaque path parser needs the following delimiter to encode the * immediately preceding space, while still reporting all space errors. */ - if (Z_TYPE_P(query) != IS_NULL) { + if (Z_TYPE_P(query) == IS_STRING && Z_STRLEN_P(query) > 0) { smart_str_appendc(&opaque_path, '?'); - } else if (Z_TYPE_P(fragment) != IS_NULL) { + } else if (Z_TYPE_P(fragment) == IS_STRING && Z_STRLEN_P(fragment) > 0) { smart_str_appendc(&opaque_path, '#'); } @@ -1324,30 +1310,16 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser_wh goto failure; } - if (Z_TYPE_P(query) == IS_STRING && Z_STRLEN_P(query) == 0) { - /* The URL API setter treats an empty string as removal. The builder - * distinguishes an empty component from an absent one. */ - lexbor_str_destroy(&lexbor_url->query, lexbor_url->mraw, false); - lexbor_str_init(&lexbor_url->query, lexbor_url->mraw, 1); - } else { - result = php_uri_parser_whatwg_query_write(lexbor_url, query, NULL); - php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors)); - if (result == FAILURE) { - goto failure; - } + result = php_uri_parser_whatwg_query_write(lexbor_url, query, NULL); + php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors)); + if (result == FAILURE) { + goto failure; } - if (Z_TYPE_P(fragment) == IS_STRING && Z_STRLEN_P(fragment) == 0) { - /* The URL API setter treats an empty string as removal. The builder - * distinguishes an empty component from an absent one. */ - lexbor_str_destroy(&lexbor_url->fragment, lexbor_url->mraw, false); - lexbor_str_init(&lexbor_url->fragment, lexbor_url->mraw, 1); - } else { - result = php_uri_parser_whatwg_fragment_write(lexbor_url, fragment, NULL); - php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors)); - if (result == FAILURE) { - goto failure; - } + result = php_uri_parser_whatwg_fragment_write(lexbor_url, fragment, NULL); + php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors)); + if (result == FAILURE) { + goto failure; } if (php_uri_pass_errors_by_ref_and_free(soft_errors_zv, Z_ARRVAL(errors)) == FAILURE) {