From 42e3002228a2e7c10ce45d34da39922d39cb5967 Mon Sep 17 00:00:00 2001
From: Satbek Turganbayev <s.turganbayev@corp.mail.ru>
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
+    -- <any CHAR except CTLs or ";">
+    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