Skip to content

Commit

Permalink
Fix APIAP routing policy rule
Browse files Browse the repository at this point in the history
The regex used to match the path of the backend in the APIAP routing policy configuration should only allow paths that _start with_ the chosen value for the path. Currently it matches all paths that _contains_ the value. This fixes it by adding a `^` to the beginning of the pattern.
  • Loading branch information
guicassolato authored and Martouta committed Mar 30, 2020
1 parent 0c20591 commit 0e775bb
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 12 deletions.
4 changes: 2 additions & 2 deletions app/models/config_path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ def initialize(value)
end

def to_regex
return '/.*' if empty?
return '^(/.*)' if empty?

"#{path}/.*|#{path}/?"
"^(#{path}/.*|#{path}/?)"
end

def empty?
Expand Down
8 changes: 4 additions & 4 deletions test/unit/backend_api_logic/routing_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ class RoutingPolicyTest < ActiveSupport::TestCase
backend_api1 = backend_apis.first
backend_api2 = backend_apis.last
injected_rules = [
{ url: backend_api2.private_endpoint, owner_id: backend_api2.id, owner_type: 'BackendApi', condition: { operations: [match: :path, op: :matches, value: '/foo/.*|/foo/?'] }, replace_path: "{{uri | remove_first: '/foo'}}" },
{ url: backend_api1.private_endpoint, owner_id: backend_api1.id, owner_type: 'BackendApi', condition: { operations: [match: :path, op: :matches, value: '/.*'] } }
{ url: backend_api2.private_endpoint, owner_id: backend_api2.id, owner_type: 'BackendApi', condition: { operations: [match: :path, op: :matches, value: '^(/foo/.*|/foo/?)'] }, replace_path: "{{uri | remove_first: '/foo'}}" },
{ url: backend_api1.private_endpoint, owner_id: backend_api1.id, owner_type: 'BackendApi', condition: { operations: [match: :path, op: :matches, value: '^(/.*)'] } }
]
apicast_policy = { name: 'apicast', 'version': 'builtin', 'configuration': {} }
injected_policy = {
Expand All @@ -52,8 +52,8 @@ class RoutingPolicyTest < ActiveSupport::TestCase
backend_api1 = backend_apis.first
backend_api2 = backend_apis.last
injected_rules = [
{ url: backend_api2.private_endpoint, owner_id: backend_api2.id, owner_type: 'BackendApi', condition: { operations: [match: :path, op: :matches, value: '/foo/.*|/foo/?'] }, replace_path: "{{uri | remove_first: '/foo'}}" },
{ url: backend_api1.private_endpoint, owner_id: backend_api1.id, owner_type: 'BackendApi', condition: { operations: [match: :path, op: :matches, value: '/.*'] } },
{ url: backend_api2.private_endpoint, owner_id: backend_api2.id, owner_type: 'BackendApi', condition: { operations: [match: :path, op: :matches, value: '^(/foo/.*|/foo/?)'] }, replace_path: "{{uri | remove_first: '/foo'}}" },
{ url: backend_api1.private_endpoint, owner_id: backend_api1.id, owner_type: 'BackendApi', condition: { operations: [match: :path, op: :matches, value: '^(/.*)'] } },
routing_rule
]
injected_routing_policy = {
Expand Down
6 changes: 3 additions & 3 deletions test/unit/config_path_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ class ConfigPathTest < ActiveSupport::TestCase
refute ConfigPath.new('/somepath').empty?
end

test '#to_regex returns "/.*" if path is not informed' do
assert_equal '/.*', ConfigPath.new('/').to_regex
test '#to_regex returns "^(/.*)" if path is not informed' do
assert_equal '^(/.*)', ConfigPath.new('/').to_regex
end

test '#to_regex returns the regex for the path if it is informed' do
assert_equal '/some/path/.*|/some/path/?', ConfigPath.new('/some/path').to_regex
assert_equal '^(/some/path/.*|/some/path/?)', ConfigPath.new('/some/path').to_regex
end
end
6 changes: 3 additions & 3 deletions test/unit/proxy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ def test_policy_chain_with_backend_apis
{"name"=>"routing", "version"=>"builtin", "enabled"=>true,
"configuration"=>{
"rules"=>[
{"url"=>"https://private-2.example.com:443", "owner_id"=>backend_api2.id, "owner_type" => "BackendApi", "condition"=>{"operations"=>[{"match"=>"path", "op"=>"matches", "value"=>"/foo/bar/.*|/foo/bar/?"}]}, 'replace_path'=>"{{uri | remove_first: '/foo/bar'}}"},
{"url"=>"https://private-1.example.com:443", "owner_id"=>backend_api1.id, "owner_type" => "BackendApi", "condition"=>{"operations"=>[{"match"=>"path", "op"=>"matches", "value"=>"/foo/.*|/foo/?"}]}, 'replace_path'=>"{{uri | remove_first: '/foo'}}"},
{"url"=>"https://echo-api.3scale.net:443", "owner_id"=>backend_api0.id, "owner_type" => "BackendApi", "condition"=>{"operations"=>[{"match"=>"path", "op"=>"matches", "value"=>"/.*"}]}}
{"url"=>"https://private-2.example.com:443", "owner_id"=>backend_api2.id, "owner_type" => "BackendApi", "condition"=>{"operations"=>[{"match"=>"path", "op"=>"matches", "value"=>"^(/foo/bar/.*|/foo/bar/?)"}]}, 'replace_path'=>"{{uri | remove_first: '/foo/bar'}}"},
{"url"=>"https://private-1.example.com:443", "owner_id"=>backend_api1.id, "owner_type" => "BackendApi", "condition"=>{"operations"=>[{"match"=>"path", "op"=>"matches", "value"=>"^(/foo/.*|/foo/?)"}]}, 'replace_path'=>"{{uri | remove_first: '/foo'}}"},
{"url"=>"https://echo-api.3scale.net:443", "owner_id"=>backend_api0.id, "owner_type" => "BackendApi", "condition"=>{"operations"=>[{"match"=>"path", "op"=>"matches", "value"=>"^(/.*)"}]}}
]
}
},
Expand Down

0 comments on commit 0e775bb

Please sign in to comment.