Skip to content

Commit

Permalink
fix(router) fix trailing slash getting automatically appended
Browse files Browse the repository at this point in the history 
Fixes: #2211
  • Loading branch information
@bungle
bungle committed Apr 3, 2017
1 parent df49932 commit 29f172b
Show file tree
Hide file tree
Showing 6 changed files with 268 additions and 10 deletions.
1 change: 1 addition & 0 deletions .luacheckrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ max_line_length = false

globals = {
"_KONG",
"table.unpack",
}


Expand Down
19 changes: 17 additions & 2 deletions kong/core/router.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ local upper = string.upper
local lower = string.lower
local find = string.find
local fmt = string.format
local sub = string.sub
local tonumber = tonumber
local ipairs = ipairs
local pairs = pairs
Expand Down Expand Up @@ -600,8 +601,22 @@ function _M.new(apis)
end


if api_t.upstream.path then
new_uri = api_t.upstream.path .. new_uri
local upstream_path = api_t.upstream.path
if upstream_path then
if new_uri == "/" then
new_uri = upstream_path
else
new_uri = upstream_path .. (sub(upstream_path, -1) == "/" and sub(new_uri, 2) or new_uri)
end
end


local req_uri_slash = sub(uri, -1) == "/"
local new_uri_slash = sub(new_uri, -1) == "/"
if new_uri_slash and not req_uri_slash and new_uri ~= "/" then
new_uri = sub(new_uri, 1, -2)
elseif not new_uri_slash and req_uri_slash and uri ~= "/" then
new_uri = new_uri .. "/"
end


Expand Down
4 changes: 0 additions & 4 deletions kong/dao/schemas/apis.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ local function validate_upstream_url(value)
end
end

if parsed_url.path and string.sub(value, #value) == "/" then
return false, "Cannot end with a slash"
end

return true
end

Expand Down
7 changes: 3 additions & 4 deletions spec/01-unit/07-entities_schemas_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,14 @@ describe("Entities Schemas", function()
assert.equal("Supported protocols are HTTP and HTTPS", errors.upstream_url)
end)

it("should return error with final slash in upstream_url", function()
it("should not return error with final slash in upstream_url", function()
local valid, errors = validate_entity({
name = "mockbin",
upstream_url = "http://mockbin.com/",
hosts = { "mockbin.com" },
}, api_schema)
assert.is_false(valid)
assert.equal("Cannot end with a slash", errors.upstream_url)

assert.is_nil(errors)
assert.is_true(valid)
end)

it("should validate with upper case protocol", function()
Expand Down
202 changes: 202 additions & 0 deletions spec/02-integration/05-proxy/01-router_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,4 +387,206 @@ describe("Router", function()
assert.equal("fixture-api", res.headers["kong-api-name"])
end)
end)


describe("trailing slash", function()

setup(function()
insert_apis {
{
name = "strip-uri-1",
upstream_url = "http://localhost:9999/",
hosts = {
"localbin-1.com",
},
uris = {
"/",
"/foo/bar",
}
},
{
name = "strip-uri-2",
upstream_url = "http://localhost:9999/foo/bar",
hosts = {
"localbin-2.com",
},
uris = {
"/",
"/foo/bar",
}
},
{
name = "strip-uri-3",
upstream_url = "http://localhost:9999/foo/bar/",
hosts = {
"localbin-3.com",
},
uris = {
"/",
"/foo/bar",
}
},
{
name = "strip-uri-4",
strip_uri = true,
upstream_url = "http://localhost:9999/",
hosts = {
"localbin-4.com",
},
uris = {
"/",
"/foo/bar",
}
},
{
name = "strip-uri-5",
strip_uri = true,
upstream_url = "http://localhost:9999/foo/bar",
hosts = {
"localbin-5.com",
},
uris = {
"/",
"/foo/bar",
}
},
{
name = "strip-uri-6",
strip_uri = true,
upstream_url = "http://localhost:9999/foo/bar/",
hosts = {
"localbin-6.com",
},
uris = {
"/",
"/foo/bar",
}
},
{
name = "strip-uri-7",
strip_uri = false,
upstream_url = "http://localhost:9999/",
hosts = {
"localbin-7.com",
},
uris = {
"/",
"/foo/bar",
}
},
{
name = "strip-uri-8",
strip_uri = false,
upstream_url = "http://localhost:9999/foo/bar",
hosts = {
"localbin-8.com",
},
uris = {
"/",
"/foo/bar",
}
},
{
name = "strip-uri-9",
strip_uri = false,
upstream_url = "http://localhost:9999/foo/bar/",
hosts = {
"localbin-9.com",
},
uris = {
"/",
"/foo/bar",
}
},
}

assert(helpers.start_kong {
nginx_conf = "spec/fixtures/custom_nginx.template",
})
end)

teardown(function()
helpers.stop_kong()
end)

local function check(upstream_uri, request_uri, expected_uri, strip_uri)
return function()
local i = 1

if strip_uri == true then
i = i + 3
elseif strip_uri == false then
i = i + 6
end

if upstream_uri ~= "/" then
i = i + 1
if string.sub(upstream_uri, -1) == "/" then
i = i + 1
end
end

local res = assert(client:send {
method = "GET",
path = request_uri,
headers = {
["Host"] = "localbin-" .. i .. ".com",
}
})

local json = assert.res_status(200, res)
local data = cjson.decode(json)

assert.equal(expected_uri, data.vars.request_uri)
end
end

local checks = {
{ "/", "/", "/", nil },
{ "/", "/foo/bar", "/", nil },
{ "/", "/foo/bar/", "/", nil },
{ "/foo/bar", "/", "/foo/bar", nil },
{ "/foo/bar/", "/", "/foo/bar/", nil },
{ "/foo/bar", "/foo/bar", "/foo/bar", nil },
{ "/foo/bar/", "/foo/bar", "/foo/bar", nil },
{ "/foo/bar", "/foo/bar/", "/foo/bar/", nil },
{ "/foo/bar/", "/foo/bar/", "/foo/bar/", nil },
{ "/", "/", "/", true },
{ "/", "/foo/bar", "/", true },
{ "/", "/foo/bar/", "/", true },
{ "/foo/bar", "/", "/foo/bar", true },
{ "/foo/bar/", "/", "/foo/bar/", true },
{ "/foo/bar", "/foo/bar", "/foo/bar", true },
{ "/foo/bar/", "/foo/bar", "/foo/bar", true },
{ "/foo/bar", "/foo/bar/", "/foo/bar/", true },
{ "/foo/bar/", "/foo/bar/", "/foo/bar/", true },
{ "/", "/", "/", false },
{ "/", "/foo/bar", "/foo/bar", false },
{ "/", "/foo/bar/", "/foo/bar/", false },
{ "/foo/bar", "/", "/foo/bar", false },
{ "/foo/bar/", "/", "/foo/bar/", false },
{ "/foo/bar", "/foo/bar", "/foo/bar/foo/bar", false },
{ "/foo/bar/", "/foo/bar", "/foo/bar/foo/bar", false },
{ "/foo/bar", "/foo/bar/", "/foo/bar/foo/bar/", false },
{ "/foo/bar/", "/foo/bar/", "/foo/bar/foo/bar/", false },
}

local unpack = table.unpack or unpack

for _, args in ipairs(checks) do

local config = "(strip_uri = n/a)"

if args[4] == true then
config = "(strip_uri = on) "
elseif args[4] == false then
config = "(strip_uri = off)"
end

local space = string.sub(args[1], -1) == "/" and "" or " "

it(config .. " is not appended to upstream uri " .. args[1] .. space .. " when requesting " .. args[2],
check(unpack(args)))
end
end)
end)
45 changes: 45 additions & 0 deletions spec/fixtures/custom_nginx.template
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,51 @@ http {
return 200;
}

location / {
content_by_lua_block {
local cjson = require "cjson"
local var = ngx.var
local req = ngx.req

req.read_body()

local json = cjson.encode {
vars = {
uri = var.uri,
host = var.host,
hostname = var.hostname,
https = var.https,
scheme = var.scheme,
is_args = var.is_args,
server_addr = var.server_addr,
server_port = var.server_port,
server_name = var.server_name,
server_protocol = var.server_protocol,
remote_addr = var.remote_addr,
remote_port = var.remote_port,
realip_remote_addr = var.realip_remote_addr,
realip_remote_port = var.realip_remote_port,
binary_remote_addr = var.binary_remote_addr,
request = var.request,
request_uri = var.request_uri,
request_time = var.request_time,
request_length = var.request_length,
request_method = var.request_method,
bytes_received = var.bytes_received,
},
method = req.get_method(),
headers = req.get_headers(0),
uri_args = req.get_uri_args(0),
post_args = req.get_post_args(0),
http_version = req.http_version(),
}

ngx.status = 200
ngx.say(json)
ngx.exit(200)
}
}

location /headers-inspect {
content_by_lua_block {
local cjson = require "cjson"
Expand Down

0 comments on commit 29f172b

Please sign in to comment.