From 42e3002228a2e7c10ce45d34da39922d39cb5967 Mon Sep 17 00:00:00 2001 From: Satbek Turganbayev Date: Thu, 25 Jun 2020 16:40:24 +0300 Subject: [PATCH] Added ability to set and get cookie without escaping. `resp:setcookie` implicitly escaped cookie values. Added ability to set cookie without any escaping `resp:setcookie('name', 'value', {raw = true})`. Also added escaping for cookie path, and changed escaping algorithm according to https://tools.ietf.org/html/rfc6265. `req:cookie` implicitly unescaped cookie values. Added ability to get cookie without unescaping `req:cookie('name', {raw = true})`. --- README.md | 4 +- http/router/request.lua | 9 +- http/router/response.lua | 52 ++++++++- http/utils.lua | 22 ++-- test/integration/request_test.lua | 79 +++++++++----- test/unit/setcookie_test.lua | 176 ++++++++++++++++++++++++++++++ 6 files changed, 297 insertions(+), 45 deletions(-) create mode 100644 test/unit/setcookie_test.lua diff --git a/README.md b/README.md index 0c82f92..59b2934 100644 --- a/README.md +++ b/README.md @@ -204,7 +204,7 @@ end | `req:post_param(name)` | returns a single POST request a parameter value. If `name` is `nil`, returns all parameters as a Lua table. | | `req:query_param(name)` | returns a single GET request parameter value. If `name` is `nil`, returns a Lua table with all arguments. | | `req:param(name)` | any request parameter, either GET or POST. | -| `req:cookie(name)` | to get a cookie in the request. | +| `req:cookie(name, {raw = true})` | to get a cookie in the request. if `raw` option was set then cookie will not be unescaped, otherwise cookie's value will be unescaped | | `req:stash(name[, value])` | **NOTE**: currently not supported inside middleware handlers. Get or set a variable "stashed" when dispatching a route. | | `req:url_for(name, args, query)` | returns the route's exact URL. | `req:redirect_to` | create a **Response** object with an HTTP redirect. @@ -221,7 +221,7 @@ 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/router/request.lua b/http/router/request.lua index 8854978..717f8bb 100644 --- a/http/router/request.lua +++ b/http/router/request.lua @@ -123,14 +123,19 @@ local function param(self, name) return utils.extend(post, query, false) end -local function cookie(self, cookiename) +local function cookie(self, cookiename, options) + options = options or {} if self:header('cookie') == nil then return nil end for k, v in string.gmatch( self:header('cookie'), "([^=,; \t]+)=([^,; \t]+)") do if k == cookiename then - return utils.uri_unescape(v) + if not options.raw then + return utils.uri_unescape(v) + else + return v + end end end return nil diff --git a/http/router/response.lua b/http/router/response.lua index ace53d3..159b858 100644 --- a/http/router/response.lua +++ b/http/router/response.lua @@ -28,7 +28,43 @@ local function expires_str(str) return os.date(fmt, gmtnow + diff) end -local function setcookie(resp, cookie) +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_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] = utils.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 setcookie(resp, cookie, options) + options = options or {} + local name = cookie.name local value = cookie.value @@ -39,10 +75,20 @@ local function setcookie(resp, cookie) error('cookie.value is undefined') end - local str = utils.sprintf('%s=%s', name, utils.uri_escape(value)) + if not options.raw then + value = escape_value(value) + end + + local str = utils.sprintf('%s=%s', name, value) + if cookie.path ~= nil then - str = utils.sprintf('%s;path=%s', str, cookie.path) + if options.raw then + str = utils.sprintf('%s;path=%s', str, cookie.path) + else + str = utils.sprintf('%s;path=%s', str, escape_path(cookie.path)) + end end + if cookie.domain ~= nil then str = utils.sprintf('%s;domain=%s', str, cookie.domain) end diff --git a/http/utils.lua b/http/utils.lua index c4a4245..e69b930 100644 --- a/http/utils.lua +++ b/http/utils.lua @@ -36,6 +36,14 @@ local function extend(tbl, tblu, raise) return res 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 uri_unescape(str, unescape_plus_sign) local res = {} if type(str) == 'table' then @@ -47,11 +55,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 @@ -63,11 +67,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 @@ -80,4 +80,6 @@ return { extend = extend, uri_unescape = uri_unescape, uri_escape = uri_escape, + escape_char = escape_char, + unescape_char = unescape_char, } diff --git a/test/integration/request_test.lua b/test/integration/request_test.lua index 919fcab..cd0b859 100755 --- a/test/integration/request_test.lua +++ b/test/integration/request_test.lua @@ -26,6 +26,57 @@ g.test_redirect_to = function() t.assert_equals(r.body, "OK") end +g.test_get_cookie = function() + g.router:route({path = '/receive_cookie'}, function(req) + local foo = req:cookie('foo') + local baz = req:cookie('baz') + return req:render({ + text = ('foo=%s; baz=%s'):format(foo, baz) + }) + end) + + local r = http_client.get(helper.base_uri .. 'receive_cookie', { + headers = { + cookie = 'foo=f%3Bf; baz=f%5Cf', + } + }) + + t.assert_equals(r.status, 200, 'status') + t.assert_equals(r.body, 'foo=f;f; baz=f\\f', 'body') +end + +g.test_get_cookie_raw = function() + g.router:route({path = '/receive_cookie_raw'}, function(req) + local foo = req:cookie('foo', {raw = true}) + local baz = req:cookie('baz', {raw = true}) + return req:render({ + text = ('foo=%s; baz=%s'):format(foo, baz) + }) + end) + + local r = http_client.get(helper.base_uri .. 'receive_cookie_raw', { + headers = { + cookie = 'foo=f%3Bf; baz=f%5Cf', + } + }) + + t.assert_equals(r.status, 200, 'status') + t.assert_equals(r.body, 'foo=f%3Bf; baz=f%5Cf', 'body') +end + +g.test_set_cookie = function() + g.router:route({path = '/cookie'}, function(req) + local resp = req:render({text = ''}) + resp:setcookie({ name = 'test', value = 'tost', + expires = '+1y', path = '/abc' }) + resp:setcookie({ name = 'xxx', value = 'yyy' }) + return resp + end) + local r = http_client.get(helper.base_uri .. 'cookie') + t.assert_equals(r.status, 200, 'status') + t.assert(r.headers['set-cookie'] ~= nil, "header") +end + g.test_server_requests = function() local r = http_client.get(helper.base_uri .. 'test') t.assert_equals(r.status, 200, '/test code') @@ -152,34 +203,6 @@ g.test_server_requests = function() t.assert_equals(r.headers['transfer-encoding'], 'chunked', 'chunked headers') t.assert_equals(r.body, 'chunkedencodingt\r\nest', 'chunked body') - -- get cookie - g.router:route({path = '/receive_cookie'}, function(req) - local foo = req:cookie('foo') - local baz = req:cookie('baz') - return req:render({ - text = ('foo=%s; baz=%s'):format(foo, baz) - }) - end) - r = http_client.get(helper.base_uri .. 'receive_cookie', { - headers = { - cookie = 'foo=bar; baz=feez', - } - }) - t.assert_equals(r.status, 200, 'status') - t.assert_equals(r.body, 'foo=bar; baz=feez', 'body') - - -- cookie - g.router:route({path = '/cookie'}, function(req) - local resp = req:render({text = ''}) - resp:setcookie({ name = 'test', value = 'tost', - expires = '+1y', path = '/abc' }) - resp:setcookie({ name = 'xxx', value = 'yyy' }) - return resp - end) - r = http_client.get(helper.base_uri .. 'cookie') - t.assert_equals(r.status, 200, 'status') - t.assert(r.headers['set-cookie'] ~= nil, "header") - -- request object with GET method g.router:route({path = '/check_req_properties'}, function(req) diff --git a/test/unit/setcookie_test.lua b/test/unit/setcookie_test.lua new file mode 100644 index 0000000..dca9da0 --- /dev/null +++ b/test/unit/setcookie_test.lua @@ -0,0 +1,176 @@ +local t = require('luatest') +local g = t.group() + +local response = require('http.router.response') + +local function get_object() + return setmetatable({}, response.metatable) +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