Skip to content

Commit

Permalink
fix(router/atc) properly escape ATC string literals
Browse files Browse the repository at this point in the history 
Fix FT-3257

Co-authored-by: Mayo <i@shoujo.io>
  • Loading branch information
@StarlightIbuki @mayocream
StarlightIbuki and mayocream authored Aug 25, 2022
1 parent 32ddcd5 commit 9ac1e40
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 53 deletions.
15 changes: 10 additions & 5 deletions kong/router/atc_compat.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ function _M._set_ngx(mock_ngx)
end


local function atc_escape_str(str)
return "\"" .. str:gsub([[\]], [[\\]]):gsub([["]], [[\"]]) .. "\""
end


local function gen_for_field(name, op, vals, val_transform)
if not vals then
return nil
Expand All @@ -145,8 +150,8 @@ local function gen_for_field(name, op, vals, val_transform)
for _, p in ipairs(vals) do
values_n = values_n + 1
local op = (type(op) == "string") and op or op(p)
values[values_n] = name .. " " .. op ..
" \"" .. (val_transform and val_transform(op, p) or p) .. "\""
values[values_n] = name .. " " .. op .. " " ..
atc_escape_str(val_transform and val_transform(op, p) or p)
end

if values_n > 0 then
Expand Down Expand Up @@ -225,7 +230,7 @@ local function get_atc(route)
end, route.paths, function(op, p)
if op == OP_REGEX then
-- Rust only recognize form '?P<>'
return sub(p, 2):gsub("?<", "?P<"):gsub("\\", "\\\\")
return sub(p, 2):gsub("?<", "?P<")
end

return normalize(p, true)
Expand All @@ -247,11 +252,11 @@ local function get_atc(route)
local value = ind
local op = OP_EQUAL
if ind:sub(1, 2) == "~*" then
value = ind:sub(3):gsub("\\", "\\\\")
value = ind:sub(3)
op = OP_REGEX
end

tb_insert(single_header, name .. " " .. op .. " \"" .. value:lower() .. "\"")
tb_insert(single_header, name .. " " .. op .. " " .. atc_escape_str(value:lower()))
end

tb_insert(headers, "(" .. tb_concat(single_header, " || ") .. ")")
Expand Down
156 changes: 108 additions & 48 deletions spec/01-unit/08-router_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3476,7 +3476,6 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
end)
end)


describe("#slash handling", function()
for i, line in ipairs(path_handling_tests) do
for j, test in ipairs(line:expand()) do
Expand Down Expand Up @@ -3571,6 +3570,66 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
end
end
end)

it("works with special characters('\"','\\')", function()
local use_case_routes = {
{
service = {
name = "service-invalid",
host = "example.org",
protocol = "http"
},
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
paths = { [[/\d]] },
},
},
{
service = {
name = "service-invalid",
host = "example.org",
protocol = "https"
},
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8102",
paths = { [[~/\d+"]] },
},
},
}

local router = assert(new_router(use_case_routes))
local _ngx = mock_ngx("GET", [[/\d]], { host = "domain.org" })
router._set_ngx(_ngx)
local match_t = router:exec()
assert.same(use_case_routes[1].route, match_t.route)

-- upstream_url_t
if flavor == "traditional" then
assert.equal("http", match_t.upstream_url_t.scheme)
end
assert.equal("example.org", match_t.upstream_url_t.host)
assert.equal(80, match_t.upstream_url_t.port)

-- upstream_uri
assert.is_nil(match_t.upstream_host) -- only when `preserve_host = true`
assert.equal([[/\d]], match_t.upstream_uri)

_ngx = mock_ngx("GET", [[/123"]], { host = "domain.org" })
router._set_ngx(_ngx)
match_t = router:exec()
assert.same(use_case_routes[2].route, match_t.route)

-- upstream_url_t
if flavor == "traditional" then
assert.equal("https", match_t.upstream_url_t.scheme)
end
assert.equal("example.org", match_t.upstream_url_t.host)
assert.equal(443, match_t.upstream_url_t.port)

-- upstream_uri
assert.is_nil(match_t.upstream_host) -- only when `preserve_host = true`
assert.equal([[/123"]], match_t.upstream_uri)
end)
end)


Expand Down Expand Up @@ -3938,62 +3997,63 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
end)
end
end)
end

describe("[both regex and prefix with regex_priority]", function()
local use_case, router

lazy_setup(function()
use_case = {
-- regex
{
service = service,
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
paths = {
"/.*"
},
hosts = {
"domain-1.org",
},
describe("[both regex and prefix with regex_priority]", function()
local use_case, router

lazy_setup(function()
use_case = {
-- regex
{
service = service,
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
paths = {
"/.*"
},
hosts = {
"domain-1.org",
},
},
-- prefix
{
service = service,
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8102",
paths = {
"/"
},
hosts = {
"domain-2.org",
},
regex_priority = 5
},
-- prefix
{
service = service,
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8102",
paths = {
"/"
},
hosts = {
"domain-2.org",
},
regex_priority = 5
},
{
service = service,
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8103",
paths = {
"/v1"
},
hosts = {
"domain-2.org",
},
},
{
service = service,
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8103",
paths = {
"/v1"
},
hosts = {
"domain-2.org",
},
},
}

router = assert(new_router(use_case))
end)
},
}

it("[prefix matching ignore regex_priority]", function()
local match_t = router:select("GET", "/v1", "domain-2.org")
assert.truthy(match_t)
assert.same(use_case[3].route, match_t.route)
end)
router = assert(new_router(use_case))
end)

it("[prefix matching ignore regex_priority]", function()
local match_t = router:select("GET", "/v1", "domain-2.org")
assert.truthy(match_t)
assert.same(use_case[3].route, match_t.route)
end)
end

end)

0 comments on commit 9ac1e40

Please sign in to comment.