Skip to content

Commit

Permalink
feat(schemas) API names forbid reserved URI characters
Browse files Browse the repository at this point in the history 
- API names cannot contain characters from RFC 3986 reserved characters
  list. If such a character is detected, it is a schema error and the
  creation/update is refused.
- This also sets an API's name to the given `request_path` if no
  `request_dns` is set. The first slash is omitted and subsequent
  slashes are replaced with dashes.
- Since spaces are not allowed anymore in API names, fixture APIs must
comply too.

See #547, #489
  • Loading branch information
@thibaultcha
thibaultcha committed Sep 22, 2015
1 parent f7b5cbb commit ad54019
Show file tree
Hide file tree
Showing 28 changed files with 176 additions and 84 deletions.
66 changes: 56 additions & 10 deletions kong/dao/schemas/apis.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ local function check_request_path(request_path, api_t)
-- 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"
return false, "request_path must only contain alphanumeric and '., -, _, ~, /' characters"
end
local is_invalid = string.match(api_t.request_path, "//+")
if is_invalid then
Expand All @@ -69,18 +69,64 @@ local function check_request_path(request_path, api_t)
return true
end

--- Define a default name for an API.
-- Chosen from request_host or request_path (in that order of preference) if they are set.
-- Normalize the name if it contains any characters from RFC 3986 reserved list.
-- @see https://tools.ietf.org/html/rfc3986#section-2.2
-- @param api_t Table representing the API
-- @return default_name Serialized chosen name or nil
local function default_name(api_t)
local default_name, err

default_name = api_t.request_host
if not default_name and api_t.request_path then
default_name = api_t.request_path:sub(2):gsub("/", "-")
end

if default_name ~= nil then
default_name, _, err = ngx.re.gsub(default_name, "[^\\w.\\-_~]", "-")
if err then
ngx.log(ngx.ERR, err)
return
end

return default_name
end
end

--- Check that a name is valid for an API.
-- It must not contain any URI reserved characters.
-- @param name Name of the API.
-- @return valid Boolean indicating if valid or not.
-- @return err String describing why the name is not valid.
local function check_name(name)
if name then
local m, err = ngx.re.match(name, "[^\\w.\\-_~]")
if err then
ngx.log(ngx.ERR, err)
return
end

if m then
return false, "name must only contain alphanumeric and '., -, _, ~' characters"
end
end

return true
end

return {
name = "API",
primary_key = {"id"},
fields = {
id = { type = "id", dao_insert_value = true },
created_at = { type = "timestamp", dao_insert_value = true },
name = { type = "string", unique = true, queryable = true, default = function(api_t) return api_t.request_host end },
request_host = { type = "string", unique = true, queryable = true, func = check_request_host_and_path,
regex = "([a-zA-Z0-9-]+(\\.[a-zA-Z0-9-]+)*)" },
request_path = { type = "string", unique = true, func = check_request_path },
strip_request_path = { type = "boolean" },
upstream_url = { type = "url", required = true, func = validate_upstream_url_protocol },
preserve_host = { type = "boolean" }
id = {type = "id", dao_insert_value = true},
created_at = {type = "timestamp", dao_insert_value = true},
name = {type = "string", unique = true, queryable = true, default = default_name, func = check_name},
request_host = {type = "string", unique = true, queryable = true, func = check_request_host_and_path,
regex = "([a-zA-Z0-9-]+(\\.[a-zA-Z0-9-]+)*)"},
request_path = {type = "string", unique = true, func = check_request_path},
strip_request_path = {type = "boolean"},
upstream_url = {type = "url", required = true, func = validate_upstream_url_protocol},
preserve_host = {type = "boolean"}
}
}
2 changes: 1 addition & 1 deletion kong/dao/schemas_validation.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function _M.validate_entity(tbl, schema, options)
for tk, t in pairs(key_values) do
if t ~= nil then
local error_prefix = ""
if stringy.strip(tk) ~= "" then
if stringy.strip(tk) ~= "" then
error_prefix = tk.."."
end

Expand Down
8 changes: 6 additions & 2 deletions kong/tools/ngx_stub.lua
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--- Stub _G.ngx for unit testing.
-- Creates a stub for `ngx` for use by Kong's modules such as the DAO. It allows to use them
-- Creates a stub for `ngx` for use by Kong's modules such as the DAO. It allows to use them
-- outside of the nginx context such as when using the CLI, or unit testing.
--
-- Monkeypatches the global `ngx` table.
Expand All @@ -20,7 +20,11 @@ _G.ngx = {
at = function() end
},
re = {
match = reg.match
match = reg.match,
gsub = function(str, pattern, sub)
local res_str, _, sub_made = reg.gsub(str, pattern, sub)
return res_str, sub_made
end
},
-- Builds a querystring from a table, separated by `&`
-- @param `tab` The key/value parameters
Expand Down
8 changes: 4 additions & 4 deletions spec/integration/admin_api/apis_routes_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe("Admin API", function()

it("[SUCCESS] should create an API", function()
send_content_types(BASE_URL, "POST", {
name="api POST tests",
name="api-POST-tests",
request_host="api.mockbin.com",
upstream_url="http://mockbin.com"
}, 201, nil, {drop_db=true})
Expand Down Expand Up @@ -57,18 +57,18 @@ describe("Admin API", function()

it("[SUCCESS] should create and update", function()
local api = send_content_types(BASE_URL, "PUT", {
name="api PUT tests",
name="api-PUT-tests",
request_host="api.mockbin.com",
upstream_url="http://mockbin.com"
}, 201, nil, {drop_db=true})

api = send_content_types(BASE_URL, "PUT", {
id=api.id,
name="api PUT tests updated",
name="api-PUT-tests-updated",
request_host="updated-api.mockbin.com",
upstream_url="http://mockbin.com"
}, 200)
assert.equal("api PUT tests updated", api.name)
assert.equal("api-PUT-tests-updated", api.name)
end)

it("[FAILURE] should return proper errors", function()
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/cli/start_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ describe("CLI", function()
it("should not work when a plugin is being used in the DB but it's not in the configuration", function()
spec_helper.get_env(SERVER_CONF).faker:insert_from_table {
api = {
{name = "tests cli 1", request_host = "foo.com", upstream_url = "http://mockbin.com"},
{name = "tests-cli", request_host = "foo.com", upstream_url = "http://mockbin.com"},
},
plugin = {
{name = "rate-limiting", config = {minute = 6}, __api = 1},
Expand Down
6 changes: 3 additions & 3 deletions spec/integration/dao/cassandra/base_dao_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ describe("Cassandra", function()
assert.True(#apis > 0)

local api_t = apis[1]
api_t.name = api_t.name.." updated"
api_t.name = api_t.name.."-updated"

local api, err = dao_factory.apis:update(api_t)
assert.falsy(err)
Expand Down Expand Up @@ -545,8 +545,8 @@ describe("Cassandra", function()
it("should find distinct plugins configurations", function()
faker:insert_from_table {
api = {
{ name = "tests distinct 1", request_host = "foo.com", upstream_url = "http://mockbin.com" },
{ name = "tests distinct 2", request_host = "bar.com", upstream_url = "http://mockbin.com" }
{ name = "tests-distinct-1", request_host = "foo.com", upstream_url = "http://mockbin.com" },
{ name = "tests-distinct-2", request_host = "bar.com", upstream_url = "http://mockbin.com" }
},
plugin = {
{ name = "key-auth", config = {key_names = {"apikey"}, hide_credentials = true}, __api = 1 },
Expand Down
6 changes: 3 additions & 3 deletions spec/integration/dao/cassandra/cascade_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ describe("Cassandra cascade delete", function()
setup(function()
local fixtures = spec_helper.insert_fixtures {
api = {
{name = "cascade delete",
{name = "cascade-delete",
request_host = "mockbin.com",
upstream_url = "http://mockbin.com"},
{name = "untouched cascade delete",
{name = "untouched-cascade-delete",
request_host = "untouched.com",
upstream_url = "http://mockbin.com"}
},
Expand Down Expand Up @@ -66,7 +66,7 @@ describe("Cassandra cascade delete", function()
setup(function()
local fixtures = spec_helper.insert_fixtures {
api = {
{name = "cascade delete",
{name = "cascade-delete",
request_host = "mockbin.com",
upstream_url = "http://mockbin.com"}
},
Expand Down
22 changes: 11 additions & 11 deletions spec/integration/proxy/api_resolver_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ describe("Resolver", function()
spec_helper.prepare_db()
spec_helper.insert_fixtures {
api = {
{name = "tests host resolver 1", request_host = "mockbin.com", upstream_url = "http://mockbin.com"},
{name = "tests host resolver 2", request_host = "mockbin-auth.com", upstream_url = "http://mockbin.com"},
{name = "tests request_path resolver", upstream_url = "http://mockbin.com", request_path = "/status"},
{name = "tests stripped request_path resolver", upstream_url = "http://mockbin.com", request_path = "/mockbin", strip_request_path = true},
{name = "tests stripped request_path resolver with pattern characters", upstream_url = "http://mockbin.com", request_path = "/mockbin-with-pattern/", strip_request_path = true},
{name = "tests deep request_path resolver", upstream_url = "http://mockbin.com", request_path = "/deep/request_path/", strip_request_path = true},
{name = "tests dup request_path resolver", upstream_url = "http://mockbin.com", request_path = "/har", strip_request_path = true},
{name = "tests wildcard subdomain", upstream_url = "http://mockbin.com/status/200", request_host = "*.wildcard.com"},
{name = "tests wildcard subdomain 2", upstream_url = "http://mockbin.com/status/201", request_host = "wildcard.*"},
{name = "tests preserve host", request_host = "httpbin-nopreserve.com", upstream_url = "http://httpbin.org"},
{name = "tests preserve host 2", request_host = "httpbin-preserve.com", upstream_url = "http://httpbin.org", preserve_host = true}
{name = "tests-host-resolver-1", request_host = "mockbin.com", upstream_url = "http://mockbin.com"},
{name = "tests-host-resolver-2", request_host = "mockbin-auth.com", upstream_url = "http://mockbin.com"},
{name = "tests-request_path-resolver", upstream_url = "http://mockbin.com", request_path = "/status"},
{name = "tests-stripped-request_path-resolver", upstream_url = "http://mockbin.com", request_path = "/mockbin", strip_request_path = true},
{name = "tests-stripped-request_path-resolver-with-pattern-characters", upstream_url = "http://mockbin.com", request_path = "/mockbin-with-pattern/", strip_request_path = true},
{name = "tests-deep-request_path-resolver", upstream_url = "http://mockbin.com", request_path = "/deep/request_path/", strip_request_path = true},
{name = "tests-dup-request_path-resolver", upstream_url = "http://mockbin.com", request_path = "/har", strip_request_path = true},
{name = "tests-wildcard-subdomain", upstream_url = "http://mockbin.com/status/200", request_host = "*.wildcard.com"},
{name = "tests-wildcard-subdomain-2", upstream_url = "http://mockbin.com/status/201", request_host = "wildcard.*"},
{name = "tests-preserve-host", request_host = "httpbin-nopreserve.com", upstream_url = "http://httpbin.org"},
{name = "tests-preserve-host-2", request_host = "httpbin-preserve.com", upstream_url = "http://httpbin.org", preserve_host = true}
},
plugin = {
{name = "key-auth", config = {key_names = {"apikey"} }, __api = 2}
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/proxy/database_cache_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe("Database cache", function()
spec_helper.prepare_db()
fixtures = spec_helper.insert_fixtures {
api = {
{ name = "tests database cache", request_host = "cache.test", upstream_url = "http://httpbin.org" }
{ name = "tests-database-cache", request_host = "cache.test", upstream_url = "http://httpbin.org" }
}
}

Expand Down
4 changes: 2 additions & 2 deletions spec/integration/proxy/dns_resolver_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ describe("DNS", function()
spec_helper.prepare_db()
spec_helper.insert_fixtures {
api = {
{ name = "tests dns 1", request_host = "dns1.com", upstream_url = "http://127.0.0.1:"..TCP_PORT },
{ name = "tests dns 2", request_host = "dns2.com", upstream_url = "http://localhost:"..TCP_PORT }
{ name = "tests-dns-1", request_host = "dns1.com", upstream_url = "http://127.0.0.1:"..TCP_PORT },
{ name = "tests-dns-2", request_host = "dns2.com", upstream_url = "http://localhost:"..TCP_PORT }
}
}

Expand Down
2 changes: 1 addition & 1 deletion spec/integration/proxy/realip_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe("Real IP", function()
spec_helper.prepare_db()
spec_helper.insert_fixtures {
api = {
{ name = "tests realip", request_host = "realip.com", upstream_url = "http://mockbin.com" }
{ name = "tests-realip", request_host = "realip.com", upstream_url = "http://mockbin.com" }
},
plugin = {
{ name = "file-log", config = { path = FILE_LOG_PATH }, __api = 1 }
Expand Down
14 changes: 7 additions & 7 deletions spec/plugins/acl/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ describe("ACL Plugin", function()
spec_helper.prepare_db()
spec_helper.insert_fixtures {
api = {
{name = "ACL 1", request_host = "acl1.com", upstream_url = "http://mockbin.com"},
{name = "ACL 2", request_host = "acl2.com", upstream_url = "http://mockbin.com"},
{name = "ACL 3", request_host = "acl3.com", upstream_url = "http://mockbin.com"},
{name = "ACL 4", request_host = "acl4.com", upstream_url = "http://mockbin.com"},
{name = "ACL 5", request_host = "acl5.com", upstream_url = "http://mockbin.com"},
{name = "ACL 6", request_host = "acl6.com", upstream_url = "http://mockbin.com"},
{name = "ACL 7", request_host = "acl7.com", upstream_url = "http://mockbin.com"}
{name = "ACL-1", request_host = "acl1.com", upstream_url = "http://mockbin.com"},
{name = "ACL-2", request_host = "acl2.com", upstream_url = "http://mockbin.com"},
{name = "ACL-3", request_host = "acl3.com", upstream_url = "http://mockbin.com"},
{name = "ACL-4", request_host = "acl4.com", upstream_url = "http://mockbin.com"},
{name = "ACL-5", request_host = "acl5.com", upstream_url = "http://mockbin.com"},
{name = "ACL-6", request_host = "acl6.com", upstream_url = "http://mockbin.com"},
{name = "ACL-7", request_host = "acl7.com", upstream_url = "http://mockbin.com"}
},
consumer = {
{username = "consumer1"},
Expand Down
2 changes: 1 addition & 1 deletion spec/plugins/basic-auth/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe("Authentication Plugin", function()
spec_helper.prepare_db()
spec_helper.insert_fixtures {
api = {
{name = "tests basicauth", request_host = "basicauth.com", upstream_url = "http://httpbin.org"}
{name = "tests-basicauth", request_host = "basicauth.com", upstream_url = "http://httpbin.org"}
},
consumer = {
{username = "basicauth_tests_consuser"}
Expand Down
4 changes: 2 additions & 2 deletions spec/plugins/cors/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ describe("CORS Plugin", function()
spec_helper.prepare_db()
spec_helper.insert_fixtures {
api = {
{ name = "tests cors 1", request_host = "cors1.com", upstream_url = "http://mockbin.com" },
{ name = "tests cors 2", request_host = "cors2.com", upstream_url = "http://mockbin.com" }
{ name = "tests-cors-1", request_host = "cors1.com", upstream_url = "http://mockbin.com" },
{ name = "tests-cors-2", request_host = "cors2.com", upstream_url = "http://mockbin.com" }
},
plugin = {
{ name = "cors", config = {}, __api = 1 },
Expand Down
2 changes: 1 addition & 1 deletion spec/plugins/hmac-auth/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe("Authentication Plugin", function()
spec_helper.prepare_db()
spec_helper.insert_fixtures {
api = {
{name = "tests hmac auth", request_host = "hmacauth.com", upstream_url = "http://mockbin.org/"}
{name = "tests-hmac-auth", request_host = "hmacauth.com", upstream_url = "http://mockbin.org/"}
},
consumer = {
{username = "hmacauth_tests_consuser"}
Expand Down
6 changes: 3 additions & 3 deletions spec/plugins/jwt/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ describe("JWT access", function()
spec_helper.prepare_db()
local fixtures = spec_helper.insert_fixtures {
api = {
{name = "tests jwt", request_host = "jwt.com", upstream_url = "http://mockbin.com"},

This comment has been minimized.

Sign in to view

Copy link
@opyate

opyate Jan 28, 2016

Contributor

What has Kong got to do with J. Walter Thompson's website?

This comment has been minimized.

Sign in to view

Copy link
@ahmadnassri

ahmadnassri Jan 28, 2016

Contributor

:) it's just a test URL

This comment has been minimized.

Sign in to view

Copy link
@sonicaghi

sonicaghi Jan 28, 2016

Member

jwt.io
{name = "tests jwt2", request_host = "jwt2.com", upstream_url = "http://mockbin.com"},
{name = "tests jwt3", request_host = "jwt3.com", upstream_url = "http://mockbin.com"}
{name = "tests-jwt", request_host = "jwt.com", upstream_url = "http://mockbin.com"},
{name = "tests-jwt2", request_host = "jwt2.com", upstream_url = "http://mockbin.com"},
{name = "tests-jwt3", request_host = "jwt3.com", upstream_url = "http://mockbin.com"}
},
consumer = {
{username = "jwt_tests_consumer"}
Expand Down
4 changes: 2 additions & 2 deletions spec/plugins/key-auth/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ describe("Authentication Plugin", function()
spec_helper.prepare_db()
spec_helper.insert_fixtures {
api = {
{name = "tests auth 1", request_host = "keyauth1.com", upstream_url = "http://mockbin.com"},
{name = "tests auth 2", request_host = "keyauth2.com", upstream_url = "http://mockbin.com"}
{name = "tests-auth1", request_host = "keyauth1.com", upstream_url = "http://mockbin.com"},
{name = "tests-auth2", request_host = "keyauth2.com", upstream_url = "http://mockbin.com"}
},
consumer = {
{username = "auth_tests_consumer"}
Expand Down
12 changes: 6 additions & 6 deletions spec/plugins/logging_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ describe("Logging Plugins", function()
spec_helper.prepare_db()
spec_helper.insert_fixtures {
api = {
{ name = "tests tcp logging", request_host = "tcp_logging.com", upstream_url = "http://mockbin.com" },
{ name = "tests tcp logging2", request_host = "tcp_logging2.com", upstream_url = "http://localhost:"..HTTP_DELAY_PORT },
{ name = "tests udp logging", request_host = "udp_logging.com", upstream_url = "http://mockbin.com" },
{ name = "tests http logging", request_host = "http_logging.com", upstream_url = "http://mockbin.com" },
{ name = "tests https logging", request_host = "https_logging.com", upstream_url = "http://mockbin.com" },
{ name = "tests file logging", request_host = "file_logging.com", upstream_url = "http://mockbin.com" }
{ name = "tests-tcp-logging", request_host = "tcp_logging.com", upstream_url = "http://mockbin.com" },
{ name = "tests-tcp-logging2", request_host = "tcp_logging2.com", upstream_url = "http://localhost:"..HTTP_DELAY_PORT },
{ name = "tests-udp-logging", request_host = "udp_logging.com", upstream_url = "http://mockbin.com" },
{ name = "tests-http-logging", request_host = "http_logging.com", upstream_url = "http://mockbin.com" },
{ name = "tests-https-logging", request_host = "https_logging.com", upstream_url = "http://mockbin.com" },
{ name = "tests-file-logging", request_host = "file_logging.com", upstream_url = "http://mockbin.com" }
},
plugin = {
{ name = "tcp-log", config = { host = "127.0.0.1", port = TCP_PORT }, __api = 1 },
Expand Down
10 changes: 5 additions & 5 deletions spec/plugins/oauth2/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ describe("Authentication Plugin", function()
spec_helper.drop_db()
spec_helper.insert_fixtures {
api = {
{ name = "tests oauth2", request_host = "oauth2.com", upstream_url = "http://mockbin.com" },
{ name = "tests oauth2 with path", request_host = "mockbin-path.com", upstream_url = "http://mockbin.com", request_path = "/somepath/" },
{ name = "tests oauth2 with hide credentials", request_host = "oauth2_3.com", upstream_url = "http://mockbin.com" },
{ name = "tests oauth2 client credentials", request_host = "oauth2_4.com", upstream_url = "http://mockbin.com" },
{ name = "tests oauth2 password grant", request_host = "oauth2_5.com", upstream_url = "http://mockbin.com" }
{ name = "tests-oauth2", request_host = "oauth2.com", upstream_url = "http://mockbin.com" },
{ name = "tests-oauth2-with-path", request_host = "mockbin-path.com", upstream_url = "http://mockbin.com", request_path = "/somepath/" },
{ name = "tests-oauth2-with-hide-credentials", request_host = "oauth2_3.com", upstream_url = "http://mockbin.com" },
{ name = "tests-oauth2-client-credentials", request_host = "oauth2_4.com", upstream_url = "http://mockbin.com" },
{ name = "tests-oauth2-password-grant", request_host = "oauth2_5.com", upstream_url = "http://mockbin.com" }
},
consumer = {
{ username = "auth_tests_consumer" }
Expand Down
Loading

1 comment on commit ad54019

@thibaultcha
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is of no importance it's the request_host, it's not actually making a request to it.

Please sign in to comment.