Skip to content

Fix encoded path traversal corruption in sub-router handlers - #2900

Merged
tsegismont merged 1 commit into
vert-x3:masterfrom
tsegismont:issue/2898
May 22, 2026
Merged

Fix encoded path traversal corruption in sub-router handlers#2900
tsegismont merged 1 commit into
vert-x3:masterfrom
tsegismont:issue/2898

Conversation

@tsegismont

Copy link
Copy Markdown
Member

See #2898

When a request with encoded dot-segments (e.g. %2e%2e%2f) hit a handler mounted inside a sub-router via a regex or parameterized route, Utils.pathOffset corrupted the route-relative path because dot-segment resolution had already consumed the mount-point prefix.

Fix by running pathOffset on the raw normalizedPath first, then decoding and resolving dot-segments on the result.

Also remove the now redundant decodeURIComponent and removeDotSegments calls from StaticHandlerImpl.handle(), since normalizedPath() already decodes unreserved characters and resolves dot-segments, and file path resolution is now fully handled in getFile().

Some portions of this content were created with the assistance of Claude Code.

See vert-x3#2898

When a request with encoded dot-segments (e.g. %2e%2e%2f) hit a handler mounted inside a sub-router via a regex or parameterized route, Utils.pathOffset corrupted the route-relative path because dot-segment resolution had already consumed the mount-point prefix.

Fix by running pathOffset on the raw normalizedPath first, then decoding and resolving dot-segments on the result.

Also remove the now redundant decodeURIComponent and removeDotSegments calls from StaticHandlerImpl.handle(), since normalizedPath() already decodes unreserved characters and resolves dot-segments, and file path resolution is now fully handled in getFile().

Some portions of this content were created with the assistance of Claude Code.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes incorrect route-relative path computation for requests containing encoded dot-segments when handlers are mounted in a sub-router (notably for regex/parameterized routes), addressing the normalization/offset ordering described in #2898.

Changes:

  • Update TemplateHandlerImpl to apply Utils.pathOffset() before decoding and dot-segment removal.
  • Refactor StaticHandlerImpl to compute the route offset first and centralize decode/dot-segment removal in getFile().
  • Add regression tests covering StaticHandler and TemplateHandler behavior in sub-routers for wildcard, regex, and path-param routes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
vertx-web/src/test/java/io/vertx/ext/web/tests/SubRouterTest.java Adds regression tests ensuring encoded dot-segments don’t corrupt route-relative paths under sub-router mounts.
vertx-web/src/main/java/io/vertx/ext/web/handler/impl/TemplateHandlerImpl.java Reorders path processing so offset is computed before decode/dot-segment resolution.
vertx-web/src/main/java/io/vertx/ext/web/handler/impl/StaticHandlerImpl.java Moves decode/dot-segment resolution to operate on the offset path and simplifies handle() path preparation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tsegismont
tsegismont merged commit 0a39ef7 into vert-x3:master May 22, 2026
12 of 15 checks passed
@tsegismont
tsegismont deleted the issue/2898 branch May 22, 2026 08:09
tsegismont added a commit that referenced this pull request May 22, 2026
Follows-up on #2900

After moving URI decoding into getFile(), sendStatic() was using context.normalizedPath() (which may still contain percent-encoded characters) as the cache key.
Requests for the same file with different encodings (e.g. /foo/%62ar.txt vs /foo/bar.txt) created separate cache entries, wasting slots in the bounded LRU cache.

Use the decoded file path from getFile() as the cache key so that encoding-equivalent paths share a single entry.

As a side effect, getFile() is now called unconditionally at the top of sendStatic(), which simplifies the code.
Previously, it was deferred when includeHidden was true (skipping the dot-segment check), requiring a null guard and a second getFile() call for the localFile computation.

Some portions of this content were created with the assistance of Claude Code.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
tsegismont added a commit that referenced this pull request May 22, 2026
…2903)

See #2898

When a request with encoded dot-segments (e.g. %2e%2e%2f) hit a handler mounted inside a sub-router via a regex or parameterized route, Utils.pathOffset corrupted the route-relative path because dot-segment resolution had already consumed the mount-point prefix.

Fix by running pathOffset on the raw normalizedPath first, then decoding and resolving dot-segments on the result.

Also remove the now redundant decodeURIComponent and removeDotSegments calls from StaticHandlerImpl.handle(), since normalizedPath() already decodes unreserved characters and resolves dot-segments, and file path resolution is now fully handled in getFile().

Some portions of this content were created with the assistance of Claude Code.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
tsegismont added a commit that referenced this pull request May 22, 2026
…2904)

See #2898

When a request with encoded dot-segments (e.g. %2e%2e%2f) hit a handler mounted inside a sub-router via a regex or parameterized route, Utils.pathOffset corrupted the route-relative path because dot-segment resolution had already consumed the mount-point prefix.

Fix by running pathOffset on the raw normalizedPath first, then decoding and resolving dot-segments on the result.

Also remove the now redundant decodeURIComponent and removeDotSegments calls from StaticHandlerImpl.handle(), since normalizedPath() already decodes unreserved characters and resolves dot-segments, and file path resolution is now fully handled in getFile().

Some portions of this content were created with the assistance of Claude Code.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
tsegismont added a commit that referenced this pull request May 22, 2026
Follows-up on #2900

After moving URI decoding into getFile(), sendStatic() was using context.normalizedPath() (which may still contain percent-encoded characters) as the cache key.
Requests for the same file with different encodings (e.g. /foo/%62ar.txt vs /foo/bar.txt) created separate cache entries, wasting slots in the bounded LRU cache.

Use the decoded file path from getFile() as the cache key so that encoding-equivalent paths share a single entry.

As a side effect, getFile() is now called unconditionally at the top of sendStatic(), which simplifies the code.
Previously, it was deferred when includeHidden was true (skipping the dot-segment check), requiring a null guard and a second getFile() call for the localFile computation.

Some portions of this content were created with the assistance of Claude Code.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
tsegismont added a commit that referenced this pull request May 22, 2026
Follows-up on #2900

After moving URI decoding into getFile(), sendStatic() was using context.normalizedPath() (which may still contain percent-encoded characters) as the cache key.
Requests for the same file with different encodings (e.g. /foo/%62ar.txt vs /foo/bar.txt) created separate cache entries, wasting slots in the bounded LRU cache.

Use the decoded file path from getFile() as the cache key so that encoding-equivalent paths share a single entry.

As a side effect, getFile() is now called unconditionally at the top of sendStatic(), which simplifies the code.
Previously, it was deferred when includeHidden was true (skipping the dot-segment check), requiring a null guard and a second getFile() call for the localFile computation.

Some portions of this content were created with the assistance of Claude Code.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants