Skip to content

Improve http_build_query() performance#18401

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.

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 3 commits into
base:master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPGRADING.INTERNALS
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,6 +14,10 @@ PHP 8.5 INTERNALS UPGRADE NOTES
1. Internal API changes
========================

- Core
. PG(arg_separator).input and PG(arg_separator).output are now `zend_string*`
instead of `char*`.

- Zend
. Added zend_safe_assign_to_variable_noref() function to safely assign
a value to a non-reference zval.
Expand DownExpand Up@@ -64,6 +68,7 @@ PHP 8.5 INTERNALS UPGRADE NOTES
- ext/standard
. Added php_url_decode_ex() and php_raw_url_decode_ex() that unlike their
non-ex counterparts do not work in-place.
. Added php_url_encode_to_smart_str() to encode a URL to a smart_str buffer.

========================
4. OpCode changes
Expand Down
2 changes: 1 addition & 1 deletion ext/mbstring/mb_gpc.c
Original file line numberDiff line numberDiff line change
Expand Up@@ -100,7 +100,7 @@ MBSTRING_API SAPI_TREAT_DATA_FUNC(mbstr_treat_data)
casePARSE_POST:
casePARSE_GET:
casePARSE_STRING:
separator= (char*) estrdup(PG(arg_separator).input);
separator= (char*) estrndup(ZSTR_VAL(PG(arg_separator).input), ZSTR_LEN(PG(arg_separator).input));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think doesn't need a char * cast.

break;
casePARSE_COOKIE:
separator=";\0";
Expand Down
2 changes: 1 addition & 1 deletion ext/mbstring/mbstring.c
Original file line numberDiff line numberDiff line change
Expand Up@@ -1537,7 +1537,7 @@ PHP_FUNCTION(mb_parse_str)
encstr = estrndup(encstr, encstr_len);

info.data_type = PARSE_STRING;
info.separator = PG(arg_separator).input;
info.separator = ZSTR_VAL(PG(arg_separator).input);
info.report_errors = true;
info.to_encoding = MBSTRG(current_internal_encoding);
info.from_encodings = MBSTRG(http_input_list);
Expand Down
36 changes: 7 additions & 29 deletions ext/standard/http.c
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,14 +37,7 @@ static void php_url_encode_scalar(zval *scalar, smart_str *form_str,
smart_str_append(form_str, key_prefix);
}
if (index_string) {
zend_string *encoded_key;
if (encoding_type == PHP_QUERY_RFC3986) {
encoded_key = php_raw_url_encode(index_string, index_string_len);
} else {
encoded_key = php_url_encode(index_string, index_string_len);
}
smart_str_append(form_str, encoded_key);
zend_string_free(encoded_key);
php_url_encode_to_smart_str(form_str, index_string, index_string_len, encoding_type == PHP_QUERY_RFC3986);
} else {
/* Numeric key */
if (num_prefix) {
Expand All@@ -59,31 +52,16 @@ static void php_url_encode_scalar(zval *scalar, smart_str *form_str,

try_again:
switch (Z_TYPE_P(scalar)) {
case IS_STRING: {
zend_string *encoded_data;
if (encoding_type == PHP_QUERY_RFC3986) {
encoded_data = php_raw_url_encode(Z_STRVAL_P(scalar), Z_STRLEN_P(scalar));
} else {
encoded_data = php_url_encode(Z_STRVAL_P(scalar), Z_STRLEN_P(scalar));
}
smart_str_append(form_str, encoded_data);
zend_string_free(encoded_data);
case IS_STRING:
php_url_encode_to_smart_str(form_str, Z_STRVAL_P(scalar), Z_STRLEN_P(scalar), encoding_type == PHP_QUERY_RFC3986);
break;
}
case IS_LONG:
smart_str_append_long(form_str, Z_LVAL_P(scalar));
break;
case IS_DOUBLE: {
zend_string *encoded_data;
zend_string *tmp = zend_double_to_str(Z_DVAL_P(scalar));
if (encoding_type == PHP_QUERY_RFC3986) {
encoded_data = php_raw_url_encode(ZSTR_VAL(tmp), ZSTR_LEN(tmp));
} else {
encoded_data = php_url_encode(ZSTR_VAL(tmp), ZSTR_LEN(tmp));
}
smart_str_append(form_str, encoded_data);
php_url_encode_to_smart_str(form_str, ZSTR_VAL(tmp), ZSTR_LEN(tmp), encoding_type == PHP_QUERY_RFC3986);
zend_string_free(tmp);
zend_string_free(encoded_data);
break;
}
case IS_FALSE:
Expand DownExpand Up@@ -124,7 +102,7 @@ PHPAPI void php_url_encode_hash_ex(HashTable *ht, smart_str *formstr,
}

if (!arg_sep) {
arg_sep = zend_ini_str("arg_separator.output", strlen("arg_separator.output"), false);
arg_sep = PG(arg_separator.output);
if (ZSTR_LEN(arg_sep) == 0) {
arg_sep = ZSTR_CHAR('&');
}
Expand DownExpand Up@@ -181,7 +159,7 @@ PHPAPI void php_url_encode_hash_ex(HashTable *ht, smart_str *formstr,
} else {
new_prefix = zend_string_concat2(ZSTR_VAL(encoded_key), ZSTR_LEN(encoded_key), "%5B", strlen("%5B"));
}
zend_string_release_ex(encoded_key, false);
zend_string_efree(encoded_key);
} else { /* is integer index */
char *index_int_as_str;
size_t index_int_as_str_len;
Expand DownExpand Up@@ -210,7 +188,7 @@ PHPAPI void php_url_encode_hash_ex(HashTable *ht, smart_str *formstr,
GC_TRY_PROTECT_RECURSION(ht);
php_url_encode_hash_ex(HASH_OF(zdata), formstr, NULL, 0, new_prefix, (Z_TYPE_P(zdata) == IS_OBJECT ? zdata : NULL), arg_sep, enc_type);
GC_TRY_UNPROTECT_RECURSION(ht);
zend_string_release_ex(new_prefix, false);
zend_string_efree(new_prefix);
} else if (Z_TYPE_P(zdata) == IS_NULL || Z_TYPE_P(zdata) == IS_RESOURCE) {
/* Skip these types */
continue;
Expand Down
36 changes: 24 additions & 12 deletions ext/standard/url.c
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,6 +27,7 @@

#include "url.h"
#include "file.h"
#include "Zend/zend_smart_str.h"

/* {{{ free_url */
PHPAPI void php_url_free(php_url *theurl)
Expand DownExpand Up@@ -449,16 +450,13 @@ static int php_htoi(const char *s)

static const unsigned char hexchars[] = "0123456789ABCDEF";

static zend_always_inline zend_string *php_url_encode_impl(const char *s, size_t len, bool raw) /* {{{ */ {
static zend_always_inline size_t php_url_encode_impl(unsigned char *to, const char *s, size_t len, bool raw) /* {{{ */ {
unsigned char c;
unsigned char *to;
unsigned char const *from, *end;
zend_string *start;
const unsigned char *to_init = to;

from = (unsigned char *)s;
end = (unsigned char *)s + len;
start = zend_string_safe_alloc(3, len, 0, 0);
to = (unsigned char*)ZSTR_VAL(start);

#ifdef __SSE2__
while (from + 16 < end) {
Expand DownExpand Up@@ -537,19 +535,24 @@ static zend_always_inline zend_string *php_url_encode_impl(const char *s, size_t
*to++ = c;
}
}
*to = '\0';

ZEND_ASSERT(!ZSTR_IS_INTERNED(start) && GC_REFCOUNT(start) == 1);
start = zend_string_truncate(start, to - (unsigned char*)ZSTR_VAL(start), 0);

return start;
return to - to_init;
}
/* }}} */

static zend_always_inline zend_string *php_url_encode_helper(char const *s, size_t len, bool raw)
{
zend_string *result = zend_string_safe_alloc(3, len, 0, false);
size_t length = php_url_encode_impl((unsigned char *) ZSTR_VAL(result), s, len, raw);
ZSTR_VAL(result)[length] = '\0';
ZEND_ASSERT(!ZSTR_IS_INTERNED(result) && GC_REFCOUNT(result) == 1);
return zend_string_truncate(result, length, false);
}

/* {{{ php_url_encode */
PHPAPI zend_string *php_url_encode(char const *s, size_t len)
{
return php_url_encode_impl(s, len, 0);
return php_url_encode_helper(s, len, false);
}
/* }}} */

Expand DownExpand Up@@ -616,10 +619,19 @@ PHPAPI size_t php_url_decode(char *str, size_t len)
/* {{{ php_raw_url_encode */
PHPAPI zend_string *php_raw_url_encode(char const *s, size_t len)
{
return php_url_encode_impl(s, len, 1);
return php_url_encode_helper(s, len, true);
}
/* }}} */

PHPAPI void php_url_encode_to_smart_str(smart_str *buf, char const *s, size_t len, bool raw)
{
size_t start_length = smart_str_get_len(buf);
size_t extend = zend_safe_address_guarded(3, len, 0);
char *dest = smart_str_extend(buf, extend);
size_t length = php_url_encode_impl((unsigned char *) dest, s, len, raw);
ZSTR_LEN(buf->s) = start_length + length;
}

/* {{{ URL-encodes string */
PHP_FUNCTION(rawurlencode)
{
Expand Down
1 change: 1 addition & 0 deletions ext/standard/url.h
Original file line numberDiff line numberDiff line change
Expand Up@@ -38,6 +38,7 @@ PHPAPI size_t php_raw_url_decode(char *str, size_t len); /* return value: length
PHPAPI size_t php_raw_url_decode_ex(char *dest, const char *src, size_t src_len);
PHPAPI zend_string *php_url_encode(char const *s, size_t len);
PHPAPI zend_string *php_raw_url_encode(char const *s, size_t len);
PHPAPI void php_url_encode_to_smart_str(smart_str *buf, char const *s, size_t len, bool raw);

#define PHP_URL_SCHEME 0
#define PHP_URL_HOST 1
Expand Down
14 changes: 7 additions & 7 deletions ext/standard/url_scanner_ex.re
Original file line numberDiff line numberDiff line change
Expand Up@@ -188,7 +188,7 @@ alphadash = ([a-zA-Z] | "-");
#define YYLIMIT q
#define YYMARKER r

static inline void append_modified_url(smart_str *url, smart_str *dest, smart_str *url_app, const char *separator, int type)
static inline void append_modified_url(smart_str *url, smart_str *dest, smart_str *url_app, const zend_string *separator, int type)
{
php_url *url_parts;

Expand DownExpand Up@@ -271,7 +271,7 @@ static inline void append_modified_url(smart_str *url, smart_str *dest, smart_st
smart_str_appendc(dest, '?');
if (url_parts->query) {
smart_str_appends(dest, ZSTR_VAL(url_parts->query));
smart_str_appends(dest, separator);
smart_str_append(dest, separator);
smart_str_append_smart_str(dest, url_app);
} else {
smart_str_append_smart_str(dest, url_app);
Expand DownExpand Up@@ -757,7 +757,7 @@ static inline void php_url_scanner_add_var_impl(const char *name, size_t name_le
}

if (url_state->url_app.s && ZSTR_LEN(url_state->url_app.s) != 0) {
smart_str_appends(&url_state->url_app, PG(arg_separator).output);
smart_str_append(&url_state->url_app, PG(arg_separator).output);
}

if (encode) {
Expand DownExpand Up@@ -902,9 +902,9 @@ static inline zend_result php_url_scanner_reset_var_impl(zend_string *name, int
/* Get end of url var */
limit = ZSTR_VAL(url_state->url_app.s) + ZSTR_LEN(url_state->url_app.s);
end = start + ZSTR_LEN(url_app.s);
separator_len = strlen(PG(arg_separator).output);
separator_len = ZSTR_LEN(PG(arg_separator).output);
while (end < limit) {
if (!memcmp(end, PG(arg_separator).output, separator_len)) {
if (!memcmp(end, ZSTR_VAL(PG(arg_separator).output), separator_len)) {
end += separator_len;
sep_removed = 1;
break;
Expand All@@ -918,8 +918,8 @@ static inline zend_result php_url_scanner_reset_var_impl(zend_string *name, int
}
/* Check preceding separator */
if (!sep_removed
&& (size_t)(start - PG(arg_separator).output) >= separator_len
&& !memcmp(start - separator_len, PG(arg_separator).output, separator_len)) {
&& (size_t)(start - ZSTR_VAL(PG(arg_separator).output)) >= separator_len
&& !memcmp(start - separator_len, ZSTR_VAL(PG(arg_separator).output), separator_len)) {
start -= separator_len;
}
/* Remove partially */
Expand Down
4 changes: 2 additions & 2 deletions main/main.c
Original file line numberDiff line numberDiff line change
Expand Up@@ -771,8 +771,8 @@ PHP_INI_BEGIN()

STD_PHP_INI_ENTRY("unserialize_callback_func", NULL, PHP_INI_ALL, OnUpdateString, unserialize_callback_func, php_core_globals, core_globals)
STD_PHP_INI_ENTRY("serialize_precision", "-1", PHP_INI_ALL, OnSetSerializePrecision, serialize_precision, php_core_globals, core_globals)
STD_PHP_INI_ENTRY("arg_separator.output", "&", PHP_INI_ALL, OnUpdateStringUnempty, arg_separator.output, php_core_globals, core_globals)
STD_PHP_INI_ENTRY("arg_separator.input", "&", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateStringUnempty, arg_separator.input, php_core_globals, core_globals)
STD_PHP_INI_ENTRY("arg_separator.output", "&", PHP_INI_ALL, OnUpdateStrNotEmpty, arg_separator.output, php_core_globals, core_globals)
STD_PHP_INI_ENTRY("arg_separator.input", "&", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateStrNotEmpty, arg_separator.input, php_core_globals, core_globals)

STD_PHP_INI_ENTRY("auto_append_file", NULL, PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateString, auto_append_file, php_core_globals, core_globals)
STD_PHP_INI_ENTRY("auto_prepend_file", NULL, PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateString, auto_prepend_file, php_core_globals, core_globals)
Expand Down
4 changes: 2 additions & 2 deletions main/php_globals.h
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,8 +48,8 @@ extern ZEND_API struct _php_core_globals core_globals;
struct _php_tick_function_entry;

typedef struct _arg_separators {
char *output;
char *input;
zend_string *output;
zend_string *input;
} arg_separators;

struct _php_core_globals {
Expand Down
2 changes: 1 addition & 1 deletion main/php_variables.c
Original file line numberDiff line numberDiff line change
Expand Up@@ -527,7 +527,7 @@ SAPI_API SAPI_TREAT_DATA_FUNC(php_default_treat_data)
switch (arg) {
case PARSE_GET:
case PARSE_STRING:
separator = PG(arg_separator).input;
separator = ZSTR_VAL(PG(arg_separator).input);
break;
case PARSE_COOKIE:
separator = ";\0";
Expand Down
4 changes: 2 additions & 2 deletions sapi/cgi/cgi_main.c
Original file line numberDiff line numberDiff line change
Expand Up@@ -2420,7 +2420,7 @@ consult the installation file that came with this distribution, or visit \n\
* test.php v1=test "v2=hello world!"
*/
if (!SG(request_info).query_string && argc > php_optind) {
size_t slen = strlen(PG(arg_separator).input);
size_t slen = ZSTR_LEN(PG(arg_separator).input);
len = 0;
for (i = php_optind; i < argc; i++) {
if (i < (argc - 1)) {
Expand All@@ -2436,7 +2436,7 @@ consult the installation file that came with this distribution, or visit \n\
for (i = php_optind; i < argc; i++) {
strlcat(s, argv[i], len);
if (i < (argc - 1)) {
strlcat(s, PG(arg_separator).input, len);
strlcat(s, ZSTR_VAL(PG(arg_separator).input), len);
}
}
SG(request_info).query_string = s;
Expand Down
Loading
close