Skip to content

Commit 399ee62

Browse files
GirgiasGirgias
committed
Review
1 parent ff47817 commit 399ee62

File tree

6 files changed

+60
-59
lines changed

6 files changed

+60
-59
lines changed

‎ext/standard/head.c

Lines changed: 38 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,46 @@ 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 int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain,
81+
bool secure, bool httponly, zend_string *samesite, bool is_raw)
8082
{
8183
zend_string *dt;
8284
sapi_header_line ctr = {0};
8385
int result;
8486
smart_str buf = {0};
8587

88+
if (ZSTR_LEN(name) == 0) {
89+
zend_argument_value_error(1, "cannot be empty");
90+
return FAILURE;
91+
}
92+
if (strpbrk(ZSTR_VAL(name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
93+
zend_argument_value_error(1, "cannot contain \"=\", " ILLEGAL_COOKIE_CHARACTER);
94+
return FAILURE;
95+
}
96+
if (is_raw && value &&
97+
strpbrk(ZSTR_VAL(value), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
98+
zend_argument_value_error(2, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
99+
return FAILURE;
100+
}
101+
/* check to make sure that the year does not exceed 4 digits in length */
102+
/* Epoch timestamp: 253402300799 corresponds to 31 December 9999 23:59:59 GMT */
103+
if (expires >= 253402300800) {
104+
zend_value_error("%s(): \"expire\" option must be less than 253402300800", get_active_function_name());
105+
return FAILURE;
106+
}
107+
if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
108+
zend_value_error("%s(): \"path\" option cannot contain "
109+
ILLEGAL_COOKIE_CHARACTER, get_active_function_name());
110+
return FAILURE;
111+
}
112+
if (domain && strpbrk(ZSTR_VAL(domain), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
113+
zend_value_error("%s(): \"domain\" option cannot contain " ILLEGAL_COOKIE_CHARACTER,
114+
get_active_function_name());
115+
return FAILURE;
116+
}
117+
/* Should check value of SameSite? */
118+
86119
if (value == NULL || ZSTR_LEN(value) == 0) {
87120
/*
88121
* MSIE doesn't delete a cookie when you set it to a null value
@@ -100,7 +133,8 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires,
100133
smart_str_appends(&buf, "Set-Cookie: ");
101134
smart_str_append(&buf, name);
102135
smart_str_appendc(&buf, '=');
103-
if (url_encode) {
136+
/* URL encode when we don't want the raw header */
137+
if (!is_raw) {
104138
zend_string *encoded_value = php_raw_url_encode(ZSTR_VAL(value), ZSTR_LEN(value));
105139
smart_str_append(&buf, encoded_value);
106140
zend_string_release_ex(encoded_value, 0);
@@ -113,15 +147,6 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires,
113147

114148
smart_str_appends(&buf, COOKIE_EXPIRES);
115149
dt = php_format_date("D, d-M-Y H:i:s T", sizeof("D, d-M-Y H:i:s T")-1, expires, 0);
116-
/* check to make sure that the year does not exceed 4 digits in length */
117-
p = zend_memrchr(ZSTR_VAL(dt), '-', ZSTR_LEN(dt));
118-
if (!p || *(p + 5) != ' ') {
119-
zend_string_free(dt);
120-
smart_str_free(&buf);
121-
zend_error(E_WARNING, "Expiry date cannot have a year greater than 9999");
122-
return FAILURE;
123-
}
124-
125150
smart_str_append(&buf, dt);
126151
zend_string_free(dt);
127152

@@ -201,7 +226,6 @@ static void php_head_parse_cookie_options_array(zval *options, zend_long *expire
201226
}
202227
}
203228

204-
#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\""
205229
static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
206230
{
207231
/* to handle overloaded function array|int */
@@ -215,6 +239,7 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
215239
Z_PARAM_OPTIONAL
216240
Z_PARAM_STR(value)
217241
Z_PARAM_ZVAL(expires_or_options)
242+
// Use path ZPP check?
218243
Z_PARAM_STR(path)
219244
Z_PARAM_STR(domain)
220245
Z_PARAM_BOOL(secure)
@@ -229,56 +254,19 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
229254
RETURN_THROWS();
230255
}
231256
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? */
245257
} else {
246258
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-
}
255259
}
256260
}
257-
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-
272261
if (!EG(exception)) {
273-
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, !is_raw) == SUCCESS) {
262+
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, is_raw) == SUCCESS) {
274263
RETVAL_TRUE;
275264
} else {
276265
RETVAL_FALSE;
277266
}
278267
}
279268

280269
if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
281-
cleanup:
282270
if (path) {
283271
zend_string_release(path);
284272
}

‎ext/standard/head.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
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 int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain,
32+
bool secure, bool httponly, zend_string *samesite, bool url_encode);
3233

3334
#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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ Warning: setcookie(): Numeric key found in the options array in %s
4646
Warning: setcookie(): No valid options were found in the given array in %s
4747

4848
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"
49+
setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
50+
setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
5151
setcookie(): Expects exactly 3 arguments when argument #3 ($expires_or_options) is an array
5252
array(4) {
5353
[0]=>

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ try {
2222
} catch (\ValueError $e) {
2323
echo $e->getMessage() . "\n";
2424
}
25+
try {
26+
setcookie('name', 'value', 253402300800);
27+
} catch (\ValueError $e) {
28+
echo $e->getMessage() . "\n";
29+
}
2530
try {
2631
setcookie('name', 'value', 100, 'invalid;');
2732
} catch (\ValueError $e) {
@@ -39,8 +44,9 @@ var_dump(headers_list());
3944
--EXPECTF--
4045
setcookie(): Argument #1 ($name) cannot be empty
4146
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"
47+
setcookie(): "expire" option must be less than 253402300800
48+
setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
49+
setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
4450
array(2) {
4551
[0]=>
4652
string(%d) "X-Powered-By: PHP/%s"

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ try {
2222
} catch (\ValueError $e) {
2323
echo $e->getMessage() . "\n";
2424
}
25+
try {
26+
setrawcookie('name', 'value', 253402300800);
27+
} catch (\ValueError $e) {
28+
echo $e->getMessage() . "\n";
29+
}
2530
try {
2631
setrawcookie('name', 'value', 100, 'invalid;');
2732
} catch (\ValueError $e) {
@@ -40,8 +45,9 @@ var_dump(headers_list());
4045
setrawcookie(): Argument #1 ($name) cannot be empty
4146
setrawcookie(): Argument #1 ($name) cannot contain "=", ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
4247
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"
48+
setrawcookie(): "expire" option must be less than 253402300800
49+
setrawcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
50+
setrawcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
4551
array(1) {
4652
[0]=>
4753
string(%d) "X-Powered-By: PHP/%s"

0 commit comments

Comments
 (0)