fix(lambda): filter path canonicalization 1.x by ckawl · Pull Request #1630 · aws/serverless-java-container

ckawl · 2026-10-01T18:20:42Z

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 any of three spellings: the canonical path, the decoded-but-not-normalized path, and the decoded path with path parameters removed from each segment. Each is load-bearing, and dropping any 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
without the parameter-free spelling, /admin;x=1/secret does not select it, because Spring removes ;x=1 before matching a handler while the other two spellings keep it. The servlet specification also has a container remove path parameters before matching url-patterns.

An earlier revision also matched the undecoded request URI. 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. Its ratio check counts .. segments, so it needs them visible: the raw URI leaves %2e%2e unrecognizable, and the canonical path has already resolved them away. That check is a heuristic that catches traversal above the root, not a general traversal guard, and the code now says so at the check. Nothing here relies on it for filter selection.
TargetCacheKey.equals compares the normalized path, dispatcher type and servlet name instead of hash codes. Comparing hash codes let two different paths that happened to share one reuse each other's cached filter chain. The trimming and trailing-slash handling that used to live only in hashCode now happens when the path is set, so existing cache behaviour is unchanged. This predates the PR but the PR relies on the cache to remember an authorization decision, so it is fixed here.
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 (26 cases) covers the matcher, canonicalizePath and stripPathParameters directly: encoded characters, encoded separators, uppercase hex, path parameters, 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.
AwsFilterChainManagerTest gains a case asserting that two cache keys with the same hash code but different paths are not equal. It pins every hash code to one value, so it tests the rule rather than depending on particular strings.
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, 39 cases. Covers spellings that resolve into the protected prefix, path parameters on the protected segment, 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.

With the pin, CI on this PR is green across all six jobs, including the Jersey, Spring and Spring Boot version matrix that had not run since 2024-01-21 because those jobs never got past the Gradle step.

Related

This is the 1.x backport. The production change is identical to the main PR apart from import lines. It also carries the Gradle pin in continuous-integration-workflow.yml described above, which the other two lines do not need. 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.

Filter selection keyed off getRequestURI(), which returns the raw undecoded path, while servlet resolution keys off getPathInfo(), which decodes. A percent-encoded or dot-segment spelling of a protected path therefore failed to select the filter mapped to that path while still routing to the servlet mapped to it, bypassing filter-based authorization. Canonicalize the path once (single percent-decode, then normalize away empty, "." and ".." segments) and use that for both matching and the filter chain cache key, so filter selection and servlet resolution can no longer disagree about which path is being requested. Decoding is not delegated to URLDecoder, which implements form encoding and would turn a literal "+" in a path segment into a space. Also make pathMatches consistently case-insensitive. Its exact-equality fast path always lowercased both sides while its segment comparison was case-sensitive, so the two halves disagreed and a differently-cased path could skip its filter. Over-selecting a filter is fail-safe; under-selecting one is an authorization bypass.

The mapping 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 via SpringBootLambdaContainerHandler, SpringLambdaContainerHandler and AwsProxyRequestDispatcher, so this was an unauthenticated crash path. Bound the loop by the request path as well. A trailing wildcard still matches the empty remainder, since the servlet spec has "/a/*" match "/a"; any other mapping segment cannot match a path that has run out. Also guard against a null path. The argument is always getPathInfo(), which is null whenever the servlet path covered the whole request, and the first statement dereferenced it. A null path is now treated as the root, which is the same branch "/" already took.

Port of the springboot3 test to the springboot2 module, adapted for the javax.servlet namespace. javax.servlet.Filter has no default init and destroy, unlike its jakarta counterpart, so the filter implements both. Same app, same assertions, same three request types. Spring Security auto-configuration is excluded as on 2.x, since Boot 2 does ship those classes and would otherwise secure every path.

…r matching Review feedback on aws#1628: resolving dot segments for filter matching alone reintroduced the filter/servlet disagreement in the opposite direction. "/admin/x/../../public" canonicalized to "/public" for filter selection, so a filter mapped to /admin/* was skipped, while getPathInfo() still returned the un-normalized "/admin/x/../../public" and so still matched a servlet mapped to /admin/*. Reproduced before and after: the filter ran on unmodified main and stopped running with the first version of this change. Canonicalization now backs both decisions. It moves to AwsHttpServletRequest alongside cleanUri and decodeRequestPath, and getPathInfo() returns the canonical path in both the API Gateway and HTTP API v2 request types, so filter selection and servlet resolution read the same path by construction. Decoding only, without resolving dot segments, would have closed the reported direction and left its mirror: a filter mapped to /public/* would be skipped for "/admin/x/../../public" while the framework routed to /public. UrlPathValidator now inspects getRequestURI() instead of getPathInfo(). It rejects paths containing traversal segments, and reading the canonical path would have normalized them away before it could see them. A validator of suspicious input has to inspect the input as it arrived. Adds four tests: a dot segment leading out of a protected path, the same percent-encoded, a dot segment leading into one, and a case asserting that filter selection and servlet resolution agree for every spelling covered. Four of them fail without the getPathInfo change.

aws-sam-tooling-bot

Code Review Results

Reviewed: df45e6e..ea389fd
Files: 12
Comments: 1

Review feedback on aws#1628: decodePathSegments converted non-escape characters to bytes one UTF-16 char at a time. A character outside the BMP is stored as a surrogate pair, and a lone surrogate cannot be encoded, so each half became the replacement byte and the character came out as "??". Reproduced: "/<U+1F600>/%61dmin" decoded to "/??/admin". Only triggered when the path also contains a '%', since the early return otherwise skips decoding entirely. BMP characters were unaffected. This reached the application, not just filter matching, because getPathInfo() now returns the canonical path in both request types. The URLDecoder call this replaced did not have the problem. A surrogate pair is now written as a unit. Adds tests for an emoji, a CJK Extension B character, BMP multi-byte characters, and unpaired surrogates, which must not throw.

Review feedback on aws#1630. Three forms of a request path matter, not two, and the previous revision conflated them. UrlPathValidator needs the path decoded but NOT normalized. The comment added in the previous revision was wrong: at the merge base getPathInfo() returned decodeRequestPath(cleanUri(path)), which decodes while leaving dot segments in place, so the validator did see "%2e%2e" as "..". Switching it to getRequestURI() left the escapes encoded, so "/%2e%2e/%2e%2e/x" scored zero dot segments and passed. It now reads a decode-only form. The context path is also excluded from what the validator inspects. It is configured rather than client-supplied, and it contributes slashes without contributing dot segments, which loosens the ratio check: "/../x" was rejected while "/prod/../x" passed. decodePath is the new decode-only helper. It gathers consecutive escapes and decodes them as a group using the configured uriEncoding, which the previous revision ignored in favour of hardcoded UTF-8, and copies literal characters straight through, so a surrogate pair can no longer be split by construction rather than by a special case. Filter matching now reads request.getPathInfo() rather than canonicalizing getRequestURI(). getRequestURI() includes the context path while getPathInfo() does not, and url-patterns are relative to the context, so with a configured stage or base path that mismatch alone could skip a path-scoped filter the servlet still matched. Filter selection and servlet resolution are now the same call, so they cannot drift apart. AwsHttpServletRequestWrapper.getPathInfo also returns the canonical path. It had kept a copy of the pre-fix logic, and it is reached on async dispatch, where AwsProxyRequestDispatcher resolves the servlet from getPathInfo. decodeRequestPath had no callers left and is removed. Adds UrlPathValidatorTraversalTest, seven cases covering encoded, uppercase and mixed-encoding traversal plus the context path. Four fail against the raw-URI version. Also adds a wrapper consistency test.

aws-sam-tooling-bot

Code Review Results

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

Review feedback on aws#1630. Resolving dot segments for both filter selection and servlet resolution still left a bypass, because Spring MVC routes on getRequestURI(), which is neither decoded nor normalized, and it leaves dot segments to a servlet container that does not exist in Lambda. Reproduced through the shipped Lambda entry point against an app with an /admin/** handler and a filter mapped /admin/*: /admin/../public/info 200 WILDCARD_TOP_SECRET filters=0 /admin/secret%2F..%2F..%2Fpublic%2Finfo 200 WILDCARD_TOP_SECRET filters=0 /admin/x/../../public/info 200 WILDCARD_TOP_SECRET filters=0 All three canonicalize to /public/info, so the /admin/* filter was not selected, while Spring matched /admin/** on the raw path and served the handler. The filter ran for all three before this PR. Consumers do not agree on which form of the path they route on: servlet resolution here uses the canonical path, Spring MVC uses the raw request URI, and other code reads the decoded-but-not-normalized form. Choosing one and assuming the rest follow is what produced this bug class repeatedly, so filter selection no longer chooses. A filter applies if its url-pattern matches under the raw, decoded, or canonical spelling. This over-selects by design. A filter may run for a request that is routed elsewhere, which is harmless for an authorization filter and a behaviour change for a filter that transforms requests or responses. Under-selecting is the authorization bypass. Matching two forms is not enough: "/%61dmin/../public" matches /admin/* in its decoded-but-not-normalized form while matching neither raw nor canonical. Not taken from the review: building getRequestURI() from the canonical form would undo the UrlPathValidator fix, which needs the raw path to see traversal segments, and conflicts with the spec requirement that getRequestURI() returns the URI as sent. Leaving a decoded %2F as a non-separator was also not taken, because under over-selection decoding it makes the protective filter more likely to apply. Tests: the invariant changed from "filter selection and servlet resolution agree" to "filter selection never under-selects", which is the property that matters. filterSelectionNeverUnderSelects covers 13 spellings, with a companion asserting unrelated paths are not swept in. AdminWildcardController is now a permanent fixture, since without an /admin/** handler the end-to-end app cannot demonstrate this class at all, and three end-to-end cases cover the step-out spellings across API Gateway, ALB and HTTP API v2.

The third arm of the union matched the raw path, and it was not earning its place. A brute-force sweep of 688,908 (path, url-pattern) pairs found 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, for example /adm%69n/*. The servlet specification matches url-patterns against the decoded path, so such a pattern would not match in a real container either. Dropping it means filter selection over-selects on two spellings instead of three, which is a smaller behaviour change to justify, and nothing changes for any pattern anyone would realistically register. The comment also credited the raw arm with covering Spring MVC. That was wrong. Spring MVC routes on the undecoded request URI, and the decoded-but-not- normalized arm is what covers it: matching on the canonical path alone leaves /admin/../public/info reaching an /admin/** handler with its filter skipped.

gha_build.sh runs Gradle against the generated archetype projects and the samples at lines 50 and 71. The Gradle preinstalled on the GitHub runners now requires JVM 17 or later, so on this branch it refuses to start and the job dies before any build runs. Every job that sets up JDK 11 failed there, which is why Jersey, Spring and SpringBoot 2 were red while Spark and Struts, which set up no JDK at all, stayed green. Gradle 8.x starts on JVM 11, so pinning 8.14 for those three jobs keeps the Gradle steps on the same JDK as the rest of the job. Pinning the wrapper instead would not help: the failure is the preinstalled launcher refusing to start, which happens before a wrapper exists. Verified locally that Gradle 8.14 under JDK 11 runs both `gradle wrapper` and `./gradlew clean build` against samples/jersey/pet-store and produces the expected pet-store.zip. This is pre-existing rot rather than anything to do with the filter fix in this branch, and is carried here so the repair is validated by this PR's own run.

bnusunny · 2026-10-08T21:41:35Z

I reviewed this at 5c20033c alongside #1628. The production diff matches #1628 apart from import lines in AwsHttpServletRequest.java, and the gaps I found there all reproduce here. Full write-up: #1628 (comment)

On JDK 11 locally, core passed all 375 tests and springboot2 passed all 79. The two main failures don't occur on 1.x. GitHub CI is green.

The gaps, confirmed on this branch:

Path parameters skip the filter. /admin;x=1/secret and /admin;/secret return TOP_SECRET with filterRan=false on API Gateway REST, ALB and HTTP API v2, using this PR's springboot2 filterauthapp. Spring 5 strips ;... before it matches a handler, but pathMatches compares the whole segment admin;x=1.
Hash-only TargetCacheKey.equals (FilterChainManager.java:345). With a constructed collision, a request to a public path cached an empty chain under an /admin/... path (1 filter, then 0 filters).
UrlPathValidator only rejects shallow traversal. /../admin/secret, /%2e%2e/admin/secret and /a/b/../admin/secret all pass.

One small correction to the description: it says only the test scaffolding differs between the three lines, but this PR also changes .github/workflows/continuous-integration-workflow.yml (the Gradle 8.14 pin).

… by value Review follow-ups. Path parameters. Spring removes everything from ";" to the end of a segment before it matches a handler, and the servlet specification has a container do the same before matching url-patterns. Neither the canonical nor the decoded spelling removed them, so a filter mapped to /admin/* was not selected for /admin;x=1/secret while the request still reached the /admin/secret handler. Filter selection now also matches the decoded path with path parameters removed from each segment. Like the other two spellings, this over-selects. Cache key equality. TargetCacheKey.equals compared hash codes rather than fields, so two different paths that happened to share a hash code shared a cached filter chain. equals now compares the normalized target path, dispatcher type and servlet name. The trimming and trailing-slash handling that used to live only inside hashCode, and so was part of equality by accident, now happens when the path is set, so existing cache behaviour is unchanged. Smaller items: - UrlPathValidator: say at the ratio check that it is a heuristic rather than a traversal guard, and why stripContextPath does not reuse the shared helper. - canonicalizePath and contextRelativeRequestUri are package-private. Only decodePath is used outside the package.

…o end The path parameter cases were only in the loop tests, which assert that the secret is not returned and stop at the first failing path. These two tests assert what review checked: a 403 from the filter, with the filter invoked, for each spelling separately on API Gateway REST, ALB and HTTP API v2. Both fail without the parameter-free matching arm.

…header The header said the validator rejects traversal attempts up front, which is the same overclaim review flagged in the PR description. Its ratio check only catches traversal above the root.

ckawl · 2026-10-12T20:53:40Z

Fixed in 9093b19 with the extra end-to-end test in 212f6b2, both cherry-picked from #1628. Details are in the reply there: #1628

The only conflict was the import block, since 1.x never had the redundant ArrayDeque and Deque imports. Thanks for catching the description. It now says this PR also carries the Gradle pin.

Locally on JDK 11: core 375, jersey 102, spring 83, springboot2 84, no failures. Core also passes on JDK 8. Both new path parameter tests fail here too with the fix removed.

ckawl added 4 commits September 22, 2026 18:19

This was referenced Oct 1, 2026

fix(lambda): filter path canonicalization #1628

Open

fix(lambda): filter path canonicalization 2.1.x #1629

Open

aws-sam-tooling-bot Bot reviewed Oct 1, 2026

View reviewed changes

Comment thread .../src/main/java/com/amazonaws/serverless/proxy/internal/servlet/filters/UrlPathValidator.java Outdated

ckawl added 2 commits October 1, 2026 18:52

aws-sam-tooling-bot Bot reviewed Oct 1, 2026

View reviewed changes

Comment thread ...r-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java Outdated

ckawl added 2 commits October 2, 2026 18:52

ckawl marked this pull request as ready for review October 2, 2026 22:17

ckawl added 3 commits October 12, 2026 18:31

test: describe UrlPathValidator as a heuristic in the traversal test …

9ad324d

…header The header said the validator rejects traversal attempts up front, which is the same overclaim review flagged in the PR description. Its ratio check only catches traversal above the root.

Navigation Menu

Repository navigation

fix(lambda): filter path canonicalization 1.x - #1630

fix(lambda): filter path canonicalization 1.x#1630
ckawl wants to merge 12 commits into
aws:1.xfrom
ckawl:fix/filter-path-canonicalization-1.x

ckawl commented Oct 1, 2026 •

edited

Loading

Uh oh!

aws-sam-tooling-bot Bot left a comment

Uh oh!

Uh oh!

aws-sam-tooling-bot Bot left a comment

Uh oh!

Uh oh!

bnusunny commented Oct 8, 2026

Uh oh!

ckawl commented Oct 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ckawl commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aws-sam-tooling-bot Bot left a comment

Choose a reason for hiding this comment

Code Review Results

Uh oh!

Uh oh!

aws-sam-tooling-bot Bot left a comment

Choose a reason for hiding this comment

Code Review Results

Uh oh!

Uh oh!

bnusunny commented Oct 8, 2026

Uh oh!

ckawl commented Oct 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ckawl commented Oct 1, 2026 •

edited

Loading