Skip to content

Commit 4a39e3f

Browse files
committed
Address review comments
1 parent af73d4a commit 4a39e3f

4 files changed

Lines changed: 110 additions & 29 deletions

File tree

ext/uri/php_uri.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
+----------------------------------------------------------------------+
1313
*/
1414

15-
#include "zend_smart_str.h"
1615
#ifdef HAVE_CONFIG_H
1716
# include <config.h>
1817
#endif
@@ -1258,6 +1257,8 @@ PHP_METHOD(Uri_Rfc3986_UriBuilder, build)
12581257

12591258
PHP_METHOD(Uri_WhatWg_UrlBuilder, reset)
12601259
{
1260+
ZEND_PARSE_PARAMETERS_NONE();
1261+
12611262
zval_ptr_dtor(Z_WHATWG_URL_PROP_SCHEME_P(ZEND_THIS));
12621263
ZVAL_EMPTY_STRING(Z_WHATWG_URL_PROP_SCHEME_P(ZEND_THIS));
12631264
convert_to_null(Z_WHATWG_URL_PROP_USERNAME_P(ZEND_THIS));
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
Test Uri\WhatWg\UrlBuilder::setScheme() - success - contains leading and trailing C0 control and space characters
3+
--FILE--
4+
<?php
5+
6+
$builder = new Uri\WhatWg\UrlBuilder();
7+
8+
try {
9+
$builder->setScheme(" \x01https \x02");
10+
} catch (Throwable $e) {
11+
echo $e::class, ": ", $e->getMessage(), PHP_EOL;
12+
}
13+
14+
?>
15+
--EXPECT--
16+
Uri\WhatWg\InvalidUrlException: The specified scheme is malformed
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
Test Uri\WhatWg\UrlBuilder::setScheme() - success - contains ignorable newline and tab characters
3+
--FILE--
4+
<?php
5+
6+
$builder = new Uri\WhatWg\UrlBuilder();
7+
$builder->setScheme("ht\ttp\ns");
8+
$builder->setHost("example.com");
9+
$url = $builder->build();
10+
11+
var_dump($url->toAsciiString());
12+
var_dump($url);
13+
var_dump($url->equals(new Uri\WhatWg\Url($url->toAsciiString())));
14+
15+
?>
16+
--EXPECTF--
17+
string(20) "https://example.com/"
18+
object(Uri\WhatWg\Url)#%d (%d) {
19+
["scheme"]=>
20+
string(5) "https"
21+
["username"]=>
22+
NULL
23+
["password"]=>
24+
NULL
25+
["host"]=>
26+
string(11) "example.com"
27+
["port"]=>
28+
NULL
29+
["path"]=>
30+
string(1) "/"
31+
["query"]=>
32+
NULL
33+
["fragment"]=>
34+
NULL
35+
}
36+
bool(true)

ext/uri/uri_parser_whatwg.c

Lines changed: 56 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -698,36 +698,46 @@ ZEND_ATTRIBUTE_NONNULL zend_result php_uri_parser_whatwg_validate_none(const zen
698698
return SUCCESS;
699699
}
700700

701-
static zend_always_inline bool php_uri_parser_whatwg_is_alpha(unsigned char c)
701+
static zend_always_inline bool php_uri_whatwg_is_ascii_tab_or_newline(const unsigned char c)
702+
{
703+
return c == 0x09 || c == 0x0A || c == 0x0D; /* \t \n \r */
704+
}
705+
706+
static zend_always_inline bool php_uri_parser_whatwg_is_alpha(const unsigned char c)
702707
{
703708
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');
704709
}
705710

706-
static zend_always_inline bool php_uri_parser_whatwg_is_digit(unsigned char c)
711+
static zend_always_inline bool php_uri_parser_whatwg_is_digit(const unsigned char c)
707712
{
708713
return c >= '0' && c <= '9';
709714
}
710715

716+
static zend_always_inline unsigned char php_uri_ascii_to_lowercase(const unsigned char c)
717+
{
718+
return c >= 'A' && c <= 'Z' ? (unsigned char) (c + ('a' - 'A')) : c;
719+
}
720+
711721
ZEND_ATTRIBUTE_NONNULL zend_result php_uri_parser_whatwg_validate_scheme(const zend_string *scheme)
712722
{
713723
const char *p = ZSTR_VAL(scheme);
714-
const size_t len = ZSTR_LEN(scheme);
724+
const char *end = p + ZSTR_LEN(scheme);
715725
bool seen_first = false;
716726

717-
for (size_t i = 0; i < len; i++) {
718-
const unsigned char c = (unsigned char)p[i];
727+
for (const char *c = p; c < end; c++) {
728+
const unsigned char uc = (unsigned char) *c;
719729

720-
if (c == '\t' || c == '\n' || c == '\r') {
730+
if (php_uri_whatwg_is_ascii_tab_or_newline(uc)) {
721731
continue;
722732
}
723733

724734
if (!seen_first) {
725-
if (!php_uri_parser_whatwg_is_alpha(c)) {
735+
if (!php_uri_parser_whatwg_is_alpha(uc)) {
726736
return php_uri_parser_whatwg_validate_component_result(false, "scheme");
727737
}
728738
seen_first = true;
729739
} else {
730-
if (!php_uri_parser_whatwg_is_alpha(c) && !php_uri_parser_whatwg_is_digit(c) && c != '+' && c != '-' && c != '.') {
740+
if (!php_uri_parser_whatwg_is_alpha(uc) && !php_uri_parser_whatwg_is_digit(uc) && uc != '+' && uc != '-' && uc != '.') {
731741
return php_uri_parser_whatwg_validate_component_result(false, "scheme");
732742
}
733743
}
@@ -747,26 +757,45 @@ ZEND_ATTRIBUTE_NONNULL zend_result php_uri_parser_whatwg_validate_port(const zen
747757
return php_uri_parser_whatwg_validate_component_result(well_formed, "port");
748758
}
749759

750-
ZEND_ATTRIBUTE_NONNULL static bool php_uri_parser_whatwg_is_special_scheme(const zval *scheme)
760+
ZEND_ATTRIBUTE_NONNULL static bool php_uri_parser_whatwg_is_special_scheme(const zend_string *scheme)
751761
{
752-
ZEND_ASSERT(Z_TYPE_P(scheme) == IS_STRING);
762+
const char *p = ZSTR_VAL(scheme);
763+
const char *end = p + ZSTR_LEN(scheme);
753764

754-
const zend_string *str = Z_STR_P(scheme);
765+
/*
766+
* Create a normalized buffer from the rest of the characters, leaving out tab and newline characters.
767+
* The longest special scheme "https" is 5 characters, therefore 6 bytes is enough.
768+
*/
769+
char buf[6];
770+
size_t buf_len = 0;
755771

756-
switch (ZSTR_LEN(str)) {
757-
case 2:
758-
return zend_string_equals_literal_ci(str, "ws");
759-
case 3:
760-
return zend_string_equals_literal_ci(str, "ftp")
761-
|| zend_string_equals_literal_ci(str, "wss");
762-
case 4:
763-
return zend_string_equals_literal_ci(str, "http")
764-
|| zend_string_equals_literal_ci(str, "file");
765-
case 5:
766-
return zend_string_equals_literal_ci(str, "https");
767-
default:
768-
return false;
769-
}
772+
for (const char *c = p; c < end; c++) {
773+
const unsigned char uc = (unsigned char) *c;
774+
775+
if (php_uri_whatwg_is_ascii_tab_or_newline(uc)) {
776+
continue;
777+
}
778+
779+
if (buf_len == sizeof(buf) - 1) {
780+
/* Longer than any special scheme */
781+
return false;
782+
}
783+
784+
buf[buf_len++] = (char) php_uri_ascii_to_lowercase(uc);
785+
}
786+
787+
switch (buf_len) {
788+
case 2:
789+
return memcmp(buf, "ws", 2) == 0;
790+
case 3:
791+
return memcmp(buf, "ftp", 3) == 0 || memcmp(buf, "wss", 3) == 0;
792+
case 4:
793+
return memcmp(buf, "http", 4) == 0 || memcmp(buf, "file", 4) == 0;
794+
case 5:
795+
return memcmp(buf, "https", 5) == 0;
796+
default:
797+
return false;
798+
}
770799
}
771800

772801
ZEND_ATTRIBUTE_NONNULL static void php_uri_parser_whatwg_build_errors(zval *errors)
@@ -779,7 +808,6 @@ ZEND_ATTRIBUTE_NONNULL static void php_uri_parser_whatwg_build_errors(zval *erro
779808

780809
if (Z_TYPE_P(errors) != IS_ARRAY) {
781810
zval_ptr_dtor(errors);
782-
ZVAL_EMPTY_ARRAY(errors);
783811
array_init_size(errors, log_len);
784812
}
785813

@@ -793,7 +821,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser_wh
793821
) {
794822
lxb_url_parser_clean(&lexbor_parser);
795823

796-
lxb_url_t *lexbor_url = lexbor_mraw_calloc(lexbor_parser.mraw, sizeof(lxb_url_t));
824+
lxb_url_t *lexbor_url = lexbor_mraw_calloc(lexbor_parser.mraw, sizeof(*lexbor_url));
797825
if (lexbor_url == NULL) {
798826
zend_throw_exception(php_uri_ce_whatwg_invalid_url_exception, "Memory allocation error", 0);
799827
return NULL;
@@ -805,7 +833,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(2, 3, 4, 5, 6, 7, 8, 9) lxb_url_t *php_uri_parser_wh
805833
* The URL is initialized as LXB_URL_SCHEMEL_TYPE__UNDEF but this would prevent the scheme to be updated
806834
* in case of non-special schemes due to https://github.com/php/php-src/blob/27d7b799c0a13578ee0506b428b8ddc209ffb010/ext/lexbor/lexbor/url/url.c#L1402
807835
*/
808-
if (!php_uri_parser_whatwg_is_special_scheme(scheme)) {
836+
if (!php_uri_parser_whatwg_is_special_scheme(Z_STR_P(scheme))) {
809837
lexbor_url->scheme.type = LXB_URL_SCHEMEL_TYPE__UNKNOWN;
810838
}
811839

0 commit comments

Comments
 (0)