Skip to content

Commit 1460c68

Browse files
authored
ensures global wildcard filters always execute while keeping namespace filters isolated to their routes (#737)
1 parent 676a35f commit 1460c68

File tree

2 files changed

+101
-9
lines changed

2 files changed

+101
-9
lines changed

spec/router_spec.cr

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,58 @@ describe "Kemal::Router" do
200200
protected_response = call_request_on_app(protected_request)
201201
protected_response.body.should eq("required")
202202
end
203+
204+
it "applies namespace filters only within the namespace" do
205+
router = Kemal::Router.new
206+
207+
router.namespace "/admin" do
208+
before do |env|
209+
halt env, 401, "unauthorized" unless env.request.headers["X-Admin"]? == "true"
210+
end
211+
212+
get "/dashboard" do |env|
213+
env.get("path").to_s
214+
end
215+
end
216+
217+
router.get "/public" do |env|
218+
env.get("path").to_s
219+
end
220+
221+
mount "/api", router
222+
223+
before_all do |env|
224+
env.set "path", env.request.path
225+
end
226+
227+
get "/public" do |env|
228+
env.get("path").to_s
229+
end
230+
231+
unauthorized_request = HTTP::Request.new("GET", "/api/admin/dashboard")
232+
unauthorized_response = call_request_on_app(unauthorized_request)
233+
unauthorized_response.status_code.should eq(401)
234+
unauthorized_response.body.should eq("unauthorized")
235+
236+
authorized_request = HTTP::Request.new(
237+
"GET",
238+
"/api/admin/dashboard",
239+
headers: HTTP::Headers{"X-Admin" => "true"},
240+
)
241+
authorized_response = call_request_on_app(authorized_request)
242+
authorized_response.status_code.should eq(200)
243+
authorized_response.body.should eq("/api/admin/dashboard")
244+
245+
api_public_request = HTTP::Request.new("GET", "/api/public")
246+
api_public_response = call_request_on_app(api_public_request)
247+
api_public_response.status_code.should eq(200)
248+
api_public_response.body.should eq("/api/public")
249+
250+
public_request = HTTP::Request.new("GET", "/public")
251+
public_response = call_request_on_app(public_request)
252+
public_response.status_code.should eq(200)
253+
public_response.body.should eq("/public")
254+
end
203255
end
204256

205257
describe "nested routers" do

src/kemal/filter_handler.cr

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,29 @@ module Kemal
33
class FilterHandler
44
include HTTP::Handler
55
INSTANCE = new
6-
property tree
6+
7+
# Path used to represent wildcard filters that apply to all routes
8+
private WILDCARD_PATH = "*"
9+
10+
@tree : Radix::Tree(Array(FilterBlock))
11+
12+
# Hash cache for exact path filters to avoid repeated tree lookups
13+
# Key format: "/#{type}/#{verb}/#{path}" (e.g., "/before/ALL/*")
14+
@exact_filters : Hash(String, Array(FilterBlock))
15+
16+
def tree
17+
@tree
18+
end
19+
20+
def tree=(tree : Radix::Tree(Array(FilterBlock)))
21+
@tree = tree
22+
@exact_filters = Hash(String, Array(FilterBlock)).new
23+
end
724

825
# This middleware is lazily instantiated and added to the handlers as soon as a call to `after_X` or `before_X` is made.
926
def initialize
1027
@tree = Radix::Tree(Array(FilterBlock)).new
28+
@exact_filters = Hash(String, Array(FilterBlock)).new
1129
Kemal.config.add_filter_handler(self)
1230
end
1331

@@ -31,15 +49,21 @@ module Kemal
3149
context
3250
end
3351

34-
# :nodoc: This shouldn't be called directly, it's not private because
35-
# I need to call it for testing purpose since I can't call the macros in the spec.
36-
# It adds the block for the corresponding verb/path/type combination to the tree.
52+
# :nodoc:
53+
# This shouldn't be called directly, it's not private because I need to call it for testing purpose since I can't call the macros in the spec.
54+
#
55+
# Registers a filter block for the given verb/path/type combination.
56+
# Uses @exact_filters hash for O(1) lookup when adding multiple filters to the same path.
3757
def _add_route_filter(verb : String, path, type, &block : HTTP::Server::Context -> _)
38-
lookup = lookup_filters_for_path_type(verb, path, type)
39-
if lookup.found? && lookup.payload.is_a?(Array(FilterBlock))
40-
lookup.payload << FilterBlock.new(&block)
58+
key = radix_path(verb, path, type)
59+
60+
if filters = @exact_filters[key]?
61+
filters << FilterBlock.new(&block)
4162
else
42-
@tree.add radix_path(verb, path, type), [FilterBlock.new(&block)]
63+
filters = [FilterBlock.new(&block)]
64+
@exact_filters[key] = filters
65+
66+
@tree.add key, filters
4367
end
4468
end
4569

@@ -57,8 +81,24 @@ module Kemal
5781
_add_route_filter verb, path, :after, &block
5882
end
5983

60-
# This will fetch the block for the verb/path/type from the tree and call it.
84+
# Executes filters for a given path, ensuring global wildcard filters run first.
85+
#
86+
# Execution order:
87+
# 1. Global wildcard filters ("*") - if path is not already a wildcard
88+
# 2. Exact path filters - filters registered for the specific path
89+
#
90+
# This ensures that global filters (like `before_all`) always execute,
91+
# while namespace-specific filters only apply to their registered paths.
6192
private def call_block_for_path_type(verb : String?, path : String, type, context : HTTP::Server::Context)
93+
if path != WILDCARD_PATH
94+
call_block_for_exact_path_type(verb, "*", type, context)
95+
end
96+
97+
# Executes all filter blocks registered for a specific verb/path/type combination
98+
call_block_for_exact_path_type(verb, path, type, context)
99+
end
100+
101+
private def call_block_for_exact_path_type(verb : String?, path : String, type, context : HTTP::Server::Context)
62102
lookup = lookup_filters_for_path_type(verb, path, type)
63103
if lookup.found? && lookup.payload.is_a? Array(FilterBlock)
64104
blocks = lookup.payload

0 commit comments

Comments
 (0)