Copilot commented on code in PR #2836:
URL: https://github.com/apache/shiro/pull/2836#discussion_r3633084241
##########
web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java:
##########
@@ -150,6 +150,15 @@ public FilterChain getChain(ServletRequest request,
ServletResponse response, Fi
}
}
+ // If no chain matched and the requestURI is null/empty, fall back to
the catch-all chain if one exists.
+ // This ensures global filters (e.g. InvalidRequestFilter) always run
when the request path cannot be resolved.
+ if (requestURI == null || "".equals(requestURI)) {
Review Comment:
The new null/empty requestURI fallback check can be simplified to avoid the
string-literal equals pattern; `isEmpty()` is clearer and avoids an unnecessary
constant comparison.
##########
web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java:
##########
@@ -150,6 +150,15 @@ public FilterChain getChain(ServletRequest request,
ServletResponse response, Fi
}
}
+ // If no chain matched and the requestURI is null/empty, fall back to
the catch-all chain if one exists.
+ // This ensures global filters (e.g. InvalidRequestFilter) always run
when the request path cannot be resolved.
+ if (requestURI == null || "".equals(requestURI)) {
+ NamedFilterList catchAllChain = filterChainManager.getChain("/**");
+ if (catchAllChain != null) {
Review Comment:
This new catch-all fallback branch (requestURI null/empty -> use "/**"
chain) isn’t currently exercised by the added tests. The new
PathMatchingFilterChainResolverTest covers normalization returning "/", but not
the resolver’s null/empty branch; consider adding a test that forces requestURI
to null/empty (e.g., by subclassing/overriding getPathWithinApplication in the
test) to prevent regressions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
