Skip to content

Commit ca7de07

Browse files
committed
fix(resolveDotSegments): preserve trailing slash on trailing dot segments
1 parent 9581407 commit ca7de07

4 files changed

Lines changed: 46 additions & 10 deletions

File tree

docs/2.utils/4.security.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,6 @@ Also decodes percent-encoded dot segments at any `%25`-nesting depth (`%2e`, `%2
140140

141141
`%2f`/`%5c` (encoded path separators) are left untouched by default — see {@link ResolveDotSegmentsOptions.decodeSlashes}.
142142

143-
Only `.`/`..` resolution and the decodes above alter the string; every other percent-encoding (`%20`, non-ASCII, `%3A`, and any `%2e` not forming a whole segment) is left intact, so the result stays in the same representation as an un-decoded `event.url.pathname` and matches routes/rules consistently. Interior empty segments are preserved (`/a//b` stays `/a//b`, per WHATWG URL normalization) — only the leading slash is guaranteed single, so a consumer doing exact prefix matching should normalize its allowlist the same way. To collapse them instead (the reading a slash-merging downstream resolves), see {@link ResolveDotSegmentsOptions.mergeSlashes}.
143+
Only `.`/`..` resolution and the decodes above alter the string; every other percent-encoding (`%20`, non-ASCII, `%3A`, and any `%2e` not forming a whole segment) is left intact, so the result stays in the same representation as an un-decoded `event.url.pathname` and matches routes/rules consistently. A trailing `.`/`..` resolves to a directory and keeps its trailing slash (`/a/b/..` -> `/a/`, `/a/.` -> `/a/`), per RFC 3986 §5.2.4 and matching what a WHATWG/nginx downstream resolves — so a scope check sees the directory form, not its file-form sibling. Interior empty segments are preserved (`/a//b` stays `/a//b`) — like WHATWG, this never merges slashes, so empty segments survive rather than collapsing. The one exception is a _leading_ run: it is always clamped to a single `/` (WHATWG would keep `//host`), so only the leading slash is guaranteed single and a consumer doing exact prefix matching should normalize its allowlist the same way. To collapse interior runs too (the reading a slash-merging downstream resolves), see {@link ResolveDotSegmentsOptions.mergeSlashes}.
144144

145145
<!-- /automd -->

src/utils/path.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,17 @@ const ENCODED_DOT_RE_G = /%(?:25)*2e/gi;
9393
* percent-encoding (`%20`, non-ASCII, `%3A`, and any `%2e` not forming a whole
9494
* segment) is left intact, so the result stays in the same representation as an
9595
* un-decoded `event.url.pathname` and matches routes/rules consistently.
96-
* Interior empty segments are preserved (`/a//b` stays `/a//b`, per WHATWG URL
97-
* normalization) — only the leading slash is guaranteed single, so a consumer
98-
* doing exact prefix matching should normalize its allowlist the same way. To
99-
* collapse them instead (the reading a slash-merging downstream resolves), see
100-
* {@link ResolveDotSegmentsOptions.mergeSlashes}.
96+
* A trailing `.`/`..` resolves to a directory and keeps its trailing slash
97+
* (`/a/b/..` -> `/a/`, `/a/.` -> `/a/`), per RFC 3986 §5.2.4 and matching what a
98+
* WHATWG/nginx downstream resolves — so a scope check sees the directory form,
99+
* not its file-form sibling.
100+
* Interior empty segments are preserved (`/a//b` stays `/a//b`) — like WHATWG,
101+
* this never merges slashes, so empty segments survive rather than collapsing.
102+
* The one exception is a *leading* run: it is always clamped to a single `/`
103+
* (WHATWG would keep `//host`), so only the leading slash is guaranteed single
104+
* and a consumer doing exact prefix matching should normalize its allowlist the
105+
* same way. To collapse interior runs too (the reading a slash-merging
106+
* downstream resolves), see {@link ResolveDotSegmentsOptions.mergeSlashes}.
101107
*/
102108
export function resolveDotSegments(path: string, opts?: ResolveDotSegmentsOptions): string {
103109
// Normalize to a single leading slash (treating a leading `\` as a
@@ -137,6 +143,7 @@ export function resolveDotSegments(path: string, opts?: ResolveDotSegmentsOption
137143
const normalizedSegment = segment.includes("%")
138144
? segment.replace(ENCODED_DOT_RE_G, ".")
139145
: segment;
146+
const isDotSegment = normalizedSegment === "." || normalizedSegment === "..";
140147
if (normalizedSegment === "..") {
141148
// Never pop past the root (first empty segment from leading slash)
142149
if (resolved.length > 1) {
@@ -148,9 +155,17 @@ export function resolveDotSegments(path: string, opts?: ResolveDotSegmentsOption
148155
// separators a `/{2,}` -> `/` collapse would remove. Skipping them here,
149156
// rather than collapsing the string first, is equivalent and lets a `..`
150157
// that an empty segment would otherwise shield pop its real parent.
151-
} else if (normalizedSegment !== ".") {
158+
} else if (!isDotSegment) {
152159
resolved.push(segment);
153160
}
161+
// A trailing `.`/`..` resolves to a directory, so preserve the trailing
162+
// slash by leaving an empty final segment (`/a/b/..` -> `/a/`, `/a/.` ->
163+
// `/a/`). This matches RFC 3986 §5.2.4 and what a WHATWG/nginx downstream
164+
// resolves, so a scope check does not see the file-form sibling of a path
165+
// the downstream serves the directory index of.
166+
if (isDotSegment && i === lastIndex) {
167+
resolved.push("");
168+
}
154169
}
155170
const result = resolved.join("/") || "/";
156171
// Decoded/normalized separators can prepend extra empty segments; collapse

src/utils/static.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,10 @@ export async function serveStatic(
106106
// reached instead of a 500. A `%`-free path (the common case) skips the
107107
// decode entirely, matching the fast-path guards at the event layer and in
108108
// `resolveDotSegments`.
109-
const resolvedId = resolveDotSegments(withoutTrailingSlash(event.url.pathname));
109+
// Strip the trailing slash AFTER resolving: a trailing `.`/`..` now resolves
110+
// to a directory-form path (`/a/b/..` -> `/a/`), and the on-disk id must stay
111+
// slash-free either way.
112+
const resolvedId = withoutTrailingSlash(resolveDotSegments(event.url.pathname));
110113
let originalId = resolvedId;
111114
if (resolvedId.includes("%")) {
112115
try {

test/unit/path.test.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,31 @@ describe("resolveDotSegments", () => {
107107

108108
it("preserves interior empty segments by default", () => {
109109
expect(resolveDotSegments("/api/orders//list.json")).toBe("/api/orders//list.json");
110-
// The empty segment shields the `..` from popping `a`
111-
expect(resolveDotSegments("/a//..")).toBe("/a");
110+
// The empty segment shields the `..` from popping `a` (it pops the empty
111+
// segment instead), leaving the directory-form `/a/`.
112+
expect(resolveDotSegments("/a//..")).toBe("/a/");
112113
expect(resolveDotSegments("/api/foo/%2e%2e/%2fadmin/secret", { decodeSlashes: true })).toBe(
113114
"/api//admin/secret",
114115
);
115116
});
116117

118+
it("preserves the trailing slash of a trailing dot segment (RFC 3986 §5.2.4)", () => {
119+
// A trailing `.`/`..` resolves to a directory: `/a/b/..` -> `/a/`, not `/a`,
120+
// matching what a WHATWG `URL`/nginx downstream resolves.
121+
expect(resolveDotSegments("/a/b/..")).toBe("/a/");
122+
expect(resolveDotSegments("/a/.")).toBe("/a/");
123+
expect(resolveDotSegments("/a/b/.")).toBe("/a/b/");
124+
// Encoded and nested forms resolve the same way.
125+
expect(resolveDotSegments("/a/b/%2e%2e")).toBe("/a/");
126+
expect(resolveDotSegments("/a/b/%252e")).toBe("/a/b/");
127+
// Popping down to the root still yields a single-slash root, not `//`.
128+
expect(resolveDotSegments("/a/..")).toBe("/");
129+
expect(resolveDotSegments("/..")).toBe("/");
130+
expect(resolveDotSegments("/a/../..")).toBe("/");
131+
// Merge mode preserves it too.
132+
expect(resolveDotSegments("/a/b/..", { mergeSlashes: true })).toBe("/a/");
133+
});
134+
117135
describe("mergeSlashes", () => {
118136
it("collapses runs of literal separators", () => {
119137
expect(resolveDotSegments("/api/orders//list.json", { mergeSlashes: true })).toBe(

0 commit comments

Comments
 (0)