From c5f9a231d5adc8cf8ab9890f0acc27206495a080 Mon Sep 17 00:00:00 2001 From: Martin Jansen Date: Tue, 6 May 2014 22:00:59 +0200 Subject: [PATCH] Streamlining of cookie handling in ext/session and setcookie Up until now the session cookie used "HttpOnly" to indicate cookies only available through HTTP while setcookie() used "httponly". The relevant RFC 6265 claims that case does not matter for this token, but only explicitely mentions "HttpOnly". Thus this seems like a logical choice when streamlining the code. Also the setcookie implementation now uses the same string constants as the session extension for other tokens like Max-Age or the domain attribute. This change poses a slight risk of backwards incompatibility in places where people deliberately ignore chapter 5.2.5 of RFC 6265 and perform case-sensitive checks for the HttpOnly attribute. --- ext/session/session.c | 9 +-------- ext/standard/head.c | 16 ++++++++-------- ext/standard/head.h | 8 ++++++++ ext/standard/tests/network/setcookie.phpt | 2 +- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/ext/session/session.c b/ext/session/session.c index 1d60c40188..c5d710096d 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -51,6 +51,7 @@ #include "ext/standard/php_smart_str.h" #include "ext/standard/url.h" #include "ext/standard/basic_functions.h" +#include "ext/standard/head.h" #include "mod_files.h" #include "mod_user.h" @@ -1289,14 +1290,6 @@ static int php_session_cache_limiter(TSRMLS_D) /* {{{ */ * Cookie Management * ********************* */ -#define COOKIE_SET_COOKIE "Set-Cookie: " -#define COOKIE_EXPIRES "; expires=" -#define COOKIE_MAX_AGE "; Max-Age=" -#define COOKIE_PATH "; path=" -#define COOKIE_DOMAIN "; domain=" -#define COOKIE_SECURE "; secure" -#define COOKIE_HTTPONLY "; HttpOnly" - /* * Remove already sent session ID cookie. * It must be directly removed from SG(sapi_header) because sapi_add_header_ex() diff --git a/ext/standard/head.c b/ext/standard/head.c index eca032a97b..6ede184df2 100644 --- a/ext/standard/head.c +++ b/ext/standard/head.c @@ -117,14 +117,14 @@ PHPAPI int php_setcookie(char *name, int name_len, char *value, int value_len, t * pick an expiry date in the past */ dt = php_format_date("D, d-M-Y H:i:s T", sizeof("D, d-M-Y H:i:s T")-1, 1, 0 TSRMLS_CC); - snprintf(cookie, len + 100, "Set-Cookie: %s=deleted; expires=%s; Max-Age=0", name, dt); + snprintf(cookie, len + 100, "%s%s=deleted; expires=%s; Max-Age=0", COOKIE_SET_COOKIE, name, dt); efree(dt); } else { - snprintf(cookie, len + 100, "Set-Cookie: %s=%s", name, value ? encoded_value : ""); + snprintf(cookie, len + 100, "%s%s=%s", COOKIE_SET_COOKIE, name, value ? encoded_value : ""); if (expires > 0) { const char *p; char tsdelta[13]; - strlcat(cookie, "; expires=", len + 100); + strlcat(cookie, COOKIE_EXPIRES, len + 100); dt = php_format_date("D, d-M-Y H:i:s T", sizeof("D, d-M-Y H:i:s T")-1, expires, 0 TSRMLS_CC); /* check to make sure that the year does not exceed 4 digits in length */ p = zend_memrchr(dt, '-', strlen(dt)); @@ -139,7 +139,7 @@ PHPAPI int php_setcookie(char *name, int name_len, char *value, int value_len, t efree(dt); snprintf(tsdelta, sizeof(tsdelta), "%li", (long) difftime(expires, time(NULL))); - strlcat(cookie, "; Max-Age=", len + 100); + strlcat(cookie, COOKIE_MAX_AGE, len + 100); strlcat(cookie, tsdelta, len + 100); } } @@ -149,18 +149,18 @@ PHPAPI int php_setcookie(char *name, int name_len, char *value, int value_len, t } if (path && path_len > 0) { - strlcat(cookie, "; path=", len + 100); + strlcat(cookie, COOKIE_PATH, len + 100); strlcat(cookie, path, len + 100); } if (domain && domain_len > 0) { - strlcat(cookie, "; domain=", len + 100); + strlcat(cookie, COOKIE_DOMAIN, len + 100); strlcat(cookie, domain, len + 100); } if (secure) { - strlcat(cookie, "; secure", len + 100); + strlcat(cookie, COOKIE_SECURE, len + 100); } if (httponly) { - strlcat(cookie, "; httponly", len + 100); + strlcat(cookie, COOKIE_HTTPONLY, len + 100); } ctr.line = cookie; diff --git a/ext/standard/head.h b/ext/standard/head.h index efca9b8637..cb9a7f4823 100644 --- a/ext/standard/head.h +++ b/ext/standard/head.h @@ -21,6 +21,14 @@ #ifndef HEAD_H #define HEAD_H +#define COOKIE_SET_COOKIE "Set-Cookie: " +#define COOKIE_EXPIRES "; expires=" +#define COOKIE_MAX_AGE "; Max-Age=" +#define COOKIE_DOMAIN "; domain=" +#define COOKIE_PATH "; path=" +#define COOKIE_SECURE "; secure" +#define COOKIE_HTTPONLY "; HttpOnly" + extern PHP_RINIT_FUNCTION(head); PHP_FUNCTION(header); PHP_FUNCTION(header_remove); diff --git a/ext/standard/tests/network/setcookie.phpt b/ext/standard/tests/network/setcookie.phpt index bf04ec78de..17db873560 100644 --- a/ext/standard/tests/network/setcookie.phpt +++ b/ext/standard/tests/network/setcookie.phpt @@ -29,7 +29,7 @@ $expected = array( 'Set-Cookie: name=value; path=/path/', 'Set-Cookie: name=value; domain=domain.tld', 'Set-Cookie: name=value; secure', - 'Set-Cookie: name=value; httponly' + 'Set-Cookie: name=value; HttpOnly' ); $headers = headers_list(); -- 2.40.0