[hahwul]

Description of the Change

Kemal::FilterHandler stored every top-level path filter in one Radix::Tree and ran only the payload of the single best match for the request path. When two path filters both covered a request, only the more specific one ran, so a broader guard silently did not:

before_all "/api/*" { |env| halt env, status_code: 401 }
before_all "/api/public" { |env| env.response.headers["X-Public"] = "1" }
get "/api/public" { "PUBLIC" }
get "/api/secret" { "SECRET" }

# GET /api/secret -> 401
# GET /api/public -> 200, the guard never ran

The same happens with a param guard next to a static path (before_get "/users/:id" was skipped for GET /users/me once before_get "/users/me" existed), with two paths that differ by a trailing slash, and with a glob next to its own prefix. The shared tree could also lose filters altogether: before_get "/users/:id" and before_get "/users/:uid/posts" together ran for neither /users/5 nor /users/5/posts, and adding before_get "/users/me" to those two failed at boot with Radix::Tree::SharedKeyError.

Router-scoped filters were taken out of that tree in GHSA-89xc-29wg-6mm5 for the same reason; this does the same for the top-level ones.

Every matching path filter runs. Each filter path now gets its own single-key Radix::Tree, kept in @path_filters and grouped by "#{type}/#{verb}" (the key the global filters already use) in the order the paths were first registered. call_block_for_path_type asks each tree of the current verb/type and runs every one that matches. Since every tree holds one key, what a single filter path matches is exactly what radix gives it on its own today (static exact, :param one segment, *glob, trailing-slash tolerance); there is no new matcher. Each tree also carries the literal start of its key (everything before the first : or *, less a trailing / so the trailing-slash tolerance still matches), and a request path that does not start with it is turned away with starts_with? before Radix::Tree#find allocates anything. With 20 before_get "/otherN/:id" filters and a GET /home route, a request allocates 35.3 KB instead of 37.5 KB without the check (release build). A fuzz run over 4.2M filter/request path pairs found no path radix matches that the check turns away. The order within a step is unchanged otherwise: global filters first, then the matching path filters, then the route's router filters. Blocks on the same path keep their definition order. call_block_for_path_type is the one place every caller goes through, so routed requests, unrouted requests (404/405) and the HEAD -> GET fallback are all covered.

A before filter that closes the response ends the before filters. Running every match made a latent problem common: the filters after a halt kept running on a response that had already been sent. With a guard halting and a more specific filter behind it setting content_type=, the client got its 401, but content_type= raised IO::Error on the closed response, which skipped after_all, and the later filter's work ran for a request the guard had refused. The first before filter that closes the response (halt, redirect) now stops every later before filter: later blocks on the same path, path filters after a halting global filter, the next step (before_get after before_all), and router filters, just as RouteHandler already returns on a closed response. after filters all still run.

A block registered for several paths at once runs once. before_all ["/admin", "/admin/*"] used to wrap the block once per path; with both paths matching /admin it would now run twice. The Enumerable overload in dsl.cr now registers one FilterBlock flagged shared? for all of its paths, and the path loop skips a shared block that already ran in the step. The list that tracks them is only allocated once a shared block matches.

@tree is no longer filled. Nothing reads it to dispatch, and it was the only source of the boot-time SharedKeyError. tree stays as a getter (now empty) and tree= still resets every filter, which spec/spec_helper.cr relies on. path_filters_empty? is replaced by the @path_filters[key]? lookup, which skips the radix key allocation and every lookup when the verb/type has no path filters. The unused private filter_for_path_type_defined? and lookup_filters_for_path_type are removed.

Specs in spec/filters_spec.cr cover each overlap case above, after filters overlapping, HEAD served by the GET route, an unrouted request, run order, the split and refused keys, the multi-path block, and halting in every position, plus a table that checks the handler runs a filter exactly where radix alone finds its path. In the overlap specs the more specific filter is registered before the halting guard, so each one shows that both filters ran. All of them fail on cfdbeee and pass here. crystal spec passes (607 examples), as do crystal tool format --check and ameba.

Alternate Designs

• Collect every matching node from the shared tree. Radix::Tree#find returns one result and the shard has no API to walk all matches. Writing one, or a new matcher, risks changing what an individual filter path matches; a tree per path reuses radix as it is.
• Compile filter paths to a regex, like the router's filter_pattern. The two give different answers for the same path (top-level before_all "/admin" matches /admin and /admin/, router.before "/admin" also matches /admin/x), so it would change what existing top-level filters match.
• Run the matches from broad to specific. There is no well-defined specificity order between overlapping globs and params, while registration order is predictable and is already how global filters and blocks on one path behave.
• Stop only between path filters after a halt. Tried first. It left the gap the halt change is about: a before_all "/api/*" guard halting still let a before_get "/api/public" filter run, since that is the next step. Not stopping at all keeps the IO::Error above on every refused request.
• Also stop once a guard sets a status that a registered error handler covers. Not done; that adds a second meaning to a status code. A guard refuses with halt, and the CHANGELOG says so.
• Keep filling @tree so tree still reflects the filters. Rejected, as it keeps the boot-time SharedKeyError for combinations the per-path trees accept. The class is :nodoc:, and no shard I checked (spec-kemal, kemal-session, kemal-cache, kemal-hmac, kemal-basic-auth) touches tree.
• De-duplicate every block per request. It would cost an allocation per request. Only the Enumerable overload shares a block across paths, so only those blocks are tracked.

Benefits

• A broader guard can no longer be skipped by a more specific filter path, in any verb, for routed and unrouted requests and for HEAD served by a GET route.
• A filter path matches what it matches on its own, so split keys that used to match nothing now run, and combinations radix refused at boot are accepted.
• A halting guard ends the request's before filters: no IO::Error from later filters writing to the closed response, no skipped after_all, and no work done for a refused request.
• before_all ["/admin", "/admin/*"] runs its block once.

Possible Drawbacks

All of these are in the two (BREAKING) CHANGELOG entries, each of which starts with what to check before upgrading.

• An app whose more specific filter happened to hide a broader guard now gets the guard there too. A login under a guarded prefix (before_all "/api/*" with any before_all "/api/login" filter) starts answering 401. Nothing fails at compile time; the fix is to skip such paths inside the guard (next if env.request.path == "/api/login").
• Side effects in overlapping filters (logging, counters, rate limits) run once per matching path, e.g. before_get "/admin" and before_get "/admin/" both run for either spelling. Only a block registered for several paths at once is de-duplicated, and only within one step (a path list that mixes "*" with a path still runs the block twice).
• Filters run in the order their paths were first registered, so a broader filter registered later now overrides a more specific one (a content_type set by /api/html is replaced by one set by /api/*). after filters run in the same order, not reversed.
• A before filter that comes after a guard and has to see refused requests (audit logging of failed logins, metrics) no longer runs for them; it belongs in an after filter.
• after filters still run on a halted response, and more of them now run. One that sets a header or the content type has to check env.response.closed? first, or its IO::Error skips the after filters behind it.
• Each request checks every filter path of the step's verb/type instead of doing one lookup per step. A filter path whose literal start the request path does not begin with costs a byte compare; only the ones that pass go through Radix::Tree#find, which allocates a Result and a params Hash per call. So the cost still grows with the number of filter paths sharing a prefix with the request.
• Kemal::FilterHandler#tree now returns an empty tree; a filter added to it directly no longer runs.

[hahwul]

Hi @sdogruyol
This includes a few breaking changes to filter behavior (see CHANGELOG)
Would appreciate your thoughts before I mark it ready :)

[sdogruyol]

Looks good to me. The breaking changes are the right call, and the CHANGELOG covers them. I checked locally: with before_all "/api/*" (halting) plus before_all "/api/public", GET /api/public is 401 here and 200 on master. CI is green.

One thing before ready: Radix::Tree#find allocates a Result and its params Hash on every call, so each path filter that doesn't match still costs two allocations per step. With 20 before_get "/otherN/:id" filters, GET /home went from 36.5 KB to 38.2 KB allocated per request (release build). Storing each path's literal prefix (up to the first :/*, trailing / chopped so trailing-slash tolerance still works) and skipping find unless radix_key.starts_with?(prefix) makes the non-matching ones a byte compare, without touching what a match means.

Nit: no other comment in the repo uses a ponytail: tag. A plain comment is enough, or drop it if the prefix check goes in.

[hahwul]

@sdogruyol Thanks a lot!

[hahwul]

@sdogruyol
I've made the updates! Please feel free to share any thoughts or feedback!

[sdogruyol]

Thanks, the prefix check is right. I ran it against radix directly: 3,600 filter/request pairs (static, :param, *glob, mid-segment params, trailing slashes), and radix never matched a path the prefix turned away. With 20 non-matching filter paths, GET /home now allocates 35.7 KB per request, less than master's 36.5 KB. Suite (609), format and ameba pass locally.

LGTM.