From 259b5b69efad6d4a4599e68e3a50fec5f3d0b7e4 Mon Sep 17 00:00:00 2001 From: jenken827 Date: Sat, 12 Sep 2026 12:27:01 +0800 Subject: [PATCH] fix(sync): create missing WebDAV directories before upload (404 on device snapshot) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WebDAV sync failed with "WebDAV PUT failed for /readany/sync/device-xxx.json: 404 Not Found" on servers where ensureDirectories() silently no-oped: mkcol treated 409 (intermediate collection missing) as success, and ensureDirectory skipped creation whenever the error message merely contained "405"/"409". The device snapshot upload also bypassed the ensure-parent-and-retry fallback that put()/putFile() had, so the 404 surfaced straight to the UI. - WebDavClient.put(): on 404/409 force-create the parent collection chain and retry once, so every upload path (put/putJSON) self-heals - WebDavClient.mkcol(): 409 no longer counts as success — recovery does not trust the PROPFIND probe (real servers 409 while the parent probes as existing): force-MKCOL every ancestor plus the target (RFC 4918 9.3.1), retrying each once without the trailing slash for gateways that mishandle collection URIs - WebDavClient.ensureDirectory(): drop error-message string matching; memoize confirmed collections to cut redundant PROPFIND probes - WebDavClient.get/getText/delete/move: attach the HTTP status to failures (WebDavError, message format unchanged) so getJSON() detects 404 by status instead of matching "404" anywhere in the message/path - WebDavBackend: share one ensure-parent-and-retry helper across put/putFile/putJSON (putJSON previously had none) and create the legacy file/cover directories via ensureDirectory instead of bare mkcol --- packages/core/src/sync/webdav-backend.test.ts | 98 +++++++++++++ packages/core/src/sync/webdav-backend.ts | 52 ++++--- packages/core/src/sync/webdav-client.test.ts | 115 +++++++++++++++ packages/core/src/sync/webdav-client.ts | 133 +++++++++++++++--- 4 files changed, 356 insertions(+), 42 deletions(-) create mode 100644 packages/core/src/sync/webdav-backend.test.ts diff --git a/packages/core/src/sync/webdav-backend.test.ts b/packages/core/src/sync/webdav-backend.test.ts new file mode 100644 index 000000000..985e42952 --- /dev/null +++ b/packages/core/src/sync/webdav-backend.test.ts @@ -0,0 +1,98 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { type FetchOptions, type IPlatformService, setPlatformService } from "../services/platform"; +import { DEFAULT_WEBDAV_REMOTE_ROOT, type WebDavConfig } from "./sync-backend"; +import { WebDavBackend } from "./webdav-backend"; + +function installFetchStub( + handler: (url: string, options?: FetchOptions) => Response | Promise, +): void { + setPlatformService({ + platformType: "web", + isMobile: false, + isDesktop: false, + fetch: handler, + } as unknown as IPlatformService); +} + +function webDavConfig(): WebDavConfig { + return { + type: "webdav", + url: "https://dav.example.com/dav", + username: "alice", + remoteRoot: DEFAULT_WEBDAV_REMOTE_ROOT, + allowInsecure: false, + autoSync: false, + syncIntervalMins: 30, + wifiOnly: false, + notifyOnComplete: false, + }; +} + +describe("WebDavBackend directory healing", () => { + afterEach(() => { + setPlatformService(null as unknown as IPlatformService); + }); + + it("putJSON creates the missing /readany/sync collection and retries instead of surfacing the 404", async () => { + const calls: { method: string; url: string }[] = []; + installFetchStub((url, options) => { + const method = String(options?.method ?? "GET"); + calls.push({ method, url }); + if (method === "PUT") { + const putCount = calls.filter((call) => call.method === "PUT").length; + return new Response("", { status: putCount === 1 ? 404 : 200 }); + } + if (method === "PROPFIND") { + return new Response("", { status: url.endsWith("/readany/") ? 207 : 404 }); + } + if (method === "MKCOL") { + return new Response("", { status: 201 }); + } + return new Response("", { status: 404 }); + }); + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + try { + const backend = new WebDavBackend(webDavConfig(), "secret"); + await backend.putJSON("/readany/sync/device-abc.json", { deviceId: "abc" }); + } finally { + warnSpy.mockRestore(); + logSpy.mockRestore(); + } + + expect(calls.filter((call) => call.method === "PUT")).toHaveLength(2); + expect( + calls.some((call) => call.method === "MKCOL" && call.url.endsWith("/readany/sync/")), + ).toBe(true); + }); + + it("ensureDirectories creates the legacy file/cover collections through ensureDirectory", async () => { + const calls: { method: string; url: string }[] = []; + installFetchStub((url, options) => { + const method = String(options?.method ?? "GET"); + calls.push({ method, url }); + if (method === "PROPFIND") { + return new Response("", { status: 404 }); + } + if (method === "MKCOL") { + return new Response("", { status: 201 }); + } + return new Response("", { status: 404 }); + }); + + const backend = new WebDavBackend(webDavConfig(), "secret"); + await backend.ensureDirectories(); + + const mkcolUrls = calls.filter((call) => call.method === "MKCOL").map((call) => call.url); + expect(mkcolUrls).toEqual([ + "https://dav.example.com/dav/readany/", + "https://dav.example.com/dav/readany/sync/", + "https://dav.example.com/dav/readany/data/", + "https://dav.example.com/dav/readany/data/books/", + "https://dav.example.com/dav/readany/data/file/", + "https://dav.example.com/dav/readany/data/cover/", + ]); + }); +}); diff --git a/packages/core/src/sync/webdav-backend.ts b/packages/core/src/sync/webdav-backend.ts index bec95b389..01f6ef39a 100644 --- a/packages/core/src/sync/webdav-backend.ts +++ b/packages/core/src/sync/webdav-backend.ts @@ -77,46 +77,50 @@ export class WebDavBackend implements ISyncBackend { await this.client.ensureDirectory(this.resolvePath(REMOTE_DATA)); // New per-book layout root await this.client.ensureDirectory(this.resolvePath(REMOTE_BOOKS_ROOT)); - // Legacy directories kept ensured during the transition window (cheap & safe) - await this.client.mkcol(this.resolvePath(REMOTE_FILES)); - await this.client.mkcol(this.resolvePath(REMOTE_COVERS)); + // Legacy directories kept ensured during the transition window (cheap & safe). + // ensureDirectory (not bare mkcol): some servers answer 409 to MKCOL when + // an intermediate collection is missing, which must not be read as success. + await this.client.ensureDirectory(this.resolvePath(REMOTE_FILES)); + await this.client.ensureDirectory(this.resolvePath(REMOTE_COVERS)); this.directoriesEnsured = true; } - async put(path: string, data: Uint8Array): Promise { + /** + * Some WebDAV servers (Synology, QNAP, 飞牛, etc.) return 403/404/409 when + * PUT-ing into a directory that doesn't exist yet. Ensure the parent and + * retry once — most uploads land on an existing dir, so this catch path + * only fires for first-time uploads into a brand-new folder. Shared by + * every upload entry point (put/putFile/putJSON) so no caller can bypass it. + */ + private async putWithDirectoryHeal( + path: string, + write: (resolvedPath: string) => Promise, + ): Promise { const resolved = this.resolvePath(path); try { - await this.client.put(resolved, data); + await write(resolved); } catch (e) { - // Some WebDAV servers (Synology, QNAP, 飞牛, etc.) return 403/404/409 when - // PUT-ing into a directory that doesn't exist yet. Ensure the parent and - // retry once — most uploads land on an existing dir, so this catch path - // only fires for first-time uploads into a brand-new per-book folder. const message = e instanceof Error ? e.message : String(e); if (!/\b(403|404|409)\b/.test(message)) throw e; const parent = resolved.substring(0, resolved.lastIndexOf("/")); if (!parent || parent === "/") throw e; await this.client.ensureDirectory(parent); - await this.client.put(resolved, data); + await write(resolved); } } + async put(path: string, data: Uint8Array): Promise { + await this.putWithDirectoryHeal(path, (resolved) => this.client.put(resolved, data)); + } + async putFile( path: string, localFilePath: string, onProgress?: (loaded: number, total: number) => void, ): Promise { - const resolved = this.resolvePath(path); - try { - await this.client.putFile(resolved, localFilePath, onProgress); - } catch (e) { - const message = e instanceof Error ? e.message : String(e); - if (!/\b(403|404|409)\b/.test(message)) throw e; - const parent = resolved.substring(0, resolved.lastIndexOf("/")); - if (!parent || parent === "/") throw e; - await this.client.ensureDirectory(parent); - await this.client.putFile(resolved, localFilePath, onProgress); - } + await this.putWithDirectoryHeal(path, (resolved) => + this.client.putFile(resolved, localFilePath, onProgress), + ); } async get(path: string): Promise { @@ -143,7 +147,11 @@ export class WebDavBackend implements ISyncBackend { } async putJSON(path: string, data: T): Promise { - await this.client.putJSON(this.resolvePath(path), data); + // Must go through the shared heal path: this is how device snapshots and + // the file manifest get written, and a missing /readany/sync used to + // surface here as "WebDAV PUT failed … 404" because client.putJSON + // bypassed the retry that put()/putFile() had. + await this.putWithDirectoryHeal(path, (resolved) => this.client.putJSON(resolved, data)); } async listDir(path: string): Promise { diff --git a/packages/core/src/sync/webdav-client.test.ts b/packages/core/src/sync/webdav-client.test.ts index 2cf079f4a..c0b80f12d 100644 --- a/packages/core/src/sync/webdav-client.test.ts +++ b/packages/core/src/sync/webdav-client.test.ts @@ -204,6 +204,121 @@ describe("WebDavClient PROPFIND parsing", () => { }); }); +describe("WebDavClient PUT/mkcol self-healing", () => { + afterEach(() => { + setPlatformService(null as unknown as IPlatformService); + }); + + it("heals a PUT 404 by force-creating the parent collection chain and retrying once", async () => { + const calls: { method: string; url: string }[] = []; + installFetchStub((url, options) => { + const method = String(options?.method ?? "GET"); + calls.push({ method, url }); + if (method === "PUT") { + const putCount = calls.filter((call) => call.method === "PUT").length; + return new Response("", { status: putCount === 1 ? 404 : 200 }); + } + if (method === "MKCOL") { + return new Response("", { status: 201 }); + } + return new Response("", { status: 404 }); + }); + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + try { + const client = new WebDavClient("https://dav.example.com/dav", "alice", "secret"); + await client.putJSON("/readany/sync/device-abc.json", { ok: true }); + } finally { + warnSpy.mockRestore(); + logSpy.mockRestore(); + } + + expect(calls.map((call) => call.method)).toEqual(["PUT", "MKCOL", "MKCOL", "PUT"]); + expect(calls[1]?.url).toBe("https://dav.example.com/dav/readany/"); + expect(calls[2]?.url).toBe("https://dav.example.com/dav/readany/sync/"); + expect(calls[0]?.url).toBe("https://dav.example.com/dav/readany/sync/device-abc.json"); + expect(calls[3]?.url).toBe(calls[0]?.url); + }); + + it("heals an MKCOL 409 by force-creating the whole collection chain", async () => { + const calls: { method: string; url: string }[] = []; + installFetchStub((url, options) => { + const method = String(options?.method ?? "GET"); + calls.push({ method, url }); + if (method === "PROPFIND") { + return new Response("", { status: 207 }); + } + if (method === "MKCOL") { + const isBooksCall = url.endsWith("/readany/data/books/"); + const booksAttempts = calls.filter( + (call) => call.method === "MKCOL" && call.url.endsWith("/readany/data/books/"), + ).length; + return new Response("", { status: isBooksCall && booksAttempts === 1 ? 409 : 201 }); + } + return new Response("", { status: 404 }); + }); + + const client = new WebDavClient("https://dav.example.com/dav", "alice", "secret"); + await client.mkcol("/readany/data/books"); + + expect(calls.map((call) => call.method)).toEqual(["MKCOL", "MKCOL", "MKCOL", "MKCOL"]); + expect(calls.map((call) => call.url)).toEqual([ + "https://dav.example.com/dav/readany/data/books/", + "https://dav.example.com/dav/readany/", + "https://dav.example.com/dav/readany/data/", + "https://dav.example.com/dav/readany/data/books/", + ]); + }); + + it("retries MKCOL without the trailing slash on servers that 409 collection URIs", async () => { + const calls: { method: string; url: string }[] = []; + installFetchStub((url, options) => { + const method = String(options?.method ?? "GET"); + calls.push({ method, url }); + if (method === "MKCOL") { + return new Response("", { status: url.endsWith("/") ? 409 : 201 }); + } + return new Response("", { status: 404 }); + }); + + const client = new WebDavClient("https://dav.example.com/dav", "alice", "secret"); + await client.mkcol("/readany/sync"); + + expect(calls.map((call) => call.method)).toEqual(["MKCOL", "MKCOL", "MKCOL", "MKCOL", "MKCOL"]); + expect(calls.map((call) => call.url)).toEqual([ + "https://dav.example.com/dav/readany/sync/", + "https://dav.example.com/dav/readany/", + "https://dav.example.com/dav/readany", + "https://dav.example.com/dav/readany/sync/", + "https://dav.example.com/dav/readany/sync", + ]); + }); + + it("getJSON returns null for 404 by status and rethrows other failures", async () => { + installFetchStub(() => new Response("", { status: 404 })); + const client = new WebDavClient("https://dav.example.com/dav", "alice", "secret"); + await expect(client.getJSON("/readany/sync/missing.json")).resolves.toBeNull(); + + installFetchStub(() => new Response("", { status: 401 })); + await expect(client.getJSON("/readany/sync/missing.json")).rejects.toThrow(); + }); + + it("skips re-probing collections confirmed earlier in the same client", async () => { + const calls: { method: string; url: string }[] = []; + installFetchStub((url, options) => { + calls.push({ method: String(options?.method ?? "GET"), url }); + return new Response("", { status: 207 }); + }); + + const client = new WebDavClient("https://dav.example.com/dav", "alice", "secret"); + await client.ensureDirectory("/readany"); + await client.ensureDirectory("/readany"); + + expect(calls).toHaveLength(1); + }); +}); + describe("sanitizeWebDavRemoteRoot", () => { it("preserves case because WebDAV paths can be case-sensitive", () => { expect(sanitizeWebDavRemoteRoot("ReadAny/DeviceSync")).toBe("ReadAny/DeviceSync"); diff --git a/packages/core/src/sync/webdav-client.ts b/packages/core/src/sync/webdav-client.ts index e74aad369..f46532676 100644 --- a/packages/core/src/sync/webdav-client.ts +++ b/packages/core/src/sync/webdav-client.ts @@ -253,6 +253,12 @@ export class WebDavClient { * failure. Reset per WebDavClient instance. */ private hadAuthSuccess = false; + /** + * Collections confirmed to exist during this client's lifetime (PROPFIND + * probe or successful MKCOL). Lets ensureDirectory() skip re-probing paths + * it created minutes ago and keeps the retry helpers cheap. + */ + private knownCollections = new Set(); constructor(url: string, username: string, password: string, allowInsecure?: boolean) { // Normalize: remove control chars/whitespace and trailing slash @@ -419,46 +425,92 @@ export class WebDavClient { } catch (e) { if (await this.collectionExists(collectionPath)) { console.warn(`[WebDAV] MKCOL ${path} failed but directory exists; continuing`); + this.knownCollections.add(collectionPath); return; } throw e; } const status = resp.status; if (resp.ok || status === 201) { + this.knownCollections.add(collectionPath); return; } - if (status === 405 || status === 409) { + if (status === 405) { + // RFC 4918 §9.3: the resource already exists. + return; + } + if (status === 409) { + // RFC 4918 §9.3.1: an intermediate collection is missing. Real-world + // servers also answer 409 when the PROPFIND probe wrongly reports the + // parent as existing, or when they mishandle the trailing slash on + // MKCOL, so recovery must not trust the probe: force-MKCOL every + // ancestor (405 = already exists) plus the target itself, retrying + // each once without the trailing slash. + await this.mkcolChain(collectionPath); return; } if ((status === 401 || status === 403) && (await this.collectionExists(collectionPath))) { console.warn(`[WebDAV] MKCOL ${path} returned ${status} but directory exists; continuing`); + this.knownCollections.add(collectionPath); return; } throw new Error(`WebDAV MKCOL failed for ${path}: ${status} ${resp.statusText || ""}`); } + /** + * MKCOL every path segment including the target, tolerating 405 (already + * exists). Unlike ensureDirectory() this does not trust PROPFIND probes — + * it is used on recovery paths where a probe has been proven wrong (409 on + * a collection whose parent probe reports as existing) and for PUT + * self-healing after a 404/409. + */ + private async mkcolChain(path: string): Promise { + const segments = path.split("/").filter(Boolean); + let current = ""; + for (const segment of segments) { + current += `/${segment}`; + const collectionPath = toCollectionPath(current); + if (this.knownCollections.has(collectionPath)) continue; + let resp = await this.request("MKCOL", collectionPath); + if (!resp.ok && resp.status !== 201 && resp.status !== 405) { + // Some servers and gateways mishandle the trailing slash on MKCOL. + resp = await this.request("MKCOL", current); + } + if (resp.ok || resp.status === 201 || resp.status === 405) { + this.knownCollections.add(collectionPath); + continue; + } + if ( + (resp.status === 401 || resp.status === 403) && + (await this.collectionExists(collectionPath)) + ) { + this.knownCollections.add(collectionPath); + continue; + } + throw new Error( + `WebDAV MKCOL failed for ${current}: ${resp.status} ${resp.statusText || ""}`, + ); + } + } + /** Ensure a full directory path exists (creates each segment) */ async ensureDirectory(path: string): Promise { const segments = path.split("/").filter(Boolean); let current = ""; for (const segment of segments) { current += `/${segment}`; + const collectionPath = toCollectionPath(current); + if (this.knownCollections.has(collectionPath)) continue; if ( - await this.propfindExists(toCollectionPath(current), { + await this.propfindExists(collectionPath, { timeoutMs: DIRECTORY_PROBE_TIMEOUT_MS, }) ) { continue; } - try { - await this.mkcol(current); - } catch (e: unknown) { - const err = e as { message?: string }; - if (err.message?.includes("405") || err.message?.includes("409")) { - continue; - } - throw e; - } + // mkcol() self-heals a 409 (missing parent) by creating the parent + // chain; any error it throws here is a real failure and must surface. + await this.mkcol(current); } } @@ -468,10 +520,28 @@ export class WebDavClient { data: string | Uint8Array | ArrayBuffer, contentType = "application/octet-stream", ): Promise { - const resp = await this.request("PUT", path, { + let resp = await this.request("PUT", path, { body: data, contentType, }); + if (!resp.ok && (resp.status === 404 || resp.status === 409)) { + // PUT never creates parent collections. First upload into a directory + // that doesn't exist yet (e.g. /readany/sync on a fresh WebDAV account) + // fails with 404 on many servers and 409 on the rest — force-create the + // parent chain and retry once so every upload path (put/putJSON/…) + // self-heals instead of bubbling a 404 to the UI. + const parent = getParentCollectionPath(path)?.parent; + if (parent && parent !== "/") { + console.warn( + `[WebDAV] PUT ${path} got ${resp.status}; ensuring parent collection and retrying once`, + ); + await this.mkcolChain(parent); + resp = await this.request("PUT", path, { + body: data, + contentType, + }); + } + } if (!resp.ok) { throw new Error(`WebDAV PUT failed for ${path}: ${resp.status} ${resp.statusText || ""}`); } @@ -508,13 +578,26 @@ export class WebDavClient { await this.put(path, JSON.stringify(data), "application/json"); } + /** + * Error for a failed HTTP verb response. Message keeps the legacy + * "WebDAV failed for : ..." format — callers match + * on it — while `status` is attached for status-based handling (getJSON). + */ + private createStatusError(method: string, path: string, resp: Response): WebDavError { + return new WebDavError( + resp.status === 404 ? "not-found" : "http", + `WebDAV ${method} failed for ${path}: ${resp.status} ${resp.statusText || ""}`, + { status: resp.status, method, url: this.buildUrl(path) }, + ); + } + /** Download data from a path (GET) — returns Uint8Array */ async get(path: string): Promise { const resp = await this.request("GET", path, { responseType: "arraybuffer", }); if (!resp.ok) { - throw new Error(`WebDAV GET failed for ${path}: ${resp.status} ${resp.statusText || ""}`); + throw this.createStatusError("GET", path, resp); } const buffer = await resp.arrayBuffer(); return new Uint8Array(buffer); @@ -544,7 +627,7 @@ export class WebDavClient { const elapsed = Date.now() - startTime; if (!resp.ok) { console.error(`[WebDAV] GET ${logPath} failed after ${elapsed}ms: ${resp.status}`); - throw new Error(`WebDAV GET failed for ${path}: ${resp.status} ${resp.statusText || ""}`); + throw this.createStatusError("GET", path, resp); } console.log(`[WebDAV] GET ${logPath} completed in ${elapsed}ms (status: ${resp.status})`); const buffer = await resp.arrayBuffer(); @@ -599,7 +682,7 @@ export class WebDavClient { responseType: "text", }); if (!resp.ok) { - throw new Error(`WebDAV GET failed for ${path}: ${resp.status} ${resp.statusText || ""}`); + throw this.createStatusError("GET", path, resp); } return resp.text(); } @@ -610,8 +693,12 @@ export class WebDavClient { const text = await this.getText(path); return JSON.parse(text) as T; } catch (e: unknown) { - const err = e as { message?: string }; - if (err.message?.includes("404") || err.message?.includes("409")) return null; + // 404/409 mean "not there yet" for callers like the device snapshot and + // the remote file manifest. Check the attached status instead of string + // matching so paths that happen to contain "404" can't fake a miss. + if (e instanceof WebDavError && (e.status === 404 || e.status === 409)) { + return null; + } throw e; } } @@ -621,7 +708,7 @@ export class WebDavClient { const resp = await this.request("DELETE", path); // 204 No Content or 404 Not Found — both OK for delete if (!resp.ok && resp.status !== 404) { - throw new Error(`WebDAV DELETE failed for ${path}: ${resp.status} ${resp.statusText || ""}`); + throw this.createStatusError("DELETE", path, resp); } } @@ -640,8 +727,10 @@ export class WebDavClient { // 201 Created (target newly created) and 204 No Content (target overwritten) are success. // 207 Multi-Status can also be returned for collection moves with partial errors — treat as failure. if (!resp.ok && resp.status !== 201 && resp.status !== 204) { - throw new Error( + throw new WebDavError( + resp.status === 404 ? "not-found" : "http", `WebDAV MOVE failed for ${fromPath} -> ${toPath}: ${resp.status} ${resp.statusText || ""}`, + { status: resp.status, method: "MOVE", url: destination }, ); } } @@ -669,7 +758,11 @@ export class WebDavClient { contentType: "application/xml", timeoutMs: options?.timeoutMs, }); - return resp.ok || resp.status === 207; + const exists = resp.ok || resp.status === 207; + if (exists) { + this.knownCollections.add(toCollectionPath(path)); + } + return exists; } catch { return false; }