Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/webdav-mkcol-percent-encoding-case.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"@dwk/webdav": patch
---

Fix `MKCOL` silently succeeding over an existing plain resource when the
request's percent-encoding hex case differs from the `PUT` that created it
(litmus `mkcol_over_plain`, following `put_get_utf8_segment`). RFC 3986 §2.1
treats `%e2` and `%E2` as the same octet, but the router resolved each
request's path straight from `URL#pathname`, which copies an already-encoded
triplet through verbatim rather than normalizing its case — so two requests
naming the same UTF-8 segment with different encoder casing produced
different path strings and missed each other in the backend's exact-match
lookup. `pathOf` now uppercases every percent-encoded triplet before it's
used anywhere downstream (backend calls, authorization, lock/precondition
checks), so encoding-case no longer affects resource identity.
21 changes: 21 additions & 0 deletions conformance/webdav-qa.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,27 @@ curl -sS -X DELETE "https://conformance.dwk.io/dav-credentials?id=<credentialId>
| Invocation path | ☐ Local / ☑ CI ([run 30052950880](https://github.com/davidwkeith/workers/actions/runs/30052950880)) |
| Notes / follow-ups | This run followed #407 (fixed `mintAppPassword`'s PBKDF2 iteration count exceeding workerd's ceiling, which blocked credential minting entirely) and #409 (fixed four RFC 4918 conformance bugs: `MKCOL`/`PUT`/`COPY`/`MOVE` onto a missing parent silently succeeding instead of `409`, `MKCOL` over a plain resource silently succeeding instead of `405`, `DELETE` of a nonexistent resource silently succeeding instead of `404`). `basic` now passes 15/16 (up from 12/16 pre-#409, and 0/16 pre-#407); the one remaining failure is a narrower UTF-8-segment-reuse edge case in `mkcol_over_plain` — see the Step 3 table. `copymove`/`props`/`locks` are still unrun since litmus stops after the first group with failures. Filed as a residual gap, not a fresh regression — worth its own follow-up increment. |

## Follow-up: 2026-07-24 fix, re-run still needed

The `mkcol_over_plain` failure from the 2026-07-23 run (see **Result** →
Notes above) was root-caused without needing litmus's `debug.log`: `pathOf`
resolved each request's path straight from `URL#pathname`, which passes an
already-percent-encoded triplet through verbatim rather than normalizing its
case. `put_get_utf8_segment` and `mkcol_over_plain` name the same UTF-8
segment but litmus's own request construction gives the two requests
different percent-encoding hex case for it (e.g. `%e2%82%ac` vs
`%E2%82%AC`) — RFC 3986 §2.1 says these are the same octets, but the
backend's exact-string-match lookup didn't treat them that way, so the
`stat()` check in `mkcol()` missed the existing resource and let the
`MKCOL` through instead of 405ing. Fixed by uppercasing every
percent-encoded triplet in `pathOf` before the resolved path is used
anywhere downstream; covered by a new colocated unit test
(`webdav.test.ts`) reproducing the case-mismatch directly. Not yet
re-verified against the hosted target — this doc's **Result** table and
`status.json` stay at `failing`/`pending` until a fresh litmus run
(Step 3) confirms `basic` passes and `copymove`/`props`/`locks` get to run
for the first time.

## Recording the result

Fill in the **Result** table above first — date, tester, and which path was
Expand Down
48 changes: 48 additions & 0 deletions packages/webdav/src/webdav.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,18 @@ describe("createWebdav — MKCOL / COPY / MOVE (spec §3)", () => {
});
});

// litmus's real-world `mkcol_over_plain` run: PUT and MKCOL name the same
// UTF-8 segment but with different percent-encoding hex case
// (`%e2%82%ac` vs `%E2%82%AC`), which RFC 3986 §2.1 says are the same
// octets. A naive string-keyed backend lookup misses the existing resource
// and lets the MKCOL through instead of 405ing.
it("405s a MKCOL whose percent-encoding case differs from the PUT that created the resource", async () => {
await withHandler(async ({ call }) => {
await call("PUT", "/res-%e2%82%ac", { body: "x" });
expect((await call("MKCOL", "/res-%E2%82%AC")).status).toBe(405);
});
});

it("copies a resource and moves another, dropping the source", async () => {
await withHandler(async ({ call }) => {
await call("PUT", "/src.txt", { body: "data" });
Expand Down Expand Up @@ -933,6 +945,42 @@ describe("createWebdav — RFC 4918 conformance (§9/§10)", () => {
});
});

it("normalizes percent-encoding case in the configured mount path too, so it matches a request using different hex case", async () => {
// Mirrors the pathOf fix, but for the config side: `resolve()` must
// normalize `mountPath`/`baseUrl` the same way `pathOf` normalizes the
// request path, or a percent-encoded mount segment could itself drift
// out of sync with a differently-cased request and 404 spuriously.
const id = harness.WEBDAV_DO.idFromName(crypto.randomUUID());
const stub = harness.WEBDAV_DO.get(id);
await runInDurableObject(stub, async (instance) => {
const now = () => 1_000_000;
const backend = new MemBackend(instance.sql, now);
const cred = await backend.credentials.mint({
webid: WEBID,
label: "Finder",
scope: { modes: ["read", "write"] },
iterations: ITER,
});
const basic = `Basic ${btoa(`${cred.username}:${cred.secret}`)}`;
const handler = createWebdav({
baseUrl: "https://pod.example/caf%e9/",
mountPath: "/caf%e9",
backend: () => backend,
now,
});
const res = await handler(
new Request("https://pod.example/caf%E9/plain.txt", {
method: "PUT",
headers: { authorization: basic },
body: "x",
}),
{} as never,
createExecutionContext(),
);
expect(res.status).toBe(201);
});
});

it("never advertises DELETE on the mount root reached via a trailing slash", async () => {
// A sub-path baseUrl makes storageRoot `/dav/`, and `pathOf` resolves the
// trailing-slash mount root down to a bare `/`; the Allow set must still omit
Expand Down
29 changes: 24 additions & 5 deletions packages/webdav/src/webdav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,32 @@ interface Resolved {
readonly now: () => number;
}

/**
* Uppercase the hex digits of every percent-encoded triplet. RFC 3986 §2.1
* treats `%e2` and `%E2` as the same octet, but `URL`'s parser copies an
* already-encoded triplet through verbatim rather than normalizing its case —
* so two requests naming the same UTF-8 segment with different encoder hex
* casing (litmus `mkcol_over_plain` reusing `put_get_utf8_segment`'s
* resource) resolve to different path strings and miss each other in the
* backend's exact-match lookup. Applied to both the resolved mount
* config (here, in {@link resolve}) and every request path (in
* {@link pathOf}) so a percent-encoded segment in `mountPath`/`baseUrl`
* can't itself drift out of sync with a differently-cased request path.
*/
function normalizePercentEncoding(pathname: string): string {
return pathname.replace(/%[0-9a-fA-F]{2}/g, (triplet) =>
triplet.toUpperCase(),
);
}

function resolve(config: WebdavConfig): Resolved {
const url = new URL(config.baseUrl);
const rawMount = config.mountPath ?? "/";
const rawMount = normalizePercentEncoding(config.mountPath ?? "/");
const mount = rawMount.endsWith("/") ? rawMount.slice(0, -1) : rawMount;
const storageRoot = url.pathname.endsWith("/")
? url.pathname
: `${url.pathname}/`;
const rawStorageRoot = normalizePercentEncoding(url.pathname);
const storageRoot = rawStorageRoot.endsWith("/")
? rawStorageRoot
: `${rawStorageRoot}/`;
const litter =
config.denyOsLitter === true
? DEFAULT_OS_LITTER
Expand Down Expand Up @@ -319,7 +338,7 @@ function isWithinPathPrefix(path: string, prefix: string): boolean {

/** Map a request URL to a pod path, or `null` when outside the mount. */
function pathOf(url: URL, resolved: Resolved): string | null {
const { pathname } = url;
const pathname = normalizePercentEncoding(url.pathname);
if (resolved.mountPrefix === "") return pathname || "/";
if (pathname === resolved.mountPrefix) return resolved.storageRoot;
if (pathname.startsWith(`${resolved.mountPrefix}/`)) {
Expand Down
Loading