Skip to content

Commit

Permalink
fix(router) preserve upstream_url path component if exists
Browse files Browse the repository at this point in the history
When an API's `upstream_url` has a path component, we want the upstream
request to include it as well.

Thanks @javeme for the report.

Fix Kong#2073
Replace Kong#2074 (incomplete)
  • Loading branch information
thibaultcha committed Feb 11, 2017
1 parent 4ebaa6e commit 47960e7
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 6 deletions.
29 changes: 23 additions & 6 deletions kong/core/router.lua
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ local function marshall_api(api)
upstream_scheme = nil,
upstream_host = nil,
upstream_port = nil,
upstream_path = nil,
}


Expand Down Expand Up @@ -186,8 +187,9 @@ local function marshall_api(api)
local parsed = url.parse(api.upstream_url)

api_t.upstream_scheme = parsed.scheme
api_t.upstream_host = parsed.host
api_t.upstream_port = tonumber(parsed.port)
api_t.upstream_host = parsed.host
api_t.upstream_port = tonumber(parsed.port)
api_t.upstream_path = parsed.path

if not api_t.upstream_port then
if parsed.scheme == "https" then
Expand Down Expand Up @@ -583,6 +585,7 @@ function _M.new(apis)
function self.exec(ngx)
local method = ngx.req.get_method()
local uri = ngx.var.uri
local new_uri = uri
local host_header
local headers

Expand All @@ -605,8 +608,22 @@ function _M.new(apis)


if api_t.strip_uri_regex then
local stripped_uri = re_sub(uri, api_t.strip_uri_regex, "/$1", "jo")
ngx.req.set_uri(stripped_uri)
local err
new_uri, err = re_sub(uri, api_t.strip_uri_regex, "/$1", "jo")
if not new_uri then
log(ERR, "could not strip URI: ", err)
return
end
end


if api_t.upstream_path then
new_uri = api_t.upstream_path .. new_uri
end


if new_uri ~= uri then
ngx.req.set_uri(new_uri)
end


Expand All @@ -624,8 +641,8 @@ function _M.new(apis)
end


return api_t.api, api_t.upstream_scheme, api_t.upstream_host,
api_t.upstream_port, host_header
return api_t.api, api_t.upstream_scheme, api_t.upstream_host,
api_t.upstream_port, host_header, api_t.upstream_path
end


Expand Down
17 changes: 17 additions & 0 deletions spec/01-unit/12-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,23 @@ describe("Router", function()
assert.equal(443, upstream_port)
end)

it("parses path component from upstream_url property", function()
local use_case_apis = {
{
name = "api-1",
uris = { "/my-api" },
upstream_url = "http://httpbin.org/get",
}
}

local router = assert(Router.new(use_case_apis))

local _ngx = mock_ngx("GET", "/my-api", {})
local api, _, _, _, _, upstream_path = router.exec(_ngx)
assert.same(use_case_apis[1], api)
assert.equal("/get", upstream_path)
end)

it("parses upstream_url port", function()
local use_case_apis = {
{
Expand Down
36 changes: 36 additions & 0 deletions spec/02-integration/05-proxy/01-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,18 @@ describe("Router", function()
methods = { "POST", "PUT" },
uris = { "/post", "/put" },
strip_uri = false,
},
{
name = "api-3",
upstream_url = "http://httpbin.org/status",
uris = { "/httpbin" },
strip_uri = true,
},
{
name = "api-4",
upstream_url = "http://httpbin.org/basic-auth",
uris = { "/user" },
strip_uri = false,
}
}

Expand Down Expand Up @@ -111,6 +123,30 @@ describe("Router", function()
assert.response(res).has_status(200)
assert.equal("api-1", res.headers["kong-api-name"])
end)

describe("API with a path component in its upstream_url", function()
it("with strip_uri = true", function()
local res = assert(client:send {
method = "GET",
path = "/httpbin/201",
headers = { ["kong-debug"] = 1 },
})

assert.res_status(201, res)
assert.equal("api-3", res.headers["kong-api-name"])
end)
end)

it("with strip_uri = false", function()
local res = assert(client:send {
method = "GET",
path = "/user/passwd",
headers = { ["kong-debug"] = 1 },
})

assert.res_status(401, res)
assert.equal("api-4", res.headers["kong-api-name"])
end)
end)

describe("invalidation", function()
Expand Down

0 comments on commit 47960e7

Please sign in to comment.