fix(lambda): filter path canonicalization

[ckawl]

Issue #, if available:

Description of changes:

Two related fixes in the servlet emulation layer, plus regression tests.

1. Filter matching and servlet resolution read different paths

Filter url-pattern matching used request.getRequestURI(), which returns the raw undecoded path, while servlet resolution used getPathInfo(), which was decoded. Because the two decisions read different spellings of the same request, a path could be written so that they disagreed about which resource was being asked for. A filter mapped to /admin/* was not selected for /%61dmin/secret, while the request still routed to the handler mapped to /admin/*.

The first revisions of this PR tried to fix that by agreeing on one canonical path. Review showed that does not work, twice, in opposite directions, because consumers downstream do not agree on which form they route on:

• servlet resolution here uses the canonical path
• Spring MVC matches on the raw request URI and leaves dot segments to a servlet container, which does not exist in Lambda
• other code reads the decoded-but-not-normalized form

So filter selection no longer chooses a winner. A filter applies if its url-pattern matches the path under either the decoded-but-not-normalized or the canonical spelling. Both are load-bearing, and dropping either one reopens a bypass that a test in this PR catches:

• without the canonical spelling, /public/../admin/secret does not select the /admin/* filter but still routes to the admin handler
• without the decoded spelling, /admin/../public/info does not select it either, because its canonical form is /public/info while Spring MVC still hands it to an /admin/** handler

An earlier revision also matched the undecoded request URI, making three spellings. That arm was dropped. Across a sweep of 688,908 (path, url-pattern) pairs it was the only matching arm in 1,462 of them, and in every one of those the url-pattern itself contained a percent escape such as /adm%69n/*. Since url-patterns are matched against the decoded path, a pattern written that way would not match in a real container either, so the arm only widened the over-selection without protecting anything.

Supporting changes:

• canonicalizePath and decodePath live in AwsHttpServletRequest, alongside cleanUri. decodePath gathers consecutive escapes and decodes them as a group using the configured uriEncoding, and copies literal characters straight through, so a surrogate pair is never split.
• getPathInfo() returns the canonical path in both AwsProxyHttpServletRequest and AwsHttpApiV2ProxyHttpServletRequest, and in AwsHttpServletRequestWrapper, which is reached on async dispatch and had kept a copy of the pre-fix logic.
• UrlPathValidator reads a decode-only path with the context path stripped. It rejects traversal segments, so it needs them visible: the raw URI leaves %2e%2e unrecognizable, and the canonical path has already resolved them away.
• Decoding is not delegated to URLDecoder, which implements form encoding and would turn a literal + in a path segment into a space. There is a test pinning that.

Behaviour changes: this PR has six observable behaviour changes, including that filters now run more often by design. They are listed explicitly in a separate comment on this PR rather than left to be inferred from the diff. The over-selection item in particular is worth a second opinion.

2. AwsServletContext.getServletForPath read past the end of the request path

The matching loop was bounded by the length of the mapping while indexing into the request path, so any request with fewer segments than a registered mapping threw ArrayIndexOutOfBoundsException. The method is reached on every request through SpringBootLambdaContainerHandler, SpringLambdaContainerHandler and AwsProxyRequestDispatcher.

It also dereferenced its argument without a null check, and that argument is always getPathInfo(), which is null whenever the servlet path covered the whole request.

Both are fixed. The loop is now bounded by the request path as well, and a trailing wildcard still matches the empty remainder since the spec has /a/* match /a. A null path is treated as the root, which is the branch / already took.

Testing

• FilterChainManagerPathBypassTest (24 cases) covers the matcher and canonicalizePath directly: encoded characters, encoded separators, uppercase hex, dot segments leading both into and out of a protected path, double encoding decoding exactly once, malformed escapes not throwing, + surviving, and cache behaviour across encoded and plain requests. The key case asserts that filter selection never UNDER-selects: if any spelling could reach a servlet under a protected prefix, that prefix's filter must run. Over-selection is permitted. An earlier version asserted exact agreement between filter selection and servlet resolution, which review showed to be the wrong property.
• UrlPathValidatorTraversalTest (7 cases) covers plain, percent-encoded, uppercase and mixed-encoding traversal, and that a configured base path does not change the verdict. Four fail against a raw-URI version.
• AwsServletContextServletForPathTest (8 cases) covers short paths, null paths, wildcard-matches-bare-prefix, and the existing prefix behaviour.
• FilterAuthorizationBypassTest drives a Spring Boot app end to end through SpringBootLambdaContainerHandler.proxy(...), protected only by a deny-by-default servlet Filter with a path-scoped url-pattern. Parameterized over API Gateway REST, ALB and HTTP API v2, 33 cases. Covers spellings that resolve into the protected prefix and spellings that step out of it, the latter via a permanent /admin/** fixture, without which the app cannot demonstrate that class at all.

All tests were written before the fixes and confirmed to fail against the unpatched code.

Full local build green across core, jersey, spring, springboot4 and the three archetypes. Two pre-existing test failures remain on this branch and are unrelated to this change: AwsServletContextTest.getMimeType_mimeTypeOfJavascript_expectApplicationJavascript and AwsProxyHttpServletRequestTest.serverName_albHostHeader_returnsHostHeader. Both fail on unmodified main.

Related

The same production change applies to the other two maintained lines. The diffs to the main source files are identical across all three; only test scaffolding differs.

• 2.x: fix(lambda): filter path canonicalization 2.1.x #1629
• 1.x: fix(lambda): filter path canonicalization 1.x #1630

By submitting this pull request

• I confirm that my contribution is made under the terms of the Apache 2.0 license.
• I confirm that I've made a best effort attempt to update all relevant documentation.

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 75822fe..8299182
Files: 8
Comments: 1