Skip to content

Commit f45c5db

Browse files
kocsismatekocsismate
committed
Refactor
1 parent cc9c15a commit f45c5db

8 files changed

Lines changed: 137 additions & 106 deletions

‎ext/uri/php_uri.c‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,26 +1225,22 @@ PHP_METHOD(Uri_Rfc3986_UriBuilder, build)
12251225
const zval *query = Z_RFC3986_URI_PROP_QUERY_P(ZEND_THIS);
12261226
const zval *fragment = Z_RFC3986_URI_PROP_FRAGMENT_P(ZEND_THIS);
12271227

1228-
zend_string *uri_str = php_uri_parser_rfc3986_recompose_from_zval(scheme, userinfo, host, port, path, query, fragment);
1229-
if (uri_str == NULL) {
1230-
RETURN_THROWS();
1231-
}
1232-
1233-
php_uri_parser_rfc3986_uris *base_uri = NULL;
1228+
php_uri_parser_rfc3986_uris *base_uris = NULL;
12341229
if (base_url != NULL) {
1235-
base_uri = Z_URI_OBJECT_P(base_url)->uri;
1230+
base_uris = Z_URI_OBJECT_P(base_url)->uri;
12361231
}
12371232

1238-
php_uri_parser_rfc3986_uris *uri = php_uri_parser_rfc3986_parse_ex(ZSTR_VAL(uri_str), ZSTR_LEN(uri_str), base_uri, false);
1239-
zend_string_release(uri_str);
1240-
if (uri == NULL) {
1233+
php_uri_parser_rfc3986_uris *uriparser_uris = php_uri_parser_rfc3986_build_from_zval(
1234+
base_uris, scheme, userinfo, host, port, path, query, fragment
1235+
);
1236+
if (uriparser_uris == NULL) {
12411237
RETURN_THROWS();
12421238
}
12431239

12441240
object_init_ex(return_value, php_uri_ce_rfc3986_uri);
12451241
php_uri_object *uri_object = Z_URI_OBJECT_P(return_value);
12461242
uri_object->parser = &php_uri_parser_rfc3986;
1247-
uri_object->uri = uri;
1243+
uri_object->uri = uriparser_uris;
12481244
}
12491245

12501246
PHPAPI php_uri_object *php_uri_object_create(zend_class_entry *class_type, const php_uri_parser *parser)

‎ext/uri/tests/rfc3986/builder/basic_error_with_base.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
Test Uri\Rfc3986\UriBuilder basic - error - with base URL
3+
--FILE--
4+
<?php
5+
6+
$builder = new Uri\Rfc3986\UriBuilder()
7+
->setPath("/foo/bar/baz");
8+
9+
try {
10+
$builder->build(new Uri\Rfc3986\Uri("/foo/bar"));
11+
} catch (Throwable $e) {
12+
echo $e::class, ": ", $e->getMessage(), PHP_EOL;
13+
}
14+
15+
?>
16+
--EXPECT--
17+
Uri\InvalidUriException: The specified base URI must be absolute

‎ext/uri/tests/rfc3986/builder/basic_success_with_base.phpt‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
Test Uri\Rfc3986\UriBuilder basic - success - with base URL
3+
--FILE--
4+
<?php
5+
6+
$builder = new Uri\Rfc3986\UriBuilder()
7+
->setPath("/foo/bar/baz");
8+
$uri = $builder->build(new Uri\Rfc3986\Uri("https://example.com"));
9+
10+
var_dump($uri->toRawString());
11+
var_dump($uri);
12+
13+
?>
14+
--EXPECTF--
15+
string(31) "https://example.com/foo/bar/baz"
16+
object(Uri\Rfc3986\Uri)#%d (%d) {
17+
["scheme"]=>
18+
string(5) "https"
19+
["username"]=>
20+
NULL
21+
["password"]=>
22+
NULL
23+
["host"]=>
24+
string(11) "example.com"
25+
["port"]=>
26+
NULL
27+
["path"]=>
28+
string(12) "/foo/bar/baz"
29+
["query"]=>
30+
NULL
31+
["fragment"]=>
32+
NULL
33+
}

‎ext/uri/tests/rfc3986/builder/path_error_missing_leading_slash.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,4 @@ try {
1515

1616
?>
1717
--EXPECT--
18-
Uri\InvalidUriException: A path must be either empty or begin with "/" when the URI contains a host
18+
Uri\InvalidUriException: The specified path is malformed

‎ext/uri/tests/rfc3986/builder/port_error_missing_host.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ try {
1414

1515
?>
1616
--EXPECT--
17-
Uri\InvalidUriException: The URI must contain a host if either the userinfo or the port component is present
17+
Uri\InvalidUriException: Cannot set a port without having a host

‎ext/uri/tests/rfc3986/builder/userinfo_error_missing_host.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ try {
1414

1515
?>
1616
--EXPECT--
17-
Uri\InvalidUriException: The URI must contain a host if either the userinfo or the port component is present
17+
Uri\InvalidUriException: Cannot set a userinfo without having a host

‎ext/uri/uri_parser_rfc3986.c‎

Lines changed: 75 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -537,20 +537,20 @@ static zend_result php_uri_parser_rfc3986_fragment_write(void *uri, const zval *
537537
}
538538
}
539539

540-
static php_uri_parser_rfc3986_uris *uriparser_create_uris(void)
540+
php_uri_parser_rfc3986_uris *uriparser_create_uris(void)
541541
{
542542
php_uri_parser_rfc3986_uris *uriparser_uris = ecalloc(1, sizeof(*uriparser_uris));
543543
uriparser_uris->normalized_uri_initialized = false;
544544

545545
return uriparser_uris;
546546
}
547547

548-
php_uri_parser_rfc3986_uris *php_uri_parser_rfc3986_parse_ex(const char *uri_str, size_t uri_str_len, const php_uri_parser_rfc3986_uris *uriparser_base_urls, bool silent)
548+
php_uri_parser_rfc3986_uris *php_uri_parser_rfc3986_parse_ex(const char *uri_str, const size_t uri_str_len, const php_uri_parser_rfc3986_uris *uriparser_base_urls, const bool silent)
549549
{
550550
UriUriA uri = {0};
551551

552552
/* Parse the URI. */
553-
int result = uriParseSingleUriExMmA(&uri, uri_str, uri_str + uri_str_len, NULL, mm);
553+
const int result = uriParseSingleUriExMmA(&uri, uri_str, uri_str + uri_str_len, NULL, mm);
554554
if (result != URI_SUCCESS) {
555555
if (!silent) {
556556
switch (result) {
@@ -572,7 +572,7 @@ php_uri_parser_rfc3986_uris *php_uri_parser_rfc3986_parse_ex(const char *uri_str
572572

573573
/* Combine the parsed URI with the base URI and store the result in 'tmp',
574574
* since the target and source URLs must be distinct. */
575-
int result = uriAddBaseUriExMmA(&tmp, &uri, &uriparser_base_urls->uri, URI_RESOLVE_STRICTLY, mm);
575+
const int result = uriAddBaseUriExMmA(&tmp, &uri, &uriparser_base_urls->uri, URI_RESOLVE_STRICTLY, mm);
576576
if (result != URI_SUCCESS) {
577577
if (!silent) {
578578
switch (result) {
@@ -727,7 +727,7 @@ ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_host(const zend_stri
727727
return uriIsWellFormedHostRegNameA(p, p + len);
728728
}
729729

730-
ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_port(zend_long port)
730+
ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_port(const zend_long port)
731731
{
732732
zend_string *tmp = zend_long_to_str(port);
733733
const char *p = ZSTR_VAL(tmp);
@@ -765,109 +765,93 @@ ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_fragment(const zend_
765765
return uriIsWellFormedFragmentA(p, p + len);
766766
}
767767

768-
ZEND_ATTRIBUTE_NONNULL zend_string *php_uri_parser_rfc3986_recompose_from_zval(
768+
ZEND_ATTRIBUTE_NONNULL_ARGS(2,3,4,5,6,7,8) php_uri_parser_rfc3986_uris *php_uri_parser_rfc3986_build_from_zval(
769+
const php_uri_parser_rfc3986_uris *uriparser_base_uris,
769770
const zval *scheme, const zval *userinfo, const zval *host, const zval *port,
770771
const zval *path, const zval *query, const zval *fragment
771772
) {
772-
if (Z_TYPE_P(host) == IS_NULL) {
773-
/* The path must not begin with "//" if the URI doesn't contain a host */
774-
if (zend_string_starts_with_literal(Z_STR_P(path), "//")) {
775-
zend_throw_exception(php_uri_ce_invalid_uri_exception, "The path must not begin with \"//\" when the URI doesn't contain a host", 0);
776-
return NULL;
777-
}
773+
php_uri_parser_rfc3986_uris *uriparser_uris = uriparser_create_uris();
778774

779-
/* The userinfo and port components must not be present if the URI doesn't contain a host */
780-
if (Z_TYPE_P(userinfo) != IS_NULL || Z_TYPE_P(port) != IS_NULL) {
781-
zend_throw_exception(php_uri_ce_invalid_uri_exception, "The URI must contain a host if either the userinfo or the port component is present", 0);
782-
return NULL;
783-
}
784-
} else {
785-
/* The path must be either empty or begin with a "/" if the URI contains a host */
786-
if (Z_STRLEN_P(path) > 0 && Z_STRVAL_P(path)[0] != '/') {
787-
zend_throw_exception(php_uri_ce_invalid_uri_exception, "A path must be either empty or begin with \"/\" when the URI contains a host", 0);
788-
return NULL;
789-
}
790-
}
775+
if (Z_STRLEN_P(path) > 0) {
776+
/* The first segment of the path must not contain ":" if the URI doesn't contain a scheme */
777+
if (Z_TYPE_P(scheme) == IS_NULL) {
778+
const char *p = Z_STRVAL_P(path);
779+
while (*p != '\0' && *p != '/') {
780+
if (*p == ':') {
781+
zend_throw_exception(php_uri_ce_invalid_uri_exception, "The path must not begin with \":\" when the URI doesn't contain a scheme", 0);
782+
goto failure;
783+
}
791784

792-
/* The first segment of the path must not contain ":" if the URI doesn't contain a scheme */
793-
if (Z_TYPE_P(scheme) == IS_NULL) {
794-
const char *p = Z_STRVAL_P(path);
795-
while (*p != '\0' && *p != '/') {
796-
if (*p == ':') {
797-
zend_throw_exception(php_uri_ce_invalid_uri_exception, "The path must not begin with \":\" when the URI doesn't contain a scheme", 0);
798-
return NULL;
785+
p++;
799786
}
787+
}
800788

801-
p++;
789+
/* The path must not begin with "//" if the URI doesn't contain a host */
790+
if (Z_TYPE_P(host) == IS_NULL && zend_string_starts_with_literal(Z_STR_P(path), "//")) {
791+
zend_throw_exception(php_uri_ce_invalid_uri_exception, "The path must not begin with \"//\" when the URI doesn't contain a host", 0);
792+
goto failure;
802793
}
803794
}
804795

805-
/* result = "" */
806-
smart_str uri_str = {0};
807-
808-
/*
809-
if defined(scheme) then
810-
append scheme to result;
811-
append ":" to result;
812-
endif;
813-
*/
814-
if (Z_TYPE_P(scheme) != IS_NULL) {
815-
smart_str_append(&uri_str, Z_STR_P(scheme));
816-
smart_str_appendc(&uri_str, ':');
817-
}
818-
819-
/*
820-
if defined(authority) then
821-
append "//" to result;
822-
append authority to result;
823-
endif;
824-
*/
825-
/* Check if authority is defined */
826-
if (Z_TYPE_P(host) != IS_NULL) {
827-
smart_str_appends(&uri_str, "//");
828-
829-
if (Z_TYPE_P(userinfo) != IS_NULL) {
830-
smart_str_append(&uri_str, Z_STR_P(userinfo));
831-
smart_str_appendc(&uri_str, '@');
832-
}
796+
zend_result result = php_uri_parser_rfc3986_scheme_write(uriparser_uris, scheme, NULL);
797+
if (result == FAILURE) {
798+
goto failure;
799+
}
800+
result = php_uri_parser_rfc3986_host_write(uriparser_uris, host, NULL);
801+
if (result == FAILURE) {
802+
goto failure;
803+
}
804+
/* Intentionally writing userinfo after host to avoid error when the userinfo is set but the host is missing */
805+
result = php_uri_parser_rfc3986_userinfo_write(uriparser_uris, userinfo, NULL);
806+
if (result == FAILURE) {
807+
goto failure;
808+
}
809+
/* Intentionally writing userinfo after host to avoid error when the port is set but the host is missing */
810+
result = php_uri_parser_rfc3986_port_write(uriparser_uris, port, NULL);
811+
if (result == FAILURE) {
812+
goto failure;
813+
}
814+
result = php_uri_parser_rfc3986_path_write(uriparser_uris, path, NULL);
815+
if (result == FAILURE) {
816+
goto failure;
817+
}
818+
result = php_uri_parser_rfc3986_query_write(uriparser_uris, query, NULL);
819+
if (result == FAILURE) {
820+
goto failure;
821+
}
822+
result = php_uri_parser_rfc3986_fragment_write(uriparser_uris, fragment, NULL);
823+
if (result == FAILURE) {
824+
goto failure;
825+
}
833826

834-
if (Z_TYPE_P(host) != IS_NULL) {
835-
smart_str_append(&uri_str, Z_STR_P(host));
836-
}
827+
if (uriparser_base_uris != NULL) {
828+
UriUriA tmp = {0};
837829

838-
if (Z_TYPE_P(port) != IS_NULL) {
839-
smart_str_appendc(&uri_str, ':');
840-
smart_str_append_long(&uri_str, Z_LVAL_P(port));
830+
const int base_uri_result = uriAddBaseUriExMmA(&tmp, &uriparser_uris->uri, &uriparser_base_uris->uri, URI_RESOLVE_STRICTLY, mm);
831+
if (base_uri_result != URI_SUCCESS) {
832+
uriFreeUriMembersMmA(&tmp, mm);
833+
switch (base_uri_result) {
834+
case URI_ERROR_ADDBASE_REL_BASE:
835+
zend_throw_exception(php_uri_ce_invalid_uri_exception, "The specified base URI must be absolute", 0);
836+
goto failure;
837+
default:
838+
/* This should be unreachable in practice. */
839+
zend_throw_exception(php_uri_ce_error, "Failed to resolve the specified URI against the base URI", 0);
840+
goto failure;
841+
}
841842
}
842-
}
843843

844-
/* append path to result; */
845-
smart_str_append(&uri_str, Z_STR_P(path));
846-
847-
/*
848-
if defined(query) then
849-
append "?" to result;
850-
append query to result;
851-
endif;
852-
*/
853-
if (Z_TYPE_P(query) != IS_NULL) {
854-
smart_str_appendc(&uri_str, '?');
855-
smart_str_append(&uri_str, Z_STR_P(query));
844+
uriMakeOwnerMmA(&tmp, mm);
845+
uriFreeUriMembersMmA(&uriparser_uris->uri, mm);
846+
uriparser_uris->uri = tmp;
856847
}
857848

858-
/*
859-
if defined(fragment) then
860-
append "#" to result;
861-
append fragment to result;
862-
endif;
863-
*/
864-
if (Z_TYPE_P(fragment) != IS_NULL) {
865-
smart_str_appendc(&uri_str, '#');
866-
smart_str_append(&uri_str, Z_STR_P(fragment));
867-
}
849+
return uriparser_uris;
868850

869-
/* return result; */
870-
return smart_str_extract(&uri_str);
851+
failure:
852+
uriFreeUriMembersMmA(&uriparser_uris->uri, mm);
853+
efree(uriparser_uris);
854+
return NULL;
871855
}
872856

873857
PHPAPI const php_uri_parser php_uri_parser_rfc3986 = {

‎ext/uri/uri_parser_rfc3986.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_path(const zend_stri
3737
ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_query(const zend_string *query);
3838
ZEND_ATTRIBUTE_NONNULL bool php_uri_parser_rfc3986_validate_fragment(const zend_string *fragment);
3939

40-
ZEND_ATTRIBUTE_NONNULL zend_string *php_uri_parser_rfc3986_recompose_from_zval(
40+
ZEND_ATTRIBUTE_NONNULL_ARGS(2,3,4,5,6,7,8) php_uri_parser_rfc3986_uris *php_uri_parser_rfc3986_build_from_zval(
41+
const php_uri_parser_rfc3986_uris *uriparser_base_uris,
4142
const zval *scheme, const zval *userinfo, const zval *host, const zval *port,
4243
const zval *path, const zval *query, const zval *fragment
4344
);

0 commit comments

Comments
 (0)