Skip to content

ext/uri: Refactors builder error handling #23834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
NickSdot wants to merge 2 commits into
base: PHP-8.6
Choose a base branch
from
Open

ext/uri: Refactors builder error handling #23834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
24cb176
review: hash tables
NickSdot Sep 22, 2026
bfbdc89
review: dead error state
NickSdot Sep 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions ext/uri/php_uri_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,20 @@ static zend_string *get_known_string_by_property_name(php_uri_property_name prop
}
}

zend_result php_uri_pass_errors_by_ref_and_free(zval *errors_zv, zval *errors)
ZEND_ATTRIBUTE_NONNULL_ARGS(2) zend_result php_uri_pass_errors_by_ref_and_free(zval *errors_zv, HashTable *errors)
{
ZEND_ASSERT(Z_TYPE_P(errors) == IS_UNDEF || Z_TYPE_P(errors) == IS_ARRAY);
zval errors_tmp;
ZVAL_ARR(&errors_tmp, errors);

/* There was no error during parsing */
if (Z_ISUNDEF_P(errors)) {
return SUCCESS;
}

/* The errors parameter is an array, but the pass-by ref argument stored by
* errors_zv was not passed - the URI implementation either doesn't support
* returning additional error information, or the caller is not interested in it */
/* The pass-by ref argument stored by errors_zv was not passed - the URI
* implementation either doesn't support returning additional error information,
* or the caller is not interested in it */
if (errors_zv == NULL) {
zval_ptr_dtor(errors);
zval_ptr_dtor(&errors_tmp);

@kocsismate kocsismate Oct 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what about using zend_array_release(errors); instead of the temporary variable? And I think ZEND_TRY_ASSIGN_REF_ARR() could also be used below.

return SUCCESS;
}

ZEND_TRY_ASSIGN_REF_TMP(errors_zv, errors);
ZEND_TRY_ASSIGN_REF_TMP(errors_zv, &errors_tmp);
if (EG(exception)) {
return FAILURE;
}
Expand Down
2 changes: 1 addition & 1 deletion ext/uri/php_uri_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ static inline const php_uri_property_handler *php_uri_parser_property_handler_by
}
}

zend_result php_uri_pass_errors_by_ref_and_free(zval *errors_zv, zval *errors);
ZEND_ATTRIBUTE_NONNULL_ARGS(2) zend_result php_uri_pass_errors_by_ref_and_free(zval *errors_zv, HashTable *errors);
void php_uri_property_read_helper(INTERNAL_FUNCTION_PARAMETERS, php_uri_property_name property_name, php_uri_component_read_mode component_read_mode);
void php_uri_property_write_str_helper(INTERNAL_FUNCTION_PARAMETERS, php_uri_property_name property_name);
void php_uri_property_write_str_or_null_helper(INTERNAL_FUNCTION_PARAMETERS, php_uri_property_name property_name);
Expand Down
62 changes: 28 additions & 34 deletions ext/uri/uri_parser_whatwg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,26 +977,19 @@ ZEND_ATTRIBUTE_NONNULL static lxb_url_scheme_type_t php_uri_parser_whatwg_get_sp
return LXB_URL_SCHEMEL_TYPE__UNDEF;
}

ZEND_ATTRIBUTE_NONNULL static const char *php_uri_parser_whatwg_build_errors(zval *errors)
ZEND_ATTRIBUTE_NONNULL static const char *php_uri_parser_whatwg_build_errors(HashTable *errors)
{
size_t log_len;

if (lexbor_parser.log == NULL || (log_len = lexbor_plog_length(lexbor_parser.log)) == 0) {
if (lexbor_parser.log == NULL || lexbor_plog_length(lexbor_parser.log) == 0) {
return NULL;
}

if (Z_TYPE_P(errors) != IS_ARRAY) {
zval_ptr_dtor(errors);
array_init_size(errors, log_len);
}

return fill_errors_inner(Z_ARRVAL_P(errors));
return fill_errors_inner(errors);
}

ZEND_ATTRIBUTE_NONNULL static void php_uri_parser_whatwg_build_errors_into_exception(zval *errors)
ZEND_ATTRIBUTE_NONNULL static void php_uri_parser_whatwg_build_errors_into_exception(HashTable *errors)
{
/* Include errors from earlier components in the exception raised by a later component. */
if (zend_hash_num_elements(Z_ARRVAL_P(errors)) > 0 && EG(exception)
if (zend_hash_num_elements(errors) > 0 && EG(exception)
&& instanceof_function(EG(exception)->ce, php_uri_ce_whatwg_invalid_url_exception)) {
zval rv;
zval *exception_errors = zend_read_property(php_uri_ce_whatwg_invalid_url_exception,
Expand All @@ -1006,15 +999,16 @@ ZEND_ATTRIBUTE_NONNULL static void php_uri_parser_whatwg_build_errors_into_excep
zval *error;
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(exception_errors), error) {
Z_TRY_ADDREF_P(error);
zend_hash_next_index_insert(Z_ARRVAL_P(errors), error);
zend_hash_next_index_insert(errors, error);
} ZEND_HASH_FOREACH_END();

zval_ptr_dtor(exception_errors);
ZVAL_COPY(exception_errors, errors);
GC_ADDREF(errors);
ZVAL_ARR(exception_errors, errors);
}
}

ZEND_ATTRIBUTE_NONNULL static void php_uri_parser_whatwg_build_errors_and_throw(const lxb_status_t status, const char *component, zval *errors)
ZEND_ATTRIBUTE_NONNULL static void php_uri_parser_whatwg_build_errors_and_throw(const lxb_status_t status, const char *component, HashTable *errors)
{
if (status != LXB_STATUS_OK) {
throw_invalid_url_exception_during_write(NULL, component);
Expand Down Expand Up @@ -1131,7 +1125,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser
lxb_url_parser_clean(&lexbor_parser);
status = lxb_url_parse_basic(&lexbor_parser, lexbor_url, lexbor_base_url,
(const lxb_char_t *) ZSTR_VAL(input), ZSTR_LEN(input), state, LXB_ENCODING_UTF_8);
php_uri_parser_whatwg_build_errors_and_throw(status, "path", &errors);
php_uri_parser_whatwg_build_errors_and_throw(status, "path", Z_ARRVAL(errors));
zend_string_release(input);
if (status != LXB_STATUS_OK) {
goto failure;
Expand All @@ -1148,7 +1142,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser
(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", &errors);
php_uri_parser_whatwg_build_errors_and_throw(status, "query", Z_ARRVAL(errors));
if (status != LXB_STATUS_OK) {
goto failure;
}
Expand All @@ -1161,29 +1155,29 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser
(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", &errors);
php_uri_parser_whatwg_build_errors_and_throw(status, "fragment", Z_ARRVAL(errors));
if (status != LXB_STATUS_OK) {
goto failure;
}
}

if (php_uri_pass_errors_by_ref_and_free(soft_errors_zv, &errors) == FAILURE) {
/* The errors zval was already consumed; goto failure would destroy it again. */
if (php_uri_pass_errors_by_ref_and_free(soft_errors_zv, Z_ARRVAL(errors)) == FAILURE) {
/* The errors array was already consumed; goto failure would destroy it again. */
lxb_url_destroy(lexbor_url);
return NULL;
}

return lexbor_url;

failure:
php_uri_parser_whatwg_build_errors_into_exception(&errors);
php_uri_parser_whatwg_build_errors_into_exception(Z_ARRVAL(errors));
zval_ptr_dtor(&errors);
lxb_url_destroy(lexbor_url);
return NULL;
}

ZEND_ATTRIBUTE_NONNULL static zend_result php_uri_parser_whatwg_build_path(
lxb_url_t *lexbor_url, const zval *path, const zval *query, const zval *fragment, zval *errors
lxb_url_t *lexbor_url, const zval *path, const zval *query, const zval *fragment, HashTable *errors
) {
zend_result result;
const char *path_start = Z_STRVAL_P(path);
Expand Down Expand Up @@ -1265,7 +1259,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser_wh
array_init(&errors);

zend_result result = php_uri_parser_whatwg_scheme_write(lexbor_url, scheme, NULL);
php_uri_parser_whatwg_build_errors(&errors);
php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors));
if (result == FAILURE) {
goto failure;
}
Expand All @@ -1274,7 +1268,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser_wh
* Otherwise, preserve the absent host so the path can be opaque. */
if (Z_TYPE_P(host) == IS_STRING || lxb_url_is_special(lexbor_url)) {
result = php_uri_parser_whatwg_host_write(lexbor_url, host, NULL);
php_uri_parser_whatwg_build_errors(&errors);
php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors));
if (result == FAILURE) {
goto failure;
}
Expand All @@ -1301,27 +1295,27 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser_wh

/* Intentionally writing username after host to avoid error when the username is set but the host is missing */
result = php_uri_parser_whatwg_username_write(lexbor_url, username, NULL);
php_uri_parser_whatwg_build_errors(&errors);
php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors));
if (result == FAILURE) {
goto failure;
}

/* Intentionally writing password after host to avoid error when the password is set but the password is missing */
result = php_uri_parser_whatwg_password_write(lexbor_url, password, NULL);
php_uri_parser_whatwg_build_errors(&errors);
php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors));
if (result == FAILURE) {
goto failure;
}

/* Intentionally writing port after host to avoid error when the port is set but the host is missing */
result = php_uri_parser_whatwg_port_write(lexbor_url, port, NULL);
php_uri_parser_whatwg_build_errors(&errors);
php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors));
if (result == FAILURE) {
goto failure;
}

result = php_uri_parser_whatwg_build_path(lexbor_url, path, query, fragment, &errors);
php_uri_parser_whatwg_build_errors(&errors);
result = php_uri_parser_whatwg_build_path(lexbor_url, path, query, fragment, Z_ARRVAL(errors));
php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors));
if (result == FAILURE) {
goto failure;
}
Expand All @@ -1333,7 +1327,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser_wh
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(&errors);
php_uri_parser_whatwg_build_errors(Z_ARRVAL(errors));
if (result == FAILURE) {
goto failure;
}
Expand All @@ -1346,22 +1340,22 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser_wh
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(&errors);
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, &errors) == FAILURE) {
/* The errors zval was already consumed; goto failure would destroy it again. */
if (php_uri_pass_errors_by_ref_and_free(soft_errors_zv, Z_ARRVAL(errors)) == FAILURE) {
/* The errors array was already consumed; goto failure would destroy it again. */
lxb_url_destroy(lexbor_url);
return NULL;
}

return lexbor_url;
failure:
ZEND_ASSERT(EG(exception));
php_uri_parser_whatwg_build_errors_into_exception(&errors);
php_uri_parser_whatwg_build_errors_into_exception(Z_ARRVAL(errors));
zval_ptr_dtor(&errors);
lxb_url_destroy(lexbor_url);
return NULL;
Expand Down
Loading