From 920f526aabd36613a3a8d14daf42b4d6806aef08 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Tue, 26 Oct 2021 22:08:52 +0300 Subject: [PATCH] Add option to set cookie without escaping and change escaping algorithm Method resp:setcookie() implicitly escapes cookie values. Commit adds ability to set cookie without any escaping with option 'raw': resp:setcookie('name', 'value', { raw = true })` Also added escaping for cookie path, and changed escaping algorithm according to RFC 6265 "HTTP State Management Mechanism", see [1]. This change was added as a part of http v2 support in commit 'Added ability to set and get cookie without escaping' (42e3002228a2e7c10ce45d34da39922d39cb5967) and later reverted in scope of ticket with discard v2. 1. https://tools.ietf.org/html/rfc6265 Follows up #126 Part of #134 --- CHANGELOG.md | 1 + README.md | 5 +- http/server.lua | 76 +++++-- .../integration/http_server_requests_test.lua | 48 ++++ test/unit/http_setcookie_test.lua | 205 ++++++++++++++++++ 5 files changed, 319 insertions(+), 16 deletions(-) create mode 100644 test/unit/http_setcookie_test.lua diff --git a/CHANGELOG.md b/CHANGELOG.md index 55966fb..cc0990c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add editorconfig to configure indentation. - Add luacheck integration. - Add option to get cookie without escaping. +- Add option to set cookie without escaping and change escaping algorithm. ### Fixed diff --git a/README.md b/README.md index c896864..cdb9414 100644 --- a/README.md +++ b/README.md @@ -240,8 +240,9 @@ end * `resp.status` - HTTP response code. * `resp.headers` - a Lua table with normalized headers. * `resp.body` - response body (string|table|wrapped\_iterator). -* `resp:setcookie({ name = 'name', value = 'value', path = '/', expires = '+1y', domain = 'example.com'))` - - adds `Set-Cookie` headers to `resp.headers`. +* `resp:setcookie({ name = 'name', value = 'value', path = '/', expires = '+1y', domain = 'example.com'}, {raw = true})` - + adds `Set-Cookie` headers to `resp.headers`, if `raw` option was set then cookie will not be escaped, + otherwise cookie's value and path will be escaped ### Examples diff --git a/http/server.lua b/http/server.lua index c02db0c..2b7e396 100644 --- a/http/server.lua +++ b/http/server.lua @@ -23,6 +23,49 @@ local function sprintf(fmt, ...) return string.format(fmt, ...) end +local function valid_cookie_value_byte(byte) + -- https://tools.ietf.org/html/rfc6265#section-4.1.1 + -- US-ASCII characters excluding CTLs, whitespace DQUOTE, comma, semicolon, + -- and backslash. + return 32 < byte and byte < 127 and byte ~= string.byte('"') and + byte ~= string.byte(",") and byte ~= string.byte(";") and byte ~= string.byte("\\") +end + +local function valid_cookie_path_byte(byte) + -- https://tools.ietf.org/html/rfc6265#section-4.1.1 + -- + return 32 <= byte and byte < 127 and byte ~= string.byte(";") +end + +local function escape_char(char) + return string.format('%%%02X', string.byte(char)) +end + +local function unescape_char(char) + return string.char(tonumber(char, 16)) +end + +local function escape_string(str, byte_filter) + local result = {} + for i = 1, str:len() do + local char = str:sub(i,i) + if byte_filter(string.byte(char)) then + result[i] = char + else + result[i] = escape_char(char) + end + end + return table.concat(result) +end + +local function escape_value(cookie_value) + return escape_string(cookie_value, valid_cookie_value_byte) +end + +local function escape_path(cookie_path) + return escape_string(cookie_path, valid_cookie_path_byte) +end + local function uri_escape(str) local res = {} if type(str) == 'table' then @@ -30,11 +73,7 @@ local function uri_escape(str) table.insert(res, uri_escape(v)) end else - res = string.gsub(str, '[^a-zA-Z0-9_]', - function(c) - return string.format('%%%02X', string.byte(c)) - end - ) + res = string.gsub(str, '[^a-zA-Z0-9_]', escape_char) end return res end @@ -50,11 +89,7 @@ local function uri_unescape(str, unescape_plus_sign) str = string.gsub(str, '+', ' ') end - res = string.gsub(str, '%%([0-9a-fA-F][0-9a-fA-F])', - function(c) - return string.char(tonumber(c, 16)) - end - ) + res = string.gsub(str, '%%([0-9a-fA-F][0-9a-fA-F])', unescape_char) end return res end @@ -265,7 +300,8 @@ local function expires_str(str) return os.date(fmt, gmtnow + diff) end -local function setcookie(resp, cookie) +local function setcookie(resp, cookie, options) + options = options or {} local name = cookie.name local value = cookie.value @@ -276,9 +312,16 @@ local function setcookie(resp, cookie) error('cookie.value is undefined') end - local str = sprintf('%s=%s', name, uri_escape(value)) + if not options.raw then + value = escape_value(value) + end + local str = sprintf('%s=%s', name, value) if cookie.path ~= nil then - str = sprintf('%s;path=%s', str, cookie.path) + local cookie_path = cookie.path + if not options.raw then + cookie_path = escape_path(cookie.path) + end + str = sprintf('%s;path=%s', str, cookie_path) end if cookie.domain ~= nil then str = sprintf('%s;domain=%s', str, cookie.domain) @@ -1280,7 +1323,12 @@ local exports = { } return self - end + end, + + internal = { + response_mt = response_mt, + request_mt = request_mt, + } } return exports diff --git a/test/integration/http_server_requests_test.lua b/test/integration/http_server_requests_test.lua index f0d3e85..55db82d 100644 --- a/test/integration/http_server_requests_test.lua +++ b/test/integration/http_server_requests_test.lua @@ -252,6 +252,54 @@ g.test_get_escaped_cookie = function() t.assert_equals(r.body, 'name=' .. str_non_escaped, 'body with escaped cookie') end +-- Set escaped cookie (Günter -> G%C3%BCnter). +g.test_set_escaped_cookie = function(g) + local str_escaped = 'G%C3%BCnter' + local str_non_escaped = 'Günter' + + local httpd = g.httpd + httpd:route({ + path = '/cookie' + }, function(req) + local resp = req:render({ + text = '' + }) + resp:setcookie({ + name = 'name', + value = str_non_escaped + }) + return resp + end) + + local r = http_client.get(helpers.base_uri .. '/cookie') + t.assert_equals(r.status, 200, 'response status') + t.assert_equals(r.headers['set-cookie'], 'name=' .. str_escaped, 'header with escaped cookie') +end + +-- Set raw cookie (Günter -> Günter). +g.test_set_raw_cookie = function(g) + local cookie = 'Günter' + local httpd = g.httpd + httpd:route({ + path = '/cookie' + }, function(req) + local resp = req:render({ + text = '' + }) + resp:setcookie({ + name = 'name', + value = cookie + }, { + raw = true + }) + return resp + end) + + local r = http_client.get(helpers.base_uri .. '/cookie') + t.assert_equals(r.status, 200, 'response status') + t.assert_equals(r.headers['set-cookie'], 'name=' .. cookie, 'header with raw cookie') +end + -- Request object methods. g.test_request_object_methods = function() local httpd = g.httpd diff --git a/test/unit/http_setcookie_test.lua b/test/unit/http_setcookie_test.lua new file mode 100644 index 0000000..ab5adcd --- /dev/null +++ b/test/unit/http_setcookie_test.lua @@ -0,0 +1,205 @@ +local t = require('luatest') + +local http_server = require('http.server') + +local g = t.group() + +local function get_object() + return setmetatable({}, http_server.internal.response_mt) +end + +g.test_values_escaping = function() + local test_table = { + whitespace = { + value = "f f", + result = 'f%20f', + }, + dquote = { + value = 'f"f', + result = 'f%22f', + }, + comma = { + value = "f,f", + result = "f%2Cf", + }, + semicolon = { + value = "f;f", + result = "f%3Bf", + }, + backslash = { + value = "f\\f", + result = "f%5Cf", + }, + unicode = { + value = "fюf", + result = "f%D1%8Ef" + }, + unprintable_ascii = { + value = string.char(15), + result = "%0F" + } + } + + for byte = 33, 126 do + if byte ~= string.byte('"') and + byte ~= string.byte(",") and + byte ~= string.byte(";") and + byte ~= string.byte("\\") then + test_table[byte] = { + value = "f" .. string.char(byte) .. "f", + result = "f" .. string.char(byte) .. "f", + } + end + end + + for case_name, case in pairs(test_table) do + local resp = get_object() + resp:setcookie({ + name='name', + value = case.value + }) + t.assert_equals(resp.headers['set-cookie'], { + "name=" .. case.result + }, case_name) + end +end + +g.test_values_raw = function() + local test_table = {} + for byte = 0, 127 do + test_table[byte] = { + value = "f" .. string.char(byte) .. "f", + result = "f" .. string.char(byte) .. "f", + } + end + + test_table.unicode = { + value = "fюf", + result = "fюf" + } + + for case_name, case in pairs(test_table) do + local resp = get_object() + resp:setcookie({ + name='name', + value = case.value + }, { + raw = true + }) + t.assert_equals(resp.headers['set-cookie'], { + "name=" .. case.result + }, case_name) + end +end + +g.test_path_escaping = function() + local test_table = { + semicolon = { + path = "f;f", + result = "f%3Bf", + }, + unicode = { + path = "fюf", + result = "f%D1%8Ef" + }, + unprintable_ascii = { + path = string.char(15), + result = "%0F" + } + } + + for byte = 32, 126 do + if byte ~= string.byte(";") then + test_table[byte] = { + path = "f" .. string.char(byte) .. "f", + result = "f" .. string.char(byte) .. "f", + } + end + end + + for case_name, case in pairs(test_table) do + local resp = get_object() + resp:setcookie({ + name='name', + value = 'value', + path = case.path + }) + t.assert_equals(resp.headers['set-cookie'], { + "name=value;" .. 'path=' .. case.result + }, case_name) + end +end + +g.test_path_raw = function() + local test_table = {} + for byte = 0, 127 do + test_table[byte] = { + path = "f" .. string.char(byte) .. "f", + result = "f" .. string.char(byte) .. "f", + } + end + + test_table.unicode = { + path = "fюf", + result = "fюf" + } + + for case_name, case in pairs(test_table) do + local resp = get_object() + resp:setcookie({ + name='name', + value = 'value', + path = case.path + }, { + raw = true + }) + t.assert_equals(resp.headers['set-cookie'], { + "name=value;" .. 'path=' .. case.result + }, case_name) + end +end + +g.test_set_header = function() + local test_table = { + name_value = { + cookie = { + name = 'name', + value = 'value' + }, + result = {"name=value"}, + }, + name_value_path = { + cookie = { + name = 'name', + value = 'value', + path = 'path' + }, + result = {"name=value;path=path"}, + }, + name_value_path_domain = { + cookie = { + name = 'name', + value = 'value', + path = 'path', + domain = 'domain', + }, + result = {"name=value;path=path;domain=domain"}, + }, + name_value_path_domain_expires = { + cookie = { + name = 'name', + value = 'value', + path = 'path', + domain = 'domain', + expires = 'expires' + }, + result = {"name=value;path=path;domain=domain;expires=expires"}, + }, + } + + for case_name, case in pairs(test_table) do + local resp = get_object() + resp:setcookie(case.cookie) + t.assert_equals(resp.headers["set-cookie"], case.result, case_name) + end +end