Skip to content

Commit 516fa16

Browse files
matyhtfmatyhtf
authored
Fix header merge in server (#5744)
* Optimize code * Fix the issue of duplicate headers not being merged in the HTTP server.
1 parent 53557e0 commit 516fa16

File tree

6 files changed

+73
-56
lines changed

6 files changed

+73
-56
lines changed

‎ext-src/php_swoole_cxx.cc

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,55 @@ void known_strings_dtor(void) {
3636
sw_zend_known_strings = nullptr;
3737
}
3838

39+
static zend_always_inline zval *sw_zend_symtable_str_add(
40+
HashTable *ht, const char *str, size_t len, zend_ulong idx, bool numeric_key, zval *pData) {
41+
if (numeric_key) {
42+
return zend_hash_index_add(ht, idx, pData);
43+
} else {
44+
return zend_hash_str_add(ht, str, len, pData);
45+
}
46+
}
47+
48+
static zend_always_inline zval *sw_zend_symtable_str_find(
49+
HashTable *ht, const char *str, size_t len, zend_ulong idx, bool numeric_key) {
50+
if (numeric_key) {
51+
return zend_hash_index_find(ht, idx);
52+
} else {
53+
return zend_hash_str_find(ht, str, len);
54+
}
55+
}
56+
57+
static zend_always_inline zval *sw_zend_symtable_str_update(
58+
HashTable *ht, const char *str, size_t len, zend_ulong idx, bool numeric_key, zval *pData) {
59+
if (numeric_key) {
60+
return zend_hash_index_update(ht, idx, pData);
61+
} else {
62+
return zend_hash_str_update(ht, str, len, pData);
63+
}
64+
}
65+
66+
void array_add_or_merge(zval *zarray, const char *key, size_t key_len, zval *new_element) {
67+
zend_ulong idx;
68+
bool numeric_key = ZEND_HANDLE_NUMERIC_STR(key, key_len, idx);
69+
70+
zend_array *array = Z_ARRVAL_P(zarray);
71+
zval *zresult = sw_zend_symtable_str_add(array, key, key_len, idx, numeric_key, new_element);
72+
// Adding element failed, indicating that this key already exists and must be converted to an array
73+
if (!zresult) {
74+
zval *current_header = sw_zend_symtable_str_find(array, key, key_len, idx, numeric_key);
75+
if (ZVAL_IS_ARRAY(current_header)) {
76+
add_next_index_zval(current_header, new_element);
77+
} else {
78+
zval zvalue_array;
79+
array_init_size(&zvalue_array, 2);
80+
Z_ADDREF_P(current_header);
81+
add_next_index_zval(&zvalue_array, current_header);
82+
add_next_index_zval(&zvalue_array, new_element);
83+
sw_zend_symtable_str_update(array, key, key_len, idx, numeric_key, &zvalue_array);
84+
}
85+
}
86+
}
87+
3988
namespace function {
4089

4190
bool call(zend_fcall_info_cache *fci_cache, uint32_t argc, zval *argv, zval *retval, const bool enable_coroutine) {

‎ext-src/php_swoole_cxx.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,13 @@ static inline void array_unset(zval *arg, const char *key, size_t l_key) {
749749
zend_hash_str_del(Z_ARRVAL_P(arg), key, l_key);
750750
}
751751

752+
/**
753+
* Add new element to the associative array or merge with existing elements.
754+
* If the key does not exist, add it to the array.
755+
* If the key already exists, merge all into a two-dimensional array.
756+
*/
757+
void array_add_or_merge(zval *zarray, const char *key, size_t key_len, zval *new_element);
758+
752759
static inline zend_long object_get_long(zval *obj, zend_string *key) {
753760
static zval rv;
754761
zval *property = zend_read_property_ex(Z_OBJCE_P(obj), Z_OBJ_P(obj), key, 1, &rv);

‎ext-src/swoole_http_client_coro.cc

Lines changed: 3 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -819,61 +819,14 @@ void Client::set_basic_auth(const std::string &username, const std::string &pass
819819
}
820820
}
821821

822-
static zend_always_inline zval *sw_zend_symtable_str_add(
823-
HashTable *ht, const char *str, size_t len, zend_ulong idx, bool numeric_key, zval *pData) {
824-
if (numeric_key) {
825-
return zend_hash_index_add(ht, idx, pData);
826-
} else {
827-
return zend_hash_str_add(ht, str, len, pData);
828-
}
829-
}
830-
831-
static zend_always_inline zval *sw_zend_symtable_str_find(
832-
HashTable *ht, const char *str, size_t len, zend_ulong idx, bool numeric_key) {
833-
if (numeric_key) {
834-
return zend_hash_index_find(ht, idx);
835-
} else {
836-
return zend_hash_str_find(ht, str, len);
837-
}
838-
}
839-
840-
static zend_always_inline zval *sw_zend_symtable_str_update(
841-
HashTable *ht, const char *str, size_t len, zend_ulong idx, bool numeric_key, zval *pData) {
842-
if (numeric_key) {
843-
return zend_hash_index_update(ht, idx, pData);
844-
} else {
845-
return zend_hash_str_update(ht, str, len, pData);
846-
}
847-
}
848-
849822
void Client::add_header(const char *key, size_t key_len, const char *str, size_t length) {
850823
zval *zheaders =
851824
sw_zend_read_and_convert_property_array(swoole_http_client_coro_ce, zobject, ZEND_STRL("headers"), 0);
852-
zend_array *array = Z_ARRVAL_P(zheaders);
853825

854-
zval zvalue_new;
855-
ZVAL_STRINGL(&zvalue_new, str, length);
826+
zval zheader_new;
827+
ZVAL_STRINGL(&zheader_new, str, length);
856828

857-
zend_ulong idx;
858-
bool numeric_key = ZEND_HANDLE_NUMERIC_STR(key, key_len, idx);
859-
860-
zval *zresult = sw_zend_symtable_str_add(array, key, key_len, idx, numeric_key, &zvalue_new);
861-
/**
862-
* Adding failed, indicating that this header already exists and must be converted to an array
863-
*/
864-
if (!zresult) {
865-
zval *zvalue_found = sw_zend_symtable_str_find(array, key, key_len, idx, numeric_key);
866-
if (ZVAL_IS_ARRAY(zvalue_found)) {
867-
add_next_index_zval(zvalue_found, &zvalue_new);
868-
} else {
869-
zval zvalue_array;
870-
array_init_size(&zvalue_array, 2);
871-
Z_ADDREF_P(zvalue_found);
872-
add_next_index_zval(&zvalue_array, zvalue_found);
873-
add_next_index_zval(&zvalue_array, &zvalue_new);
874-
sw_zend_symtable_str_update(array, key, key_len, idx, numeric_key, &zvalue_array);
875-
}
876-
}
829+
zend::array_add_or_merge(zheaders, key, key_len, &zheader_new);
877830
}
878831

879832
bool Client::connect() {

‎ext-src/swoole_http_request.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ static int http_request_on_header_value(swoole_http_parser *parser, const char *
412412
} else {
413413
char *new_header_name = estrndup(header_name, header_len);
414414
zend_str_tolower_copy(new_header_name, header_name, header_len);
415-
zend_hash_str_update(Z_ARR_P(zheader), new_header_name, header_len, &tmp);
415+
zend::array_add_or_merge(zheader, new_header_name, header_len, &tmp);
416416
efree(new_header_name);
417417
}
418418

‎ext-src/swoole_socket_coro.cc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1731,10 +1731,7 @@ static PHP_METHOD(swoole_socket_coro, sendFile) {
17311731

17321732
swoole_get_socket_coro(sock, ZEND_THIS);
17331733
if (!sock->socket->sendfile(file, offset, length)) {
1734-
zend_update_property_long(
1735-
swoole_socket_coro_ce, SW_Z8_OBJ_P(ZEND_THIS), ZEND_STRL("errCode"), sock->socket->errCode);
1736-
zend_update_property_string(
1737-
swoole_socket_coro_ce, SW_Z8_OBJ_P(ZEND_THIS), ZEND_STRL("errMsg"), sock->socket->errMsg);
1734+
socket_coro_sync_properties(ZEND_THIS, sock);
17381735
RETVAL_FALSE;
17391736
} else {
17401737
RETVAL_TRUE;

‎tests/swoole_http_server/duplicate_header.phpt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,15 @@ $pm->parentFunc = function () use ($pm) {
1212
$ch = curl_init();
1313
curl_setopt($ch, CURLOPT_URL, "http://127.0.0.1:{$pm->getFreePort()}/");
1414
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
15-
curl_setopt($ch, CURLOPT_HEADER, TRUE);
15+
curl_setopt($ch, CURLOPT_HTTPHEADER, [
16+
'X-Test-Header1: value1',
17+
'X-Test-Header2: value2',
18+
'X-Test-Header2: value3',
19+
'X-Test-Header3: value4',
20+
'X-Test-Header3: value5',
21+
'X-Test-Header3: value6',
22+
]);
23+
curl_setopt($ch, CURLOPT_HEADER, true);
1624
echo curl_exec($ch);
1725
curl_close($ch);
1826
$pm->kill();
@@ -30,6 +38,9 @@ $pm->childFunc = function () use ($pm) {
3038
});
3139
$http->on('request', function (Swoole\Http\Request $request, Swoole\Http\Response $response) {
3240
$msg = "hello world";
41+
Assert::eq($request->header['x-test-header1'], 'value1');
42+
Assert::eq($request->header['x-test-header2'], ['value2', 'value3']);
43+
Assert::eq($request->header['x-test-header3'], ['value4', 'value5', 'value6']);
3344
$response->header("content-length", strlen($msg) . " ");
3445
$response->header("Test-Value", [
3546
"a\r\n",

0 commit comments

Comments
 (0)