From c5f9a231d5adc8cf8ab9890f0acc27206495a080 Mon Sep 17 00:00:00 2001 From: Martin Jansen Date: Tue, 6 May 2014 22:00:59 +0200 Subject: [PATCH 1/2] 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 1d60c40188a..c5d710096d3 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 eca032a97b1..6ede184df26 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 efca9b8637f..cb9a7f48230 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 bf04ec78de1..17db873560a 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(); From 068bf645e1c9952008e08570dad27bb03101e689 Mon Sep 17 00:00:00 2001 From: Martin Jansen Date: Sun, 18 May 2014 10:45:31 +0200 Subject: [PATCH 2/2] Remove usage of pointless COOKIE_SET_COOKIE constant. --- ext/session/session.c | 2 +- ext/standard/head.c | 4 ++-- ext/standard/head.h | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ext/session/session.c b/ext/session/session.c index c5d710096d3..62ebaf28a14 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1355,7 +1355,7 @@ static void php_session_send_cookie(TSRMLS_D) /* {{{ */ e_session_name = php_url_encode(PS(session_name), strlen(PS(session_name)), NULL); e_id = php_url_encode(PS(id), strlen(PS(id)), NULL); - smart_str_appends(&ncookie, COOKIE_SET_COOKIE); + smart_str_appends(&ncookie, "Set-Cookie: "); smart_str_appends(&ncookie, e_session_name); smart_str_appendc(&ncookie, '='); smart_str_appends(&ncookie, e_id); diff --git a/ext/standard/head.c b/ext/standard/head.c index 6ede184df26..0316903bc64 100644 --- a/ext/standard/head.c +++ b/ext/standard/head.c @@ -117,10 +117,10 @@ 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, "%s%s=deleted; expires=%s; Max-Age=0", COOKIE_SET_COOKIE, name, dt); + snprintf(cookie, len + 100, "Set-Cookie: %s=deleted; expires=%s; Max-Age=0", name, dt); efree(dt); } else { - snprintf(cookie, len + 100, "%s%s=%s", COOKIE_SET_COOKIE, name, value ? encoded_value : ""); + snprintf(cookie, len + 100, "Set-Cookie: %s=%s", name, value ? encoded_value : ""); if (expires > 0) { const char *p; char tsdelta[13]; diff --git a/ext/standard/head.h b/ext/standard/head.h index cb9a7f48230..59b1518676e 100644 --- a/ext/standard/head.h +++ b/ext/standard/head.h @@ -21,7 +21,6 @@ #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="