fix(lambda): filter path canonicalization 1.x

[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, spark, spring, struts, springboot2 and the five archetypes. All 11 modules build with zero test failures, on both JDK 8 and JDK 11.

One workflow change is included, and it is not part of the fix. CI on this branch was already broken before this PR. gha_build.sh runs Gradle against the generated archetype projects and the samples at lines 50 and 71, those steps run under JDK 11, and the Gradle preinstalled on the runners now requires JVM 17 or later, so it refuses to start. All three jobs that set up JDK 11 died there, which is why Jersey, Spring and SpringBoot 2 were red while Spark and Struts, which set up no JDK at all, were green.

The fix is to pin Gradle 8.14 in those three jobs, since 8.x starts on JVM 11. Pinning the wrapper would not have worked, because the failure is the preinstalled launcher refusing to start before a wrapper exists. Verified locally: Gradle 8.14 runs gradle wrapper and ./gradlew clean build against samples/jersey/pet-store under JDK 11 and produces the expected pet-store.zip.

It is carried here rather than in its own PR so the repair is validated by this PR's own run. If reviewers would rather see it separately, say so and I will split it out.

Because those jobs never got past the Gradle step, nothing after it has run in CI since 2024-01-21. I reproduced the steps that exercise this change locally: core on JDK 8 (372 tests), Jersey 2.27 on JDK 8 (102), Spring 5.0 on JDK 11 (83), Spring Boot 2.2 on JDK 11 (78), all passing. The remaining matrix entries are unexercised and may hold unrelated rot.

Related

This is the 1.x backport. The production change is identical to the main PR. The test scaffolding differs in four ways, all forced by the older stack:

• javax.servlet imports instead of jakarta.servlet.
• AdminAuthorizationFilter implements init and destroy, because javax.servlet.Filter has no default methods.
• Spring Security's auto-configuration is excluded explicitly, since Boot 2 ships those classes and would otherwise secure every path in the test app.
• The end-to-end test lives in the springboot2 module rather than springboot4.

Review suggestion: review the main PR for the logic, and this one for the scaffolding delta.

• 3.x (main): fix(lambda): filter path canonicalization #1628
• 2.x: fix(lambda): filter path canonicalization 2.1.x #1629

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: df45e6e..ea389fd
Files: 12
Comments: 1

[aws-sam-tooling-bot]

Code Review Results

Reviewed: df45e6e..44236f4
Files: 14
Comments: 1