diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d8116e31..0788fb636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix issues when OPENTRACING_FORWARD_HEADER was set [PR #1109](https://github.com/3scale/APIcast/pull/1109), [THREESCALE-1660](https://issues.jboss.org/browse/THREESCALE-1660) - New TLS termination policy [PR #1108](https://github.com/3scale/APIcast/pull/1108), [THREESCALE-2898](https://issues.jboss.org/browse/THREESCALE-2898) - Fix exception on rate limit policy when window was set as 0. [PR #1113](https://github.com/3scale/APIcast/pull/1108), [THREESCALE-3382](https://issues.jboss.org/browse/THREESCALE-3382) +- Fix issues with escaped characters in uri [THREESCALE-3468](https://issues.jboss.org/browse/THREESCALE-3468) [PR #1123](https://github.com/3scale/APIcast/pull/1123) ## [3.6.0] - 2019-08-30 diff --git a/gateway/src/apicast/proxy.lua b/gateway/src/apicast/proxy.lua index db8150980..285b09d0b 100644 --- a/gateway/src/apicast/proxy.lua +++ b/gateway/src/apicast/proxy.lua @@ -13,6 +13,7 @@ local backend_cache_handler = require('apicast.backend.cache_handler') local Usage = require('apicast.usage') local errors = require('apicast.errors') local Upstream = require('apicast.upstream') +local escape = require("resty.http.uri_escape") local assert = assert local type = type @@ -242,7 +243,12 @@ function _M:rewrite(service, context) return errors.no_credentials(service) end - local usage, matched_rules = service:get_usage(ngx.req.get_method(), ngx.var.uri) + -- URI need to be escaped to be able to match values with special characters + -- (like spaces), request_uri is the original one, but rewrite_uri can modify + -- the value and mapping rule will not match. + -- Example: if URI is `/foo /bar` it will be translated to `/foo%20/bar` + local target_uri = escape.escape_uri(ngx.var.uri) + local usage, matched_rules = service:get_usage(ngx.req.get_method(), target_uri) local cached_key = { service.id } -- remove integer keys for serialization diff --git a/gateway/src/resty/http/uri_escape.lua b/gateway/src/resty/http/uri_escape.lua new file mode 100644 index 000000000..977dfc13a --- /dev/null +++ b/gateway/src/resty/http/uri_escape.lua @@ -0,0 +1,36 @@ + +local ffi = require "ffi" + +local C = ffi.C +local ffi_str = ffi.string + +ffi.cdef[[ + uintptr_t ngx_escape_uri(unsigned char *dst, unsigned char *src, size_t size, unsigned int type); +]] + +local ngx_escape_uri = C.ngx_escape_uri + +local _M = { } + + +function _M.escape_uri(source_uri) + if not source_uri then + return "" + end + + local source_uri_len = #source_uri + + local source_str = ffi.new("unsigned char[?]", source_uri_len + 1) + ffi.copy(source_str, source_uri) + + -- If destination is NUL ngx_escape_uri returns the number of characters that + -- are going to be escaped, need to calculate first to make sure to + -- allocate the right amount of memory. + local escape_len = 2 * ngx_escape_uri(nil, source_str, source_uri_len, 0) + + local dst = ffi.new("unsigned char[?]", source_uri_len + 1 + tonumber(escape_len)) + ngx_escape_uri(dst, source_str, source_uri_len, 0) + return ffi_str(dst) +end + +return _M diff --git a/spec/resty/http/uri_escape_spec.lua b/spec/resty/http/uri_escape_spec.lua new file mode 100644 index 000000000..29bf49e61 --- /dev/null +++ b/spec/resty/http/uri_escape_spec.lua @@ -0,0 +1,23 @@ +local escape = require("resty.http.uri_escape") + +describe('resty.http.uri_escape', function() + + it("escapes uri correctly", function() + local test_cases = { + {"/foo /test", "/foo%20/test"}, + {"/foo/test", "/foo/test"}, + {"/foo/", "/foo/"}, + {"/foo / test", "/foo%20/%20test"}, + {"/foo / test", "/foo%20%20%20%20/%20%20test"}, + {"/foo#/test", "/foo%23/test"}, + {"/foo$/test", "/foo$/test"}, + {"/foo=/test", "/foo=/test"}, + {"/foo!/test", "/foo!/test"} , + {"/foo,/test", "/foo,/test"}, + } + + for _,val in ipairs(test_cases) do + assert.are.same(escape.escape_uri(val[1]), val[2]) + end + end) +end) diff --git a/t/apicast-mapping-rules.t b/t/apicast-mapping-rules.t index d3f1c337d..db0be4cd0 100644 --- a/t/apicast-mapping-rules.t +++ b/t/apicast-mapping-rules.t @@ -280,3 +280,46 @@ GET /abc/def?user_key=uk --- response_body yay, api backend --- error_code: 200 + +=== TEST 6: request uri with special chars +Call with special chars and validate that are correctly matched. +--- http_config + include $TEST_NGINX_UPSTREAM_CONFIG; + lua_package_path "$TEST_NGINX_LUA_PATH"; + init_by_lua_block { + require('apicast.configuration_loader').mock({ + services = { + { + id = 42, + backend_version = 1, + backend_authentication_type = 'service_token', + backend_authentication_value = 'token-value', + proxy = { + api_backend = "http://127.0.0.1:$TEST_NGINX_SERVER_PORT/api-backend/", + proxy_rules = { + { id = 1, http_method = "GET", + pattern = "/foo%20/bar/", + metric_system_name = "weeee", delta = 1, + } + } + } + } + } + }) + } + lua_shared_dict api_keys 10m; +--- config + include $TEST_NGINX_APICAST_CONFIG; + + location /transactions/authrep.xml { + content_by_lua_block { ngx.exit(200) } + } + + location /api-backend/ { + echo 'yay, api backend'; + } +--- request +GET /foo%20/bar/?user_key=foo +--- response_body +yay, api backend +--- error_code: 200 diff --git a/t/apicast-policy-url-rewriting.t b/t/apicast-policy-url-rewriting.t index 9775067d5..0d698787f 100644 --- a/t/apicast-policy-url-rewriting.t +++ b/t/apicast-policy-url-rewriting.t @@ -681,3 +681,55 @@ yay, api backend --- error_code: 200 --- no_error_log [error] + +=== TEST 13: url rewriting policy placed before the apicast with special chars +The url rewriting policy is placed before the apicast one in the policy chain, +this means that the request will be rewritten before matching the mapping rules +and it'll use special character to make sure that the encode works correctly +--- backend + location /transactions/authrep.xml { + content_by_lua_block { + ngx.exit(200) + } + } +--- configuration +{ + "services": [ + { + "id": 42, + "backend_version": 1, + "backend_authentication_type": "service_token", + "backend_authentication_value": "token-value", + "proxy": { + "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/", + "proxy_rules": [ + { "pattern": "/new%20/foo/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 } + ], + "policy_chain": [ + { + "name": "apicast.policy.url_rewriting", + "configuration": { + "commands": [ + { "op": "sub", "regex": "original", "replace": "new /foo/" } + ] + } + }, + { "name": "apicast.policy.apicast" } + ] + } + } + ] +} +--- upstream + location /new { + content_by_lua_block { + ngx.say('yay, api backend'); + } + } +--- request +GET /original?user_key=value +--- response_body +yay, api backend +--- error_code: 200 +--- no_error_log +[error]