Skip to content

Commit 6521060

Browse files
committed
Make Option array parsing stricter
1 parent 4a21486 commit 6521060

File tree

2 files changed

+51
-58
lines changed

2 files changed

+51
-58
lines changed

ext/standard/head.c

+31-38
Original file line numberDiff line numberDiff line change
@@ -192,43 +192,35 @@ PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t e
192192
return result;
193193
}
194194

195-
static void php_head_parse_cookie_options_array(zval *options, zend_long *expires, zend_string **path, zend_string **domain, zend_bool *secure, zend_bool *httponly, zend_string **samesite) {
196-
int found = 0;
195+
static zend_result php_head_parse_cookie_options_array(zval *options, zend_long *expires, zend_string **path,
196+
zend_string **domain, zend_bool *secure, zend_bool *httponly, zend_string **samesite)
197+
{
197198
zend_string *key;
198199
zval *value;
199200

200201
ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(options), key, value) {
201-
if (key) {
202-
if (zend_string_equals_literal_ci(key, "expires")) {
203-
*expires = zval_get_long(value);
204-
found++;
205-
} else if (zend_string_equals_literal_ci(key, "path")) {
206-
*path = zval_get_string(value);
207-
found++;
208-
} else if (zend_string_equals_literal_ci(key, "domain")) {
209-
*domain = zval_get_string(value);
210-
found++;
211-
} else if (zend_string_equals_literal_ci(key, "secure")) {
212-
*secure = zval_is_true(value);
213-
found++;
214-
} else if (zend_string_equals_literal_ci(key, "httponly")) {
215-
*httponly = zval_is_true(value);
216-
found++;
217-
} else if (zend_string_equals_literal_ci(key, "samesite")) {
218-
*samesite = zval_get_string(value);
219-
found++;
220-
} else {
221-
php_error_docref(NULL, E_WARNING, "Unrecognized key '%s' found in the options array", ZSTR_VAL(key));
222-
}
202+
if (!key) {
203+
zend_value_error("%s(): option array cannot have numeric keys", get_active_function_name());
204+
return FAILURE;
205+
}
206+
if (zend_string_equals_literal_ci(key, "expires")) {
207+
*expires = zval_get_long(value);
208+
} else if (zend_string_equals_literal_ci(key, "path")) {
209+
*path = zval_get_string(value);
210+
} else if (zend_string_equals_literal_ci(key, "domain")) {
211+
*domain = zval_get_string(value);
212+
} else if (zend_string_equals_literal_ci(key, "secure")) {
213+
*secure = zval_is_true(value);
214+
} else if (zend_string_equals_literal_ci(key, "httponly")) {
215+
*httponly = zval_is_true(value);
216+
} else if (zend_string_equals_literal_ci(key, "samesite")) {
217+
*samesite = zval_get_string(value);
223218
} else {
224-
php_error_docref(NULL, E_WARNING, "Numeric key found in the options array");
219+
zend_value_error("%s(): option \"%s\" is invalid", get_active_function_name(), ZSTR_VAL(key));
220+
return FAILURE;
225221
}
226222
} ZEND_HASH_FOREACH_END();
227-
228-
/* Array is not empty but no valid keys were found */
229-
if (found == 0 && zend_hash_num_elements(Z_ARRVAL_P(options)) > 0) {
230-
php_error_docref(NULL, E_WARNING, "No valid options were found in the given array");
231-
}
223+
return SUCCESS;
232224
}
233225

234226
static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
@@ -257,22 +249,23 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
257249
"($expires_or_options) is an array", get_active_function_name());
258250
RETURN_THROWS();
259251
}
260-
php_head_parse_cookie_options_array(expires_or_options, &expires, &path,
261-
&domain, &secure, &httponly, &samesite);
252+
if (FAILURE == php_head_parse_cookie_options_array(expires_or_options, &expires, &path,
253+
&domain, &secure, &httponly, &samesite)) {
254+
goto cleanup;
255+
}
262256
} else {
263257
expires = zval_get_long(expires_or_options);
264258
}
265259
}
266260

267-
if (!EG(exception)) {
268-
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, !is_raw) == SUCCESS) {
269-
RETVAL_TRUE;
270-
} else {
271-
RETVAL_FALSE;
272-
}
261+
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, !is_raw) == SUCCESS) {
262+
RETVAL_TRUE;
263+
} else {
264+
RETVAL_FALSE;
273265
}
274266

275267
if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
268+
cleanup:
276269
if (path) {
277270
zend_string_release(path);
278271
}

ext/standard/tests/network/setcookie_array_option_error.phpt

+20-20
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,23 @@ date.timezone=UTC
88
ob_start();
99

1010
// Unrecognized key and no valid keys
11-
setcookie('name', 'value', ['unknown_key' => 'only']);
11+
try {
12+
setcookie('name', 'value', ['unknown_key' => 'only']);
13+
} catch (\ValueError $e) {
14+
echo $e->getMessage() . "\n";
15+
}
1216
// Numeric key and no valid keys
13-
setcookie('name2', 'value2', [0 => 'numeric_key']);
17+
try {
18+
setcookie('name2', 'value2', [0 => 'numeric_key']);
19+
} catch (\ValueError $e) {
20+
echo $e->getMessage() . "\n";
21+
}
1422
// Unrecognized key
15-
setcookie('name3', 'value3', ['path' => '/path/', 'foo' => 'bar']);
23+
try {
24+
setcookie('name3', 'value3', ['path' => '/path/', 'foo' => 'bar']);
25+
} catch (\ValueError $e) {
26+
echo $e->getMessage() . "\n";
27+
}
1628
// Invalid expiration date
1729
// To go above year 9999: 60 * 60 * 24 * 365 * 9999
1830
try {
@@ -44,26 +56,14 @@ var_dump(headers_list());
4456
--EXPECTHEADERS--
4557

4658
--EXPECTF--
47-
Warning: setcookie(): Unrecognized key 'unknown_key' found in the options array in %s
48-
49-
Warning: setcookie(): No valid options were found in the given array in %s
50-
51-
Warning: setcookie(): Numeric key found in the options array in %s
52-
53-
Warning: setcookie(): No valid options were found in the given array in %s
54-
55-
Warning: setcookie(): Unrecognized key 'foo' found in the options array in %s
59+
setcookie(): option "unknown_key" is invalid
60+
setcookie(): option array cannot have numeric keys
61+
setcookie(): option "foo" is invalid
5662
setcookie(): "expires" option cannot have a year greater than 9999
5763
setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
5864
setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
5965
setcookie(): Expects exactly 3 arguments when argument #3 ($expires_or_options) is an array
60-
array(4) {
66+
array(1) {
6167
[0]=>
62-
string(%d) "X-Powered-By: PHP/%s"
63-
[1]=>
64-
string(22) "Set-Cookie: name=value"
65-
[2]=>
66-
string(24) "Set-Cookie: name2=value2"
67-
[3]=>
68-
string(37) "Set-Cookie: name3=value3; path=/path/"
68+
string(%s) "X-Powered-By: PHP/%s"
6969
}

0 commit comments

Comments
 (0)