Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Rewrite URL captures" policy #827

Merged
merged 8 commits into from
Aug 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Keycloak Role Check policy [PR #773](https://github.com/3scale/apicast/pull/773), [THREESCALE-1158](https://issues.jboss.org/browse/THREESCALE-1158)
- Conditional policy. This policy includes a condition and a policy chain, and only executes the chain when the condition is true [PR #812](https://github.com/3scale/apicast/pull/812), [PR #814](https://github.com/3scale/apicast/pull/814), [PR #820](https://github.com/3scale/apicast/pull/820)
- Request headers are now exposed in the context available when evaluating Liquid [PR #819](https://github.com/3scale/apicast/pull/819)
- Rewrite URL captures policy. This policy captures arguments in a URL and rewrites the URL using them [PR #827](https://github.com/3scale/apicast/pull/827), [THREESCALE-1139](https://issues.jboss.org/browse/THREESCALE-1139)

### Changed

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"$schema": "http://apicast.io/policy-v1/schema#manifest#",
"name": "Rewrite URL captures",
"summary": "Captures arguments in a URL and rewrites the URL using them",
"description":
["Captures arguments in a URL and rewrites the URL using those arguments. ",
"For example, we can specify a matching rule with arguments like ",
"'/{orderId}/{accountId}' and a template that specifies how to rewrite ",
"the URL using those arguments, for example: ",
"'/sales/v2/{orderId}?account={accountId}'. In that case, the request ",
"'/123/456' will be transformed into '/sales/v2/123?account=456'"],
"version": "builtin",
"configuration": {
"type": "object",
"properties": {
"transformations": {
"type": "array",
"items": {
"type": "object",
"properties": {
"match_rule": {
"type": "string",
"description": "Rule to be matched"
},
"template": {
"type": "string",
"description": "Template in which the matched args are replaced"
}
}
}
}
}
}
}
1 change: 1 addition & 0 deletions gateway/src/apicast/policy/rewrite_url_captures/init.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
return require 'rewrite_url_captures'
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
local re_gsub = ngx.re.gsub
local re_match = ngx.re.match
local re_gmatch = ngx.re.gmatch
local re_split = require('ngx.re').split
local insert = table.insert
local format = string.format
local ipairs = ipairs
local setmetatable = setmetatable

local _M = {}

local mt = { __index = _M }

local function split(string, separator, max_matches)
return re_split(string, separator, 'oj', nil, max_matches)
end

-- Returns a list of named args extracted from a match_rule.
-- For example, for the rule /{abc}/{def}?{ghi}=1, it returns this list:
Copy link
Contributor

Choose a reason for hiding this comment

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

So, do you think it makes sense to use just {} instead of {{ }} to match with the liquid syntax?

Copy link
Contributor Author

@davidor davidor Aug 3, 2018

Choose a reason for hiding this comment

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

I think {} is better in this case because it's the same format that we are using for the mapping rules.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thats fair point. And for the replacement ? Shouldn't we just pass it through liquid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. That would only replace the code we have in replace_in_template which is not problematic, in my opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep. I'm thinking about consistency. If push people everywhere to use liquid, then it would make sense to stick to it. I have no hard feelings one way or the other, just want to avoid confused people that can't use what they do everywhere else.

But it will not be hard to migrate. Just adding the template_type. So if you have no feelings one way or the other then lets merge this 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather leave this one as it is, at least for now 👍

-- { "{abc}", "{def}", "{ghi}" }.
--
-- Notice that each named arg is wrapped between "{" and "}". That's because
-- we always need to match those "{}", so we can add them here and avoid
-- string concatenations later.
local function extract_named_args(match_rule)
local iterator, err = re_gmatch(match_rule, [[\{(.+?)\}]], 'oj')

if not iterator then
return nil, err
end

local named_args = {}

while true do
local m, err_iter = iterator()
if err_iter then
return nil, err_iter
end

if not m then
break
end

insert(named_args, format('{%s}', m[1]))
end

return named_args
end

-- Rules contain {} for named args. This function replaces those with "()" to
-- be able to capture those args when matching the regex.
local function transform_rule_to_regex(match_rule)
return re_gsub(
match_rule,
[[\{.+?\}]],
[[([\w-.~%!$$&'()*+,;=@:]+)]], -- Same as in the MappingRule module
'oj'
)
end

-- Transforms a string representing the args of a query like:
-- "a=1&b=2&c=3" into 2 tables one with the arguments, and
-- another with the values:
-- { 'a', 'b', 'c' } and { '1', '2', '3' }.
local function string_params_to_tables(string_params)
if not string_params then return {}, {} end

local args = {}
local values = {}

local params_split = split(string_params, '&', 2)

for _, param in ipairs(params_split) do
local parts = split(param, '=', 2) -- avoid unpack, not jitted.
insert(args, parts[1])
insert(values, parts[2])
end

return args, values
end

local function replace_in_template(args, vals, template)
local res = template

for i = 1, #args do
res = re_gsub(res, args[i], vals[i], 'oj')
end

return res
end

local function uri_and_params_from_template(template)
local parts = split(template, [[\?]], 2) -- avoid unpack, not jitted.
return parts[1], parts[2]
end

--- Initialize a NamedArgsMatcher
-- @tparam string match_rule Rule to be matched and that contains named args
-- with "{}". For example: "/{var_1}/something/{var_2}".
-- @tparam string template Template in which the named args matched will be
-- replaced. For example: "/v2/something/{var_1}?my_arg={var_2}".
function _M.new(match_rule, template)
local self = setmetatable({}, mt)

self.named_args = extract_named_args(match_rule)
self.regex_rule = transform_rule_to_regex(match_rule)
self.template = template

return self
end

--- Match a path
-- @tparam string path The path of the URL
-- @treturn boolean True if there's a match, false otherwise
-- @treturn string The new path. If there's a match
-- @treturn table The new args. If there's a match
-- @treturn table The new values for the args. If there's a match.
-- Note: this method generates a new url and new query args when there is a
-- match, but does not modify the current ones.
-- Note: this method returns in separate tables the query args and their values
-- this is so callers can iterate through them with pairs (jitted) instead of
-- ipairs (non-jitted).
function _M:match(path)
local matches = re_match(path, self.regex_rule, 'oj')

if not matches or #self.named_args ~= #matches then
return false
end

local replaced_template = replace_in_template(
self.named_args, matches, self.template)

local uri, raw_params = uri_and_params_from_template(replaced_template)

local params, vals = string_params_to_tables(raw_params)

return true, uri, params, vals
end

return _M
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
--- Rewrite URL Captures policy
-- This policy captures arguments in a URL and rewrites the URL using those
-- arguments.
-- For example, we can specify a matching rule with arguments like
-- '/{orderId}/{accountId}' and a template that specifies how to rewrite the
-- URL using those arguments, for example:
-- '/sales/v2/{orderId}?account={accountId}'.
-- In that case, the request '/123/456' will be transformed into
-- '/sales/v2/123?account=456'

local NamedArgsMatcher = require('named_args_matcher')
local QueryParams = require('apicast.query_params')

local ipairs = ipairs
local insert = table.insert

local policy = require('apicast.policy')
local _M = policy.new('Capture args policy')

local new = _M.new

function _M.new(config)
local self = new(config)

self.matchers = {}
Copy link
Contributor

@mikz mikz Aug 3, 2018

Choose a reason for hiding this comment

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

Just for the future. Using tab_new(#config.transformations, 0) would save table expansion because it would preallocate slots in the table for each matcher.


for _, transformation in ipairs(config.transformations or {}) do
local matcher = NamedArgsMatcher.new(
transformation.match_rule,
transformation.template
)

insert(self.matchers, matcher)
end

return self
end

local function change_uri(new_uri)
ngx.req.set_uri(new_uri)
end

-- When a param in 'new_params' exist in the request, this function replaces
-- its value. When it does not exist, it simply adds it.
-- This function does not delete or modify the params in the query that do not
-- appear in 'new_params'.
local function set_query_params(params, param_vals)
local query_params = QueryParams.new()

for i = 1, #params do
query_params:set(params[i], param_vals[i])
end
end

-- This function only applies the first rule that matches.
-- Defining rules that take into account previous matches can become quite
-- complex and I don't think it's a common use case. Notice that it's possible
-- to do that anyway by chaining multiple instances of this policy.
function _M:rewrite()
local uri = ngx.var.uri

for _, matcher in ipairs(self.matchers) do
local match, new_uri, params, param_vals = matcher:match(uri)

if match then
change_uri(new_uri)
set_query_params(params, param_vals)
return
end
end
end

return _M
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ local ipairs = ipairs
local sub = ngx.re.sub
local gsub = ngx.re.gsub

local QueryParams = require 'apicast.policy.url_rewriting.query_params'
local QueryParams = require 'apicast.query_params'
local TemplateString = require 'apicast.template_string'
local default_value_type = 'plain'

Expand Down
96 changes: 96 additions & 0 deletions spec/policy/rewrite_url_captures/named_args_matcher_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
local NamedArgsMatcher = require('apicast.policy.rewrite_url_captures.named_args_matcher')

describe('named_args_matcher', function()
describe('.match', function()
describe('when there is a match', function()
describe('and there are query args in the template', function()
it('returns true, the new url and query args', function()
local match_rule = "/{var_1}/blah/{var_2}/{var_3}"
local template = "/{var_2}/something/{var_1}?my_arg={var_3}"
local matcher = NamedArgsMatcher.new(match_rule, template)
local request_path = "/abc/blah/def/ghi"

local matched, new_uri, args, arg_vals = matcher:match(request_path)

assert.is_true(matched)
assert.equals('/def/something/abc', new_uri)
assert.same({ 'my_arg' }, args)
assert.same({ 'ghi' }, arg_vals)
end)
end)

describe('and there are no query args in the template', function()
it('returns true, the new url and an empty list of args', function()
local match_rule = "/{var_1}/blah/{var_2}"
local template = "/{var_2}/something/{var_1}"
local matcher = NamedArgsMatcher.new(match_rule, template)
local request_path = "/abc/blah/def"

local matched, new_uri, args, arg_vals = matcher:match(request_path)

assert.is_true(matched)
assert.equals('/def/something/abc', new_uri)
assert.same({}, args)
assert.same({}, arg_vals)
end)
end)

describe('and only the params part of the template has args', function()
it('returns true, the new url and an empty list of args', function()
local match_rule = "/v2/{var_1}"
local template = "/?my_arg={var_1}"
local matcher = NamedArgsMatcher.new(match_rule, template)
local request_path = "/v2/abc"

local matched, new_uri, args, arg_vals = matcher:match(request_path)

assert.is_true(matched)
assert.equals('/', new_uri)
assert.same({ 'my_arg' }, args)
assert.same({ 'abc' }, arg_vals)
end)
end)
end)

describe('when there is not a match', function()
describe('because no args matched', function()
it('returns false', function()
local match_rule = "/{var_1}/blah/{var_2}/{var_3}"
local template = "/{var_2}/something/{var_1}?my_arg={var_3}"
local matcher = NamedArgsMatcher.new(match_rule, template)
local request_path = "/"

local matched = matcher:match(request_path)

assert.is_false(matched)
end)
end)

describe('because only some args matched', function()
it('returns false', function()
local match_rule = "/{var_1}/blah/{var_2}/{var_3}"
local template = "/{var_2}/something/{var_1}?my_arg={var_3}"
local matcher = NamedArgsMatcher.new(match_rule, template)
local request_path = "/abc/blah"

local matched = matcher:match(request_path)

assert.is_false(matched)
end)
end)

describe('and there are no args in the rule', function()
it('returns false', function()
local match_rule = "/i_dont_match_the_request_path"
local template = "/something"
local matcher = NamedArgsMatcher.new(match_rule, template)
local request_path = "/abc/def"

local matched = matcher:match(request_path)

assert.is_false(matched)
end)
end)
end)
end)
end)
Loading