Skip to content

uri: Improve exceptions for Uri\WhatWg\Url #18855

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
wants to merge 1 commit into
base: master
Choose a base branch
from
+45 −44
Open

uri: Improve exceptions for Uri\WhatWg\Url #18855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 37 additions & 36 deletions ext/uri/php_lexbor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,20 @@ static void lexbor_cleanup_parser(void)
* When errors is NULL, the caller is not interested in the additional error information,
* so the function does nothing.
*/
static void fill_errors(zval *errors)
static zend_string *fill_errors(zval *errors)
{
zend_string *result = NULL;

if (errors == NULL) {
return;
return result;
}

ZEND_ASSERT(Z_ISUNDEF_P(errors));

array_init(errors);

if (lexbor_parser.log == NULL) {
return;
return result;
}

lexbor_plog_entry_t *lxb_error;
Expand Down Expand Up @@ -223,32 +225,14 @@ static void fill_errors(zval *errors)

zend_update_property(uri_whatwg_url_validation_error_ce, Z_OBJ(error), ZEND_STRL("failure"), &failure);

if (Z_TYPE(failure) == IS_TRUE) {
result = error_str;
}

add_next_index_zval(errors, &error);
}
}

static void throw_invalid_url_exception(zval *errors)
{
ZEND_ASSERT(errors != NULL && Z_TYPE_P(errors) == IS_ARRAY);

zval exception;

object_init_ex(&exception, uri_whatwg_invalid_url_exception_ce);

zval value;
ZVAL_STRING(&value, "URL parsing failed");
zend_update_property_ex(uri_whatwg_invalid_url_exception_ce, Z_OBJ(exception), ZSTR_KNOWN(ZEND_STR_MESSAGE), &value);
zval_ptr_dtor_str(&value);

zend_update_property(uri_whatwg_invalid_url_exception_ce, Z_OBJ(exception), ZEND_STRL("errors"), errors);

zend_throw_exception_object(&exception);
}

static void throw_invalid_url_exception_during_write(zval *errors)
Copy link
Member

Choose a reason for hiding this comment

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

Why did you get rid of this function? It seems useful 🤔 especially now that 3 lines are copy-pasted 8 times

Copy link
Member Author

Choose a reason for hiding this comment

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

When using a different message for each component I felt that just copying the code was easier than trying to pass along the error message correctly to the helper function (especially when needing additional component-specific placeholders, I initially included the broken value in the message itself).

{
fill_errors(errors);
throw_invalid_url_exception(errors);
return result;
}

static lxb_status_t lexbor_serialize_callback(const lxb_char_t *data, size_t length, void *ctx)
Expand Down Expand Up @@ -281,7 +265,9 @@ static zend_result lexbor_write_scheme(struct uri_internal_t *internal_uri, zval
zval_string_or_null_to_lexbor_str(value, &str);

if (lxb_url_api_protocol_set(lexbor_uri, &lexbor_parser, str.data, str.length) != LXB_STATUS_OK) {
throw_invalid_url_exception_during_write(errors);
zend_string *reason = fill_errors(errors);
zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified scheme is malformed%s%s%s", reason ? " (" : "", reason ? ZSTR_VAL(reason) : "", reason ? ")" : "");
zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors);

return FAILURE;
}
Expand Down Expand Up @@ -310,7 +296,9 @@ static zend_result lexbor_write_username(uri_internal_t *internal_uri, zval *val
zval_string_or_null_to_lexbor_str(value, &str);

if (lxb_url_api_username_set(lexbor_uri, str.data, str.length) != LXB_STATUS_OK) {
throw_invalid_url_exception_during_write(errors);
zend_string *reason = fill_errors(errors);
zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified username is malformed%s%s%s", reason ? " (" : "", reason ? ZSTR_VAL(reason) : "", reason ? ")" : "");
zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors);

return FAILURE;
}
Expand Down Expand Up @@ -339,7 +327,9 @@ static zend_result lexbor_write_password(struct uri_internal_t *internal_uri, zv
zval_string_or_null_to_lexbor_str(value, &str);

if (lxb_url_api_password_set(lexbor_uri, str.data, str.length) != LXB_STATUS_OK) {
throw_invalid_url_exception_during_write(errors);
zend_string *reason = fill_errors(errors);
zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified password is malformed%s%s%s", reason ? " (" : "", reason ? ZSTR_VAL(reason) : "", reason ? ")" : "");
zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors);

return FAILURE;
}
Expand Down Expand Up @@ -411,7 +401,9 @@ static zend_result lexbor_write_host(struct uri_internal_t *internal_uri, zval *
zval_string_or_null_to_lexbor_str(value, &str);

if (lxb_url_api_hostname_set(lexbor_uri, &lexbor_parser, str.data, str.length) != LXB_STATUS_OK) {
throw_invalid_url_exception_during_write(errors);
zend_string *reason = fill_errors(errors);
zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified host is malformed%s%s%s", reason ? " (" : "", reason ? ZSTR_VAL(reason) : "", reason ? ")" : "");
zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors);

return FAILURE;
}
Expand Down Expand Up @@ -440,7 +432,9 @@ static zend_result lexbor_write_port(struct uri_internal_t *internal_uri, zval *
zval_long_or_null_to_lexbor_str(value, &str);

if (lxb_url_api_port_set(lexbor_uri, &lexbor_parser, str.data, str.length) != LXB_STATUS_OK) {
throw_invalid_url_exception_during_write(errors);
zend_string *reason = fill_errors(errors);
zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified port is malformed%s%s%s", reason ? " (" : "", reason ? ZSTR_VAL(reason) : "", reason ? ")" : "");
zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors);

return FAILURE;
}
Expand Down Expand Up @@ -469,7 +463,9 @@ static zend_result lexbor_write_path(struct uri_internal_t *internal_uri, zval *
zval_string_or_null_to_lexbor_str(value, &str);

if (lxb_url_api_pathname_set(lexbor_uri, &lexbor_parser, str.data, str.length) != LXB_STATUS_OK) {
throw_invalid_url_exception_during_write(errors);
zend_string *reason = fill_errors(errors);
zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified path is malformed%s%s%s", reason ? " (" : "", reason ? ZSTR_VAL(reason) : "", reason ? ")" : "");
zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors);

return FAILURE;
}
Expand Down Expand Up @@ -498,7 +494,9 @@ static zend_result lexbor_write_query(struct uri_internal_t *internal_uri, zval
zval_string_or_null_to_lexbor_str(value, &str);

if (lxb_url_api_search_set(lexbor_uri, &lexbor_parser, str.data, str.length) != LXB_STATUS_OK) {
throw_invalid_url_exception_during_write(errors);
zend_string *reason = fill_errors(errors);
zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified query string is malformed%s%s%s", reason ? " (" : "", reason ? ZSTR_VAL(reason) : "", reason ? ")" : "");
zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors);

return FAILURE;
}
Expand Down Expand Up @@ -527,7 +525,9 @@ static zend_result lexbor_write_fragment(struct uri_internal_t *internal_uri, zv
zval_string_or_null_to_lexbor_str(value, &str);

if (lxb_url_api_hash_set(lexbor_uri, &lexbor_parser, str.data, str.length) != LXB_STATUS_OK) {
throw_invalid_url_exception_during_write(errors);
zend_string *reason = fill_errors(errors);
zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified fragment is malformed%s%s%s", reason ? " (" : "", reason ? ZSTR_VAL(reason) : "", reason ? ")" : "");
zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors);

return FAILURE;
}
Expand Down Expand Up @@ -569,10 +569,11 @@ lxb_url_t *lexbor_parse_uri_ex(const zend_string *uri_str, const lxb_url_t *lexb
lexbor_cleanup_parser();

lxb_url_t *url = lxb_url_parse(&lexbor_parser, lexbor_base_url, (unsigned char *) ZSTR_VAL(uri_str), ZSTR_LEN(uri_str));
fill_errors(errors);
zend_string *reason = fill_errors(errors);

if (url == NULL && !silent) {
throw_invalid_url_exception(errors);
zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified URI is malformed%s%s%s", reason ? " (" : "", reason ? ZSTR_VAL(reason) : "", reason ? ")" : "");
zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors);
}

return url;
Expand Down
4 changes: 2 additions & 2 deletions ext/uri/tests/004.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ var_dump(Uri\WhatWg\Url::parse("192.168/contact.html", null));
var_dump(Uri\WhatWg\Url::parse("http://RuPaul's Drag Race All Stars 7 Winners Cast on This Season's", null));

?>
--EXPECTF--
URL parsing failed
--EXPECT--
The specified URI is malformed (MissingSchemeNonRelativeUrl)
NULL
NULL
NULL
2 changes: 1 addition & 1 deletion ext/uri/tests/007.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var_dump($failures);

?>
--EXPECTF--
URL parsing failed
The specified URI is malformed (PortInvalid)
array(%d) {
[0]=>
object(Uri\WhatWg\UrlValidationError)#%d (%d) {
Expand Down
4 changes: 2 additions & 2 deletions ext/uri/tests/023.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ try {
--EXPECT--
string(5) "https"
string(4) "http"
URL parsing failed
URL parsing failed
The specified scheme is malformed
The specified scheme is malformed
4 changes: 2 additions & 2 deletions ext/uri/tests/026.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ string(8) "test.com"
string(8) "test.com"
string(8) "test.com"
NULL
URL parsing failed
The specified host is malformed (DomainInvalidCodePoint)
object(Uri\WhatWg\Url)#%d (%d) {
["scheme"]=>
string(5) "https"
Expand All @@ -62,6 +62,6 @@ object(Uri\WhatWg\Url)#%d (%d) {
["fragment"]=>
NULL
}
URL parsing failed
The specified host is malformed (HostMissing)
string(7) "foo.com"
string(8) "test.com"
2 changes: 1 addition & 1 deletion ext/uri/tests/051.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ var_dump($softErrors);

?>
--EXPECTF--
URL parsing failed
The specified URI is malformed (Ipv4TooManyParts)
string(23) "https://example.com/foo"
array(%d) {
[0]=>
Expand Down