Skip to content

Commit 4e45d80

Browse files
committed
Improve URI path matching
It turns out that root endpoints were struggling with the prior patch matching algorithm. We didn't notice this previously because we always had at least one level of nesting inside the initial path declaration: ```ruby json_api :api do namespace :v1 do get '/' => 'root_endpoint#index' # ... end end ``` Since there is no `/api` route everything works just great. Things in `v1` properly match `/api/*` including `/api/v1`. However, we now have some endpoints which need to break away from the existing public API wrapper. To support this they need to shift the routes to: ```ruby namespace :api do json_api :v1 do get '/' => 'root_endpoint#index' # ... end # .. more stuff end ``` This causes the wild card matcher to be `/api/v1/*` which excludes a match on `/api/v1`. We don't want the wild card to be `/api/v1*` as that would improperly match something like `/api/v1-old`. Thus we need to check both the actual base path and the nested wild card. However, it turns out that using `Pathname` was the wrong choice. While these _are_ "relative paths" the choice of `Pathname` was the incorrect abstraction wrapper to use. The `Pathname` class is really meant for _file system_ paths. While it has nice support for things like `join` it is a poor choice for URI relative paths. This was discovered when trying to find a good way to adjust the wild card. Too much was attempting to touch the underlying server file system making this extremely prone to info leak and general misconceptions on how things behave. Looking at the available URI wrappers they also have problems when working only with partial / relative paths. Instead it turns out that the simple old school string comparison is not only _fast_ but it is also the simplest solution. To ensure we don't end up with something like `/api/v1//` for the nested path we use the `normalize_path` helper. This ensures consistent formatting and will strip off any trailing separators.
1 parent fae3e75 commit 4e45d80

File tree

2 files changed

+11
-4
lines changed

2 files changed

+11
-4
lines changed

lib/kracken/json_api/path.rb

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,22 @@
11
module Kracken
22
module JsonApi
33
class Path
4-
attr_reader :path_match
4+
attr_reader :basename
5+
attr_reader :pathname
56

67
def initialize(path)
7-
@path_match = Pathname(path).join('*').to_path
8+
@basename = ActionDispatch::Journey::Router::Utils.normalize_path(path)
9+
@pathname = @basename + "/"
810
end
911

1012
def matches?(request)
11-
request.supports_json_format? && request.path.fnmatch?(path_match)
13+
request.supports_json_format? && path_matches?(request.path)
14+
end
15+
16+
private
17+
18+
def path_matches?(path)
19+
path == basename || path.start_with?(pathname)
1220
end
1321
end
1422
end

lib/kracken/json_api/request.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ def path
2121
env['json_api.original_path'] ||= (
2222
env["action_dispatch.original_path"] || env["PATH_INFO"]
2323
)
24-
Pathname(env['json_api.original_path'])
2524
end
2625

2726
# File actionpack/lib/action_dispatch/http/mime_negotiation.rb

0 commit comments

Comments
 (0)