Warning to error promotion for set(raw)cookie() #5819
Conversation
Girgias
force-pushed
the
setcookie-warning-to-error
branch
2 times, most recently
from
c1691bf to
987ca60
Compare
|
If there are no objections I'll merge this at the end of the week. |
ext/standard/head.c
Outdated
| } | ||
| php_head_parse_cookie_options_array(expires_or_options, &expires, &path, &domain, &secure, &httponly, &samesite); | ||
| if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */ | ||
| zend_value_error("%s(): Argument #3 ($expires_or_options[\"path\"]) cannot contain " |
There was a problem hiding this comment.
cc @kocsismate, but I don't think this format makes sense. $expires_or_options["path"] is not the parameter name.
There was a problem hiding this comment.
FWIW, I don't think we should try too hard to get the argument error formatting everywhere. If it's inconvenient from the implementation POV, don't do it. Here, it would be more elegant to keep checks inside php_setcookie.
There was a problem hiding this comment.
we used unserialize(): "max_depth" option must be of type int, %s given in a similar situation :)
ext/standard/head.c
Outdated
| zend_value_error("%s(): Argument #3 ($expires_or_options[\"path\"]) cannot contain " | ||
| ILLEGAL_COOKIE_CHARACTER, get_active_function_name()); | ||
| goto cleanup; | ||
| RETURN_THROWS(); |
There was a problem hiding this comment.
This RETURN_THROWS() doesn't make much sense, as it cannot be executed after the goto cleanup.
Girgias
force-pushed
the
setcookie-warning-to-error
branch
2 times, most recently
from
399ee62 to
39d24b4
Compare
Girgias
force-pushed
the
setcookie-warning-to-error
branch
from
39d24b4 to
59e239c
Compare
Girgias
force-pushed
the
setcookie-warning-to-error
branch
from
59e239c to
6521060
Compare
| @@ -76,36 +76,41 @@ PHPAPI int php_header(void) | |||
| } | |||
| } | |||
|
|
|||
| 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) | |||
| #define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\"" | |||
There was a problem hiding this comment.
| #define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\"" | |
| #define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", or \"\\014\"" |
I think...
| } | ||
| // To go above year 9999: 60 * 60 * 24 * 365 * 9999 | ||
| try { | ||
| setcookie('name', 'value', 315328464000); |
There was a problem hiding this comment.
| setcookie('name', 'value', 315328464000); | |
| setrawcookie('name', 'value', 315328464000); |
I've merged
setcookie()andsetrawcookie()into a common implementation at the same time