Skip to content

Commit 4a21486

Browse files
GirgiasGirgias
committed
Reviews
1 parent 6d3e478 commit 4a21486

File tree

6 files changed

+68
-50
lines changed

6 files changed

+68
-50
lines changed

‎ext/standard/head.c

Lines changed: 35 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,42 @@ PHPAPI int php_header(void)
7676
}
7777
}
7878

79-
PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int httponly, zend_string *samesite, int url_encode)
79+
#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\""
80+
PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t expires,
81+
zend_string *path, zend_string *domain, bool secure, bool httponly,
82+
zend_string *samesite, bool url_encode)
8083
{
8184
zend_string *dt;
8285
sapi_header_line ctr = {0};
83-
int result;
86+
zend_result result;
8487
smart_str buf = {0};
8588

89+
if (!ZSTR_LEN(name)) {
90+
zend_argument_value_error(1, "cannot be empty");
91+
return FAILURE;
92+
}
93+
if (strpbrk(ZSTR_VAL(name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
94+
zend_argument_value_error(1, "cannot contain \"=\", " ILLEGAL_COOKIE_CHARACTER);
95+
return FAILURE;
96+
}
97+
if (!url_encode && value &&
98+
strpbrk(ZSTR_VAL(value), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
99+
zend_argument_value_error(2, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
100+
return FAILURE;
101+
}
102+
103+
if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
104+
zend_value_error("%s(): \"path\" option cannot contain " ILLEGAL_COOKIE_CHARACTER,
105+
get_active_function_name());
106+
return FAILURE;
107+
}
108+
if (domain && strpbrk(ZSTR_VAL(domain), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
109+
zend_value_error("%s(): \"domain\" option cannot contain " ILLEGAL_COOKIE_CHARACTER,
110+
get_active_function_name());
111+
return FAILURE;
112+
}
113+
/* Should check value of SameSite? */
114+
86115
if (value == NULL || ZSTR_LEN(value) == 0) {
87116
/*
88117
* MSIE doesn't delete a cookie when you set it to a null value
@@ -118,7 +147,8 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires,
118147
if (!p || *(p + 5) != ' ') {
119148
zend_string_free(dt);
120149
smart_str_free(&buf);
121-
zend_error(E_WARNING, "Expiry date cannot have a year greater than 9999");
150+
zend_value_error("%s(): \"expires\" option cannot have a year greater than 9999",
151+
get_active_function_name());
122152
return FAILURE;
123153
}
124154

@@ -201,7 +231,6 @@ static void php_head_parse_cookie_options_array(zval *options, zend_long *expire
201231
}
202232
}
203233

204-
#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\""
205234
static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
206235
{
207236
/* to handle overloaded function array|int */
@@ -228,47 +257,13 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
228257
"($expires_or_options) is an array", get_active_function_name());
229258
RETURN_THROWS();
230259
}
231-
php_head_parse_cookie_options_array(expires_or_options, &expires, &path, &domain, &secure, &httponly, &samesite);
232-
if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
233-
zend_value_error("%s(): Argument #3 ($expires_or_options[\"path\"]) cannot contain "
234-
ILLEGAL_COOKIE_CHARACTER, get_active_function_name());
235-
goto cleanup;
236-
RETURN_THROWS();
237-
}
238-
if (domain && strpbrk(ZSTR_VAL(domain), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
239-
zend_value_error("%s(): Argument #3 ($expires_or_options[\"domain\"]) cannot contain "
240-
ILLEGAL_COOKIE_CHARACTER, get_active_function_name());
241-
goto cleanup;
242-
RETURN_THROWS();
243-
}
244-
/* Should check value of SameSite? */
260+
php_head_parse_cookie_options_array(expires_or_options, &expires, &path,
261+
&domain, &secure, &httponly, &samesite);
245262
} else {
246263
expires = zval_get_long(expires_or_options);
247-
if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
248-
zend_argument_value_error(4, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
249-
RETURN_THROWS();
250-
}
251-
if (domain && strpbrk(ZSTR_VAL(domain), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
252-
zend_argument_value_error(5, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
253-
RETURN_THROWS();
254-
}
255264
}
256265
}
257266

258-
if (!ZSTR_LEN(name)) {
259-
zend_argument_value_error(1, "cannot be empty");
260-
RETURN_THROWS();
261-
}
262-
if (strpbrk(ZSTR_VAL(name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
263-
zend_argument_value_error(1, "cannot contain \"=\", " ILLEGAL_COOKIE_CHARACTER);
264-
RETURN_THROWS();
265-
}
266-
if (is_raw && value &&
267-
strpbrk(ZSTR_VAL(value), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
268-
zend_argument_value_error(2, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
269-
RETURN_THROWS();
270-
}
271-
272267
if (!EG(exception)) {
273268
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, !is_raw) == SUCCESS) {
274269
RETVAL_TRUE;
@@ -278,7 +273,6 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
278273
}
279274

280275
if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
281-
cleanup:
282276
if (path) {
283277
zend_string_release(path);
284278
}

‎ext/standard/head.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
extern PHP_RINIT_FUNCTION(head);
2929

3030
PHPAPI int php_header(void);
31-
PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int httponly, zend_string *samesite, int url_encode);
31+
PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t expires,
32+
zend_string *path, zend_string *domain, bool secure, bool httponly,
33+
zend_string *samesite, bool url_encode);
3234

3335
#endif

‎ext/standard/tests/network/bug69948.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@ try {
1717
===DONE===
1818
--EXPECTHEADERS--
1919
--EXPECT--
20-
setcookie(): Argument #4 ($path) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
21-
setcookie(): Argument #5 ($domain) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
20+
setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
21+
setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
2222
===DONE===

‎ext/standard/tests/network/setcookie_array_option_error.phpt

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ setcookie('name', 'value', ['unknown_key' => 'only']);
1313
setcookie('name2', 'value2', [0 => 'numeric_key']);
1414
// Unrecognized key
1515
setcookie('name3', 'value3', ['path' => '/path/', 'foo' => 'bar']);
16+
// Invalid expiration date
17+
// To go above year 9999: 60 * 60 * 24 * 365 * 9999
18+
try {
19+
setcookie('name', 'value', ['expires' => 315328464000]);
20+
} catch (\ValueError $e) {
21+
echo $e->getMessage() . "\n";
22+
}
1623
// Invalid path key content
1724
try {
1825
setcookie('name', 'value', ['path' => '/;/']);
@@ -46,8 +53,9 @@ Warning: setcookie(): Numeric key found in the options array in %s
4653
Warning: setcookie(): No valid options were found in the given array in %s
4754

4855
Warning: setcookie(): Unrecognized key 'foo' found in the options array in %s
49-
setcookie(): Argument #3 ($expires_or_options["path"]) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
50-
setcookie(): Argument #3 ($expires_or_options["domain"]) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
56+
setcookie(): "expires" option cannot have a year greater than 9999
57+
setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
58+
setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
5159
setcookie(): Expects exactly 3 arguments when argument #3 ($expires_or_options) is an array
5260
array(4) {
5361
[0]=>

‎ext/standard/tests/network/setcookie_error.phpt

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ try {
2222
} catch (\ValueError $e) {
2323
echo $e->getMessage() . "\n";
2424
}
25+
// To go above year 9999: 60 * 60 * 24 * 365 * 9999
26+
try {
27+
setcookie('name', 'value', 315328464000);
28+
} catch (\ValueError $e) {
29+
echo $e->getMessage() . "\n";
30+
}
2531
try {
2632
setcookie('name', 'value', 100, 'invalid;');
2733
} catch (\ValueError $e) {
@@ -39,8 +45,9 @@ var_dump(headers_list());
3945
--EXPECTF--
4046
setcookie(): Argument #1 ($name) cannot be empty
4147
setcookie(): Argument #1 ($name) cannot contain "=", ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
42-
setcookie(): Argument #4 ($path) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
43-
setcookie(): Argument #5 ($domain) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
48+
setcookie(): "expires" option cannot have a year greater than 9999
49+
setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
50+
setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
4451
array(2) {
4552
[0]=>
4653
string(%d) "X-Powered-By: PHP/%s"

‎ext/standard/tests/network/setrawcookie_error.phpt

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ try {
2222
} catch (\ValueError $e) {
2323
echo $e->getMessage() . "\n";
2424
}
25+
// To go above year 9999: 60 * 60 * 24 * 365 * 9999
26+
try {
27+
setcookie('name', 'value', 315328464000);
28+
} catch (\ValueError $e) {
29+
echo $e->getMessage() . "\n";
30+
}
2531
try {
2632
setrawcookie('name', 'value', 100, 'invalid;');
2733
} catch (\ValueError $e) {
@@ -40,8 +46,9 @@ var_dump(headers_list());
4046
setrawcookie(): Argument #1 ($name) cannot be empty
4147
setrawcookie(): Argument #1 ($name) cannot contain "=", ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
4248
setrawcookie(): Argument #2 ($value) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
43-
setrawcookie(): Argument #4 ($path) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
44-
setrawcookie(): Argument #5 ($domain) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
49+
setcookie(): "expires" option cannot have a year greater than 9999
50+
setrawcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
51+
setrawcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
4552
array(1) {
4653
[0]=>
4754
string(%d) "X-Powered-By: PHP/%s"

0 commit comments

Comments
 (0)