Merge pull request Kong#1361 from Mashape/fix/percentencoded_path

allows url encoded request_paths

kong/dao/schemas/apis.lua

@@ -83,12 +83,20 @@ local function check_request_path(request_path, api_t)
   if request_path ~= nil and request_path ~= "" then
     if sub(request_path, 1, 1) ~= "/" then
       return false, fmt("must be prefixed with slash: '%s'", request_path)
-    elseif match(request_path, "//+") then
+    end
+    if match(request_path, "//+") then
       -- Check for empty segments (/status//123)
       return false, fmt("invalid: '%s'", request_path)
-    elseif not match(request_path, "^/[%w%.%-%_~%/]*$") then
-      -- Check if characters are in RFC 3986 unreserved list
-      return false, "must only contain alphanumeric and '., -, _, ~, /' characters"
+    end
+    if not match(request_path, "^/[%w%.%-%_~%/%%]*$") then
+      -- Check if characters are in RFC 3986 unreserved list, and % for percent encoding
+      return false, "must only contain alphanumeric and '., -, _, ~, /, %' characters"
+    end
+    local esc = request_path:gsub("%%%x%x", "___") -- drop all proper %-encodings
+    if match(esc, "%%") then
+      -- % is remaining, so not properly encoded
+      local err = request_path:sub(esc:find("%%.?.?"))
+      return false, "must use proper encoding; '"..err.."' is invalid"
     end
 
     -- From now on, the request_path is considered valid.

spec/01-unit/11-entities_schemas_spec.lua

@@ -156,7 +156,7 @@ describe("Entities Schemas", function()
           local t = {request_path = v, upstream_url = "http://mockbin.com", name = "mockbin"}
           local valid, errors = validate_entity(t, api_schema)
           assert.False(valid)
-          assert.equal("must only contain alphanumeric and '., -, _, ~, /' characters", errors.request_path)
+          assert.equal("must only contain alphanumeric and '., -, _, ~, /, %' characters", errors.request_path)
         end
       end)
       it("should accept unreserved characters from RFC 3986", function()
@@ -169,6 +169,32 @@ describe("Entities Schemas", function()
           assert.True(valid)
         end
       end)
+      it("should not accept bad %-encoded characters", function()
+        local invalids = {
+          "/some%2words",
+          "/some%0Xwords",
+          "/some%2Gwords",
+          "/some%20words%",
+          "/some%20words%a",
+          "/some%20words%ax",
+        }
+        local errstr = { "%2w", "%0X", "%2G", "%", "%a", "%ax" }
+        for i, v in ipairs(invalids) do
+          local t = {request_path = v, upstream_url = "http://mockbin.com", name = "mockbin"}
+          local valid, errors = validate_entity(t, api_schema)
+          assert.False(valid)
+          assert.equal("must use proper encoding; '"..errstr[i].."' is invalid", errors.request_path)
+        end
+      end)
+      it("should accept properly %-encoded characters", function()
+        local valids = {"/abcd%aa%10%ff%AA%FF"}
+        for _, v in ipairs(valids) do
+          local t = {request_path = v, upstream_url = "http://mockbin.com", name = "mockbin"}
+          local valid, errors = validate_entity(t, api_schema)
+          assert.falsy(errors)
+          assert.True(valid)
+        end
+      end)
       it("should not accept without prefix slash", function()
         local invalids = {"status", "status/123"}
 

spec/02-integration/05-proxy/01-resolver_spec.lua

@@ -61,6 +61,10 @@ describe("Resolver", function()
       upstream_url = "http://mockbin.com/status/204",
       strip_request_path = true
     })
+    assert(helpers.dao.apis:insert {
+      request_path = "/request/urlenc/%20%20",
+      upstream_url = "http://mockbin.com/",
+    })
 
     assert(helpers.start_kong())
     client = helpers.proxy_client()
@@ -198,7 +202,7 @@ describe("Resolver", function()
       })
       assert.res_status(200, res)
     end)
-    it("doesn't append trailing shalsh when strip_request_path is false", function()
+    it("doesn't append trailing slash when strip_request_path is false", function()
       local res = assert(client:send {
         method = "GET",
         path = "/request"
@@ -207,6 +211,15 @@ describe("Resolver", function()
       local json = cjson.decode(body)
       assert.equal("http://mockbin.com/request", json.url)
     end)
+    it("proxies percent-encoded request_path", function()
+      local res = assert(client:send {
+        method = "GET",
+        path = "/request/urlenc/%20%20"
+      })
+      assert.response(res).has.status(200)
+      local json = assert.response(res).has.jsonbody()
+      assert.equals("http://mockbin.com/request/urlenc/%20%20", json.url)
+    end)
 
     describe("strip_request_path", function()
       it("sanity", function()