Mapping Rule: Use unescaped rule

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
 

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

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

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)

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

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]