Skip to content

Commit 39d24b4

Browse files
GirgiasGirgias
committed
Review
1 parent ccebc7e commit 39d24b4

File tree

6 files changed

+60
-60
lines changed

6 files changed

+60
-60
lines changed

‎ext/standard/head.c

+38-51
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,28 +133,19 @@ 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);
107141
} else {
108142
smart_str_append(&buf, value);
109143
}
110144
if (expires > 0) {
111-
const char *p;
112145
double diff;
113146

114147
smart_str_appends(&buf, COOKIE_EXPIRES);
115148
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-
125149
smart_str_append(&buf, dt);
126150
zend_string_free(dt);
127151

@@ -201,7 +225,6 @@ static void php_head_parse_cookie_options_array(zval *options, zend_long *expire
201225
}
202226
}
203227

204-
#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\""
205228
static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
206229
{
207230
/* to handle overloaded function array|int */
@@ -215,6 +238,7 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
215238
Z_PARAM_OPTIONAL
216239
Z_PARAM_STR(value)
217240
Z_PARAM_ZVAL(expires_or_options)
241+
// Use path ZPP check?
218242
Z_PARAM_STR(path)
219243
Z_PARAM_STR(domain)
220244
Z_PARAM_BOOL(secure)
@@ -229,56 +253,19 @@ static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
229253
RETURN_THROWS();
230254
}
231255
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? */
245256
} else {
246257
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-
}
255258
}
256259
}
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-
272260
if (!EG(exception)) {
273-
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, !is_raw) == SUCCESS) {
261+
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, is_raw) == SUCCESS) {
274262
RETVAL_TRUE;
275263
} else {
276264
RETVAL_FALSE;
277265
}
278266
}
279267

280268
if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
281-
cleanup:
282269
if (path) {
283270
zend_string_release(path);
284271
}

‎ext/standard/head.h

+2-1
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

+2-2
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

+2-2
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

+8-2
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

+8-2
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)