Merge pull request Kong#881 from Mashape/fix/api-validation

refactor(schema) stronger request_path validation

kong/dao/schemas/apis.lua

@@ -1,6 +1,10 @@
 local url = require "socket.url"
 local stringy = require "stringy"
 
+local fmt = string.format
+local sub = string.sub
+local match = string.match
+
 local function validate_upstream_url_protocol(value)
   local parsed_url = url.parse(value)
   if parsed_url.scheme and parsed_url.host then
@@ -77,20 +81,22 @@ local function check_request_path(request_path, api_t)
   end
 
   if request_path ~= nil and request_path ~= "" then
-    request_path = string.gsub(request_path, "^/*", "")
-    request_path = string.gsub(request_path, "/*$", "")
-
-    -- Add a leading slash for the sake of consistency
-    api_t.request_path = "/"..request_path
-
-    -- Check if characters are in RFC 3986 unreserved list
-    local is_alphanumeric = string.match(api_t.request_path, "^/[%w%.%-%_~%/]*$")
-    if not is_alphanumeric then
-      return false, "request_path must only contain alphanumeric and '., -, _, ~, /' characters"
+    if request_path == "/" then
+      return false, "cannot be an empty path: '/'"
+    elseif sub(request_path, 1, 1) ~= "/" then
+      return false, fmt("must be prefixed with slash: '%s'", request_path)
+    elseif 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
-    local is_invalid = string.match(api_t.request_path, "//+")
-    if is_invalid then
-      return false, "request_path is invalid: "..api_t.request_path
+
+    -- From now on, the request_path is considered valid.
+    -- Remove trailing slash
+    if sub(request_path, -1) == "/" then
+      api_t.request_path = sub(request_path, 1, -2)
     end
   end
 

spec/integration/admin_api/kong_routes_spec.lua

@@ -15,7 +15,7 @@ describe("Admin API", function()
   teardown(function()
     spec_helper.stop_kong()
   end)
-  
+
   describe("Kong routes", function()
     describe("/", function()
       local constants = require "kong.constants"
@@ -86,12 +86,11 @@ describe("Admin API", function()
 
   describe("Request size", function()
     it("should properly hanlde big POST bodies < 10MB", function()
-      local response, status = http_client.post(spec_helper.API_URL.."/apis", { request_path = "hello.com", upstream_url = "http://mockbin.org" })
+      local response, status = http_client.post(spec_helper.API_URL.."/apis", {request_host = "hello.com", upstream_url = "http://mockbin.org"})
       assert.equal(201, status)
       local api_id = json.decode(response).id
       assert.truthy(api_id)
 
-
       local big_value = string.rep("204.48.16.0,", 1000)
       big_value = string.sub(big_value, 1, string.len(big_value) - 1)
       assert.truthy(string.len(big_value) > 10000) -- More than 10kb
@@ -101,7 +100,7 @@ describe("Admin API", function()
     end)
 
     it("should fail with requests > 10MB", function()
-      local response, status = http_client.post(spec_helper.API_URL.."/apis", { request_path = "hello2.com", upstream_url = "http://mockbin.org" })
+      local response, status = http_client.post(spec_helper.API_URL.."/apis", {request_host = "hello2.com", upstream_url = "http://mockbin.org"})
       assert.equal(201, status)
       local api_id = json.decode(response).id
       assert.truthy(api_id)