diff --git a/.changeset/webdav-mkcol-percent-encoding-case.md b/.changeset/webdav-mkcol-percent-encoding-case.md new file mode 100644 index 00000000..c050fd38 --- /dev/null +++ b/.changeset/webdav-mkcol-percent-encoding-case.md @@ -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. diff --git a/conformance/webdav-qa.md b/conformance/webdav-qa.md index 460a890d..072d357b 100644 --- a/conformance/webdav-qa.md +++ b/conformance/webdav-qa.md @@ -165,6 +165,27 @@ curl -sS -X DELETE "https://conformance.dwk.io/dav-credentials?id= | 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 diff --git a/packages/webdav/src/webdav.test.ts b/packages/webdav/src/webdav.test.ts index fc921eaa..0c9986e3 100644 --- a/packages/webdav/src/webdav.test.ts +++ b/packages/webdav/src/webdav.test.ts @@ -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" }); @@ -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 diff --git a/packages/webdav/src/webdav.ts b/packages/webdav/src/webdav.ts index 139e6858..5189a4bc 100644 --- a/packages/webdav/src/webdav.ts +++ b/packages/webdav/src/webdav.ts @@ -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 @@ -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}/`)) {