Skip to content

Commit

Permalink
Fix truncated cookie
Browse files Browse the repository at this point in the history
Previously, Max-Age was added to the cookie, but cookie_size wasn't
updated to account for the additional string size. This lead to the data
being truncated in cases where everything was set.

This change updates the cookie_size based on the length of max_age and
then increases the size of cookie to the increased cookie_size.
  • Loading branch information
stnguyen90 committed Dec 13, 2023
1 parent fc5d7d2 commit af1a66a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
33 changes: 30 additions & 3 deletions ext-src/swoole_http_response.cc
Original file line number Diff line number Diff line change
Expand Up @@ -987,9 +987,6 @@ static void php_swoole_http_response_cookie(INTERNAL_FUNCTION_PARAMETERS, const
RETURN_FALSE;
}

size_t cookie_size = name_len /* + value_len */ + path_len + domain_len + 100;
char *cookie = nullptr, *date = nullptr;

if (name_len > 0 && strpbrk(name, "=,; \t\r\n\013\014") != nullptr) {
php_swoole_error(E_WARNING, "Cookie names can't contain any of the following '=,; \\t\\r\\n\\013\\014'");
RETURN_FALSE;
Expand All @@ -999,6 +996,36 @@ static void php_swoole_http_response_cookie(INTERNAL_FUNCTION_PARAMETERS, const
RETURN_FALSE;
}

char *cookie = nullptr, *date = nullptr;
size_t cookie_size = name_len + 1; // add 1 for null char
cookie_size += 50; // strlen("; expires=Fri, 31-Dec-9999 23:59:59 GMT; Max-Age=0")
if (value_len == 0) {
cookie_size += 8; // strlen("=deleted")
}
if (expires > 0) {
// Max-Age will be no longer than 12 digits since the
// maximum expires is Fri, 31-Dec-9999 23:59:59 GMT.
cookie_size += 11;
}
if (path_len > 0) {
cookie_size += path_len + 7; // strlen("; path=")
}
if (domain_len > 0) {
cookie_size += domain_len + 9; // strlen("; domain=")
}
if (secure) {
cookie_size += 8; // strlen("; secure")
}
if (httponly) {
cookie_size += 10; // strlen("; httponly")
}
if (samesite_len > 0) {
cookie_size += samesite_len + 11; // strlen("; samesite=")
}
if (priority_len > 0) {
cookie_size += priority_len + 11; // strlen("; priority=")
}

if (value_len == 0) {
cookie = (char *) emalloc(cookie_size);
date = php_swoole_format_date((char *) ZEND_STRL("D, d-M-Y H:i:s T"), 1, 0);
Expand Down
14 changes: 13 additions & 1 deletion tests/swoole_http_server/max-age.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ $pm->parentFunc = function () use ($pm) {
$uri = "http://127.0.0.1:{$pm->getFreePort()}";
$cookies = httpRequest($uri)['set_cookie_headers'];

var_dump(strpos($cookies[0], 'test=123456789') !== false);
var_dump(strpos($cookies[0], 'expires='.date('D, d-M-Y H:i:s \G\M\T', time() + 3600)) !== false);
var_dump(strpos($cookies[0], 'Max-Age=3600') !== false);
var_dump(strpos($cookies[0], 'path=/') !== false);
var_dump(strpos($cookies[0], 'domain=example.com') !== false);
var_dump(strpos($cookies[0], 'secure') !== false);
var_dump(strpos($cookies[0], 'httponly') !== false);
var_dump(strpos($cookies[0], 'samesite=None') !== false);
var_dump(strpos($cookies[1], 'test=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT') !== false);
var_dump(strpos($cookies[1], 'Max-Age=0') !== false);
});
Expand All @@ -29,7 +35,7 @@ $pm->childFunc = function () use ($pm) {
});

$http->on('request', function (Swoole\Http\Request $request, Swoole\Http\Response $response) use ($pm) {
$response->cookie('test', '123456789', time() + 3600);
$response->cookie('test', '123456789', time() + 3600, '/', 'example.com', true, true, 'None');
$response->cookie('test', '');
$response->end();
});
Expand All @@ -43,4 +49,10 @@ bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
DONE

0 comments on commit af1a66a

Please sign in to comment.