diff --git a/CHANGELOG.md b/CHANGELOG.md index a660ed1f5..795bc8a4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Set request timeout for OIDC query. [PR #1601](https://github.com/3scale/APIcast/pull/1601) [THREESCALE-8006](https://redhat.atlassian.net/browse/THREESCALE-8006) - Set upstream metrics when sending request via proxy. [PR #1598](https://github.com/3scale/APIcast/pull/1598) [THREESCALE-10571](https://redhat.atlassian.net/browse/THREESCALE-15071) - Certificate is never verified in API request for https backend via proxy [PR #1573](https://github.com/3scale/APIcast/pull/1568) [THREESCALE-11944](https://redhat.atlassian.net/browse/THREESCALE-11944) +- Routing Policy: keep an encoded "?" (`%3F`) in the path when the backend has a path [PR #1613](https://github.com/3scale/APIcast/pull/1613) [Issue #1612](https://github.com/3scale/APIcast/issues/1612) ### Added - Update APIcast schema manifest [PR #1550](https://github.com/3scale/APIcast/pull/1550) diff --git a/gateway/src/apicast/upstream.lua b/gateway/src/apicast/upstream.lua index b76c196b4..63f791baf 100644 --- a/gateway/src/apicast/upstream.lua +++ b/gateway/src/apicast/upstream.lua @@ -8,6 +8,7 @@ local setmetatable = setmetatable local str_format = string.format +local str_find = string.find local resty_resolver = require('resty.resolver') local resty_url = require('resty.url') @@ -121,7 +122,17 @@ function _M:set_path(path) end function _M:append_path(path) - local tmp_path, tmp_query = url_helper.split_path(path) + local tmp_path, tmp_query + + -- ngx.var.uri is decoded, so a "?" in it was sent as "%3F" and is part of a + -- path segment. A path rendered from it must not be split there. + local uri = ngx.var.uri + if uri and str_find(uri, '?', 1, true) then + tmp_path = path + else + tmp_path, tmp_query = url_helper.split_path(path) + end + if not self.uri.path then self.uri.path = "/" end diff --git a/spec/upstream_spec.lua b/spec/upstream_spec.lua index b24b8f4c3..51374af95 100644 --- a/spec/upstream_spec.lua +++ b/spec/upstream_spec.lua @@ -53,6 +53,8 @@ describe('Upstream', function() end) describe(':append_path', function() + before_each(function() ngx.var = { } end) + it('return valid path when is not set', function() local up = Upstream.new('http://host:8090') up:append_path("/test") @@ -71,6 +73,22 @@ describe('Upstream', function() assert.same(up.uri.path, "/test") end) + it('keeps an encoded "?" of the request path in the path', function() + ngx.var = { uri = '/REST/svc/files/a?b' } + local up = Upstream.new('http://host:8090/REST/svc') + up:append_path("/files/a?b") + assert.same(up.uri.path, "/REST/svc/files/a?b") + assert.is_nil(up.uri.query) + end) + + it('does not use the query of the rendered path', function() + ngx.var = { uri = '/REST/svc/files/a' } + local up = Upstream.new('http://host:8090/REST/svc') + up:append_path("/files/a?b") + assert.same(up.uri.path, "/REST/svc/files/a") + assert.is_nil(up.uri.query) + end) + end) local function stub_ngx_request() diff --git a/t/apicast-policy-routing.t b/t/apicast-policy-routing.t index 86745fb21..6d011747e 100644 --- a/t/apicast-policy-routing.t +++ b/t/apicast-policy-routing.t @@ -2586,3 +2586,54 @@ content_by_lua_block { } --- no_error_log [error] + + +=== TEST 37: Encoded "?" in the path is kept when the backend has a path +The rule is the one 3scale generates for a Backend used with a path. A "%3F" +in the request path must reach the upstream as "%3F" and not cut the path. +--- configuration +{ + "services": [ + { + "id": 42, + "proxy": { + "policy_chain": [ + { + "name": "apicast.policy.routing", + "configuration": { + "rules": [ + { + "url": "http://test:$TEST_NGINX_SERVER_PORT/REST/svc", + "replace_path": "{{uri | remove_first: '/REST/svc'}}", + "condition": { + "operations": [ + { + "match": "path", + "op": "matches", + "value": "^(/REST/svc/.*|/REST/svc/?)" + } + ] + } + } + ] + } + } + ] + } + } + ] +} +--- upstream + location ~ / { + content_by_lua_block { + ngx.say(ngx.var.request_uri) + } + } +--- request eval +["GET /REST/svc/files/a%3Fb", "GET /REST/svc/files/a%3Fb?x=1", "GET /REST/svc/files/a?x=1"] +--- response_body eval +["/REST/svc/files/a%3Fb\n", "/REST/svc/files/a%3Fb?x=1\n", "/REST/svc/files/a?x=1\n"] +--- error_code eval +[200, 200, 200] +--- no_error_log +[error]