From 946339a03553766cd41bed55abefe484e5caf3e4 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Mon, 30 Mar 2026 17:22:31 +0200 Subject: [PATCH] feat(sync): replace WebDAV header-based conflict detection with content hashing Replace HTTP-header-based conflict detection (If-Unmodified-Since, If-Match, Last-Modified, ETag) with application-level content hashing (MD5), reducing WebDAV server requirements to just GET, PUT, and MKCOL. This mirrors the proven pattern from the LocalFile sync provider and eliminates compatibility issues with WebDAV servers that don't properly support conditional headers, strip response headers via reverse proxies, or return inconsistent PROPFIND XML. Changes: - download() now computes MD5 hash of response body as rev - upload() uses GET-compare-PUT instead of conditional PUT headers - Remove testConditionalHeaders(), _cleanRev(), _getFileMetaViaHead() - Remove WebdavServerCapabilities, WebdavServerType, basicCompatibilityMode - Remove conditional header warning dialog from config page - Remove legacyRev from provider interface - Simplify webdav.const.ts (remove unused headers/methods/statuses) - Add E2E test for near-simultaneous two-client sync - Add unit test for rev stability across downloads --- .../webdav-single-client-rapid-sync.spec.ts | 28 +- e2e/tests/sync/webdav-sync-full.spec.ts | 274 ++++ .../core/persistence/storage-keys.const.ts | 2 - .../file-based-sync-adapter.service.ts | 13 +- .../file-based/webdav/nextcloud.ts | 7 - .../file-based/webdav/webdav-api.spec.ts | 1255 +++-------------- .../file-based/webdav/webdav-api.ts | 401 +----- .../file-based/webdav/webdav-base-provider.ts | 27 +- .../webdav/webdav-xml-parser.spec.ts | 8 +- .../file-based/webdav/webdav-xml-parser.ts | 2 +- .../file-based/webdav/webdav.const.ts | 9 - .../file-based/webdav/webdav.model.ts | 53 - .../sync-providers/provider.interface.ts | 1 - .../config-page/config-page.component.spec.ts | 3 - .../config-page/config-page.component.ts | 40 - src/app/t.const.ts | 4 - src/assets/i18n/en.json | 2 - 17 files changed, 526 insertions(+), 1603 deletions(-) diff --git a/e2e/tests/sync/webdav-single-client-rapid-sync.spec.ts b/e2e/tests/sync/webdav-single-client-rapid-sync.spec.ts index e66dbc3c33..c2e1a31164 100644 --- a/e2e/tests/sync/webdav-single-client-rapid-sync.spec.ts +++ b/e2e/tests/sync/webdav-single-client-rapid-sync.spec.ts @@ -13,18 +13,10 @@ import { /** * WebDAV Single Client Rapid Sync E2E Tests * - * These tests verify the bug fix for false 412 Precondition Failed errors - * during WebDAV sync with a single client. The bug was: - * - * 1. HTTP Last-Modified headers have second-level precision - * 2. Some WebDAV servers store modification times with millisecond precision - * 3. Client reads Last-Modified, then quickly uploads with If-Unmodified-Since - * 4. Server's internal timestamp was a few ms later than Last-Modified header - * 5. Server incorrectly returns 412 even though no other client modified the file - * - * The fix: Add a 1-second buffer to the If-Unmodified-Since header to account - * for this precision mismatch. Vector clocks still provide authoritative - * conflict detection, so this buffer doesn't compromise data integrity. + * These tests verify that rapid successive syncs from a single client + * complete without errors. Conflict detection uses content hashing (MD5) + * to compare the remote file before uploading, so timing precision + * is not a concern. * * Prerequisites: * - WebDAV server running at http://127.0.0.1:2345/ @@ -46,15 +38,7 @@ test.describe('@webdav Rapid Sync (Single Client)', () => { }; /** - * Scenario: Single client rapid syncs do not cause false 412 errors - * - * The 412 bug was most likely to occur when: - * - A single client syncs - * - Immediately creates a task - * - Syncs again within the same second - * - * The If-Unmodified-Since header used the Last-Modified timestamp exactly, - * but the server's internal timestamp could be a few milliseconds later. + * Scenario: Single client rapid syncs complete without errors * * Setup: * - Client A with WebDAV sync configured @@ -65,7 +49,7 @@ test.describe('@webdav Rapid Sync (Single Client)', () => { * 3. Repeat 5 times in rapid succession * * Verify: - * - All 5 syncs complete successfully (no 412 errors) + * - All 5 syncs complete successfully * - All 5 tasks are present */ test('Single client rapid syncs do not cause 412 errors', async ({ diff --git a/e2e/tests/sync/webdav-sync-full.spec.ts b/e2e/tests/sync/webdav-sync-full.spec.ts index 83b9f00a0b..01b3f96dee 100644 --- a/e2e/tests/sync/webdav-sync-full.spec.ts +++ b/e2e/tests/sync/webdav-sync-full.spec.ts @@ -1,3 +1,4 @@ +import { type Page } from '@playwright/test'; import { test, expect } from '../../fixtures/webdav.fixture'; import { SyncPage } from '../../pages/sync.page'; import { WorkViewPage } from '../../pages/work-view.page'; @@ -8,8 +9,24 @@ import { createSyncFolder, waitForSyncComplete, generateSyncFolderName, + closeContextsSafely, } from '../../utils/sync-helpers'; +/** + * Dismiss the Vite error overlay if present. + * In development mode, TypeScript errors can cause a full-page overlay + * that intercepts all pointer events. Pressing Escape dismisses it. + */ +const dismissViteOverlay = async (page: Page): Promise => { + const overlay = page.locator('vite-error-overlay'); + const isVisible = await overlay.isVisible().catch(() => false); + if (isVisible) { + console.log('[Test] Dismissing vite-error-overlay'); + await page.keyboard.press('Escape'); + await overlay.waitFor({ state: 'hidden', timeout: 3000 }).catch(() => {}); + } +}; + test.describe('@webdav WebDAV Sync Full Flow', () => { // Run sync tests serially to avoid WebDAV server contention test.describe.configure({ mode: 'serial' }); @@ -290,4 +307,261 @@ test.describe('@webdav WebDAV Sync Full Flow', () => { await contextA.close(); await contextB2.close(); }); + + /** + * Scenario: Near-simultaneous uploads from two clients preserve all data + * + * Verifies that the content-hash-based conflict detection (GET-compare-PUT) + * correctly handles concurrent uploads. When two clients sync at nearly the + * same time, the second client should detect the hash mismatch, retry with + * merged data, and no tasks should be lost. + * + * Setup: + * - Client A and Client B both configured with WebDAV sync to the same folder + * - Client A creates TaskA, syncs; Client B downloads TaskA + * + * Actions: + * 1. Client A creates TaskA2, Client B creates TaskB + * 2. Trigger sync on Client A, then Client B shortly after (without waiting + * for A to finish). The slight offset gives A time to start uploading so + * B's hash check can detect the change, triggering conflict retry. + * 3. Wait for both syncs to complete (handling any conflict dialogs) + * 4. Run convergence syncs until both clients have all data + * + * Verify: + * - Both clients have TaskA, TaskA2, and TaskB -- no data lost + */ + test('should preserve all data when two clients sync near-simultaneously', async ({ + browser, + baseURL, + request, + webdavServerUp, + }) => { + test.slow(); // Sync tests take longer + + const url = baseURL || 'http://localhost:4242'; + const concurrentFolder = generateSyncFolderName('e2e-concurrent'); + const concurrentConfig = { + ...WEBDAV_CONFIG_TEMPLATE, + syncFolderPath: `/${concurrentFolder}`, + }; + + await createSyncFolder(request, concurrentFolder); + + // --- Setup Client A --- + const { context: contextA, page: pageA } = await setupSyncClient(browser, url); + await dismissViteOverlay(pageA); + const syncPageA = new SyncPage(pageA); + const workViewPageA = new WorkViewPage(pageA); + + pageA.on('console', (msg) => { + const text = msg.text(); + if ( + text.includes('FileBasedSyncAdapter') || + text.includes('OperationLogSyncService') || + text.includes('SyncService') + ) { + console.log(`[Concurrent-A] ${msg.type()}: ${text}`); + } + }); + + // --- Setup Client B --- + const { context: contextB, page: pageB } = await setupSyncClient(browser, url); + await dismissViteOverlay(pageB); + const syncPageB = new SyncPage(pageB); + const workViewPageB = new WorkViewPage(pageB); + + pageB.on('console', (msg) => { + const text = msg.text(); + if ( + text.includes('FileBasedSyncAdapter') || + text.includes('OperationLogSyncService') || + text.includes('SyncService') || + text.includes('RemoteOpsProcessingService') || + text.includes('OperationApplierService') + ) { + console.log(`[Concurrent-B] ${msg.type()}: ${text}`); + } + }); + + try { + // --- Initialize both clients --- + await workViewPageA.waitForTaskList(); + await syncPageA.setupWebdavSync(concurrentConfig); + await expect(syncPageA.syncBtn).toBeVisible(); + console.log('[Concurrent] Client A configured'); + + await workViewPageB.waitForTaskList(); + await syncPageB.setupWebdavSync(concurrentConfig); + await expect(syncPageB.syncBtn).toBeVisible(); + console.log('[Concurrent] Client B configured'); + + // --- Step 1: Client A creates TaskA and syncs --- + await workViewPageA.addTask('TaskA'); + await expect(pageA.locator('task')).toHaveCount(1); + await waitForStatePersistence(pageA); + + await syncPageA.triggerSync(); + await waitForSyncComplete(pageA, syncPageA); + console.log('[Concurrent] Client A synced TaskA'); + + // --- Step 2: Client B downloads TaskA --- + await syncPageB.triggerSync(); + await waitForSyncComplete(pageB, syncPageB); + await expect(pageB.locator('task')).toHaveCount(1, { timeout: 10000 }); + await expect(pageB.locator('task').first()).toContainText('TaskA'); + console.log('[Concurrent] Client B downloaded TaskA'); + + // --- Step 3: Both clients create tasks independently --- + await workViewPageA.addTask('TaskA2'); + await expect(pageA.locator('task')).toHaveCount(2); + await waitForStatePersistence(pageA); + console.log('[Concurrent] Client A created TaskA2'); + + await workViewPageB.addTask('TaskB'); + await expect(pageB.locator('task')).toHaveCount(2); + await waitForStatePersistence(pageB); + console.log('[Concurrent] Client B created TaskB'); + + // --- Step 4: Near-simultaneous sync --- + // Trigger Client A first, then Client B after a short delay. We do NOT + // wait for A to complete, but the slight offset ensures A's upload is + // in-flight (or complete) when B attempts its own upload. This exercises + // the content-hash conflict detection: B's pre-upload GET returns a + // different hash than expected, triggering a retry with merged data. + console.log('[Concurrent] Triggering near-simultaneous syncs...'); + + const syncPromiseA = (async () => { + await syncPageA.triggerSync(); + return waitForSyncComplete(pageA, syncPageA); + })(); + + // Brief delay so A's upload has a head start, then fire B + await pageB.waitForTimeout(500); + + const syncPromiseB = (async () => { + await syncPageB.triggerSync(); + return waitForSyncComplete(pageB, syncPageB); + })(); + + // Wait for both syncs to complete + const [resultA, resultB] = await Promise.all([syncPromiseA, syncPromiseB]); + console.log(`[Concurrent] Simultaneous sync results: A=${resultA}, B=${resultB}`); + + // Handle conflict dialogs if they appear + if (resultA === 'conflict') { + console.log('[Concurrent] Client A got conflict, using remote'); + await pageA + .locator('dialog-sync-conflict button', { hasText: /Remote/i }) + .click(); + const confirmDialog = pageA.locator('dialog-confirm'); + try { + await confirmDialog.waitFor({ state: 'visible', timeout: 3000 }); + await confirmDialog.locator('button[color="warn"]').click(); + } catch { + // Confirmation might not appear + } + await waitForSyncComplete(pageA, syncPageA); + } + + if (resultB === 'conflict') { + console.log('[Concurrent] Client B got conflict, using remote'); + await pageB + .locator('dialog-sync-conflict button', { hasText: /Remote/i }) + .click(); + const confirmDialog = pageB.locator('dialog-confirm'); + try { + await confirmDialog.waitFor({ state: 'visible', timeout: 3000 }); + await confirmDialog.locator('button[color="warn"]').click(); + } catch { + // Confirmation might not appear + } + await waitForSyncComplete(pageB, syncPageB); + } + + // --- Step 5: Convergence syncs --- + // Sync both clients sequentially to ensure they converge on the same state. + // Multiple rounds may be needed: one client's upload from step 4 + // might not yet include the other client's data. + console.log('[Concurrent] Starting convergence syncs...'); + for (let round = 1; round <= 3; round++) { + await syncPageA.triggerSync(); + const convergenceResultA = await waitForSyncComplete(pageA, syncPageA); + if (convergenceResultA === 'conflict') { + await pageA + .locator('dialog-sync-conflict button', { hasText: /Remote/i }) + .click(); + const cd = pageA.locator('dialog-confirm'); + try { + await cd.waitFor({ state: 'visible', timeout: 3000 }); + await cd.locator('button[color="warn"]').click(); + } catch { + // Confirmation might not appear + } + await waitForSyncComplete(pageA, syncPageA); + } + + await syncPageB.triggerSync(); + const convergenceResultB = await waitForSyncComplete(pageB, syncPageB); + if (convergenceResultB === 'conflict') { + await pageB + .locator('dialog-sync-conflict button', { hasText: /Remote/i }) + .click(); + const cd = pageB.locator('dialog-confirm'); + try { + await cd.waitFor({ state: 'visible', timeout: 3000 }); + await cd.locator('button[color="warn"]').click(); + } catch { + // Confirmation might not appear + } + await waitForSyncComplete(pageB, syncPageB); + } + + // Check if both clients have all 3 tasks + const countA = await pageA.locator('task').count(); + const countB = await pageB.locator('task').count(); + console.log( + `[Concurrent] Convergence round ${round}: A has ${countA} tasks, B has ${countB} tasks`, + ); + + if (countA >= 3 && countB >= 3) { + console.log(`[Concurrent] Both clients converged after round ${round}`); + break; + } + } + + // --- Step 6: Verify no data was lost --- + // Both clients should have all 3 tasks: TaskA, TaskA2, TaskB + const titlesA = await pageA.locator('.task-title').allInnerTexts(); + const titlesB = await pageB.locator('.task-title').allInnerTexts(); + console.log(`[Concurrent] Final tasks on A: ${JSON.stringify(titlesA)}`); + console.log(`[Concurrent] Final tasks on B: ${JSON.stringify(titlesB)}`); + + // Verify Client A has all tasks + await expect(pageA.locator('task', { hasText: 'TaskA2' })).toBeVisible({ + timeout: 5000, + }); + await expect(pageA.locator('task', { hasText: 'TaskB' })).toBeVisible({ + timeout: 5000, + }); + + // Verify Client B has all tasks + await expect(pageB.locator('task', { hasText: 'TaskA2' })).toBeVisible({ + timeout: 5000, + }); + await expect(pageB.locator('task', { hasText: 'TaskB' })).toBeVisible({ + timeout: 5000, + }); + + // Both should have exactly 3 tasks (TaskA, TaskA2, TaskB) + await expect(pageA.locator('task')).toHaveCount(3, { timeout: 5000 }); + await expect(pageB.locator('task')).toHaveCount(3, { timeout: 5000 }); + + console.log( + '[Concurrent] All tasks preserved on both clients after near-simultaneous sync', + ); + } finally { + await closeContextsSafely(contextA, contextB); + } + }); }); diff --git a/src/app/core/persistence/storage-keys.const.ts b/src/app/core/persistence/storage-keys.const.ts index 617c20386f..4ee7e43ba4 100644 --- a/src/app/core/persistence/storage-keys.const.ts +++ b/src/app/core/persistence/storage-keys.const.ts @@ -23,8 +23,6 @@ export enum LS { LAST_NOTE_BANNER_DAY = 'SUP_LAST_NOTE_BANNER_DAY', - WEBDAV_CONDITIONAL_HEADER_WARNING_DISMISSED = 'SUP_WEBDAV_CONDITIONAL_HEADER_WARNING_DISMISSED', - CAL_EVENTS_CACHE = 'SUP_CAL_EVENTS_CACHE', CALENDER_EVENTS_SKIPPED_TODAY = 'SUP_CALENDER_EVENTS_SKIPPED_TODAY', CALENDER_EVENTS_LAST_SKIP_DAY = 'SUP_CALENDER_EVENTS_LAST_SKIP_DAY', diff --git a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts index c0dfcb0ddc..4583a2be65 100644 --- a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts +++ b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts @@ -506,11 +506,12 @@ export class FileBasedSyncAdapterService { 'FileBasedSyncAdapter._uploadWithRetry(retry)', ); - // Compare against previous attempt's rev (not original revToMatch) - const isServerRevInconsistent = freshRev === previousRev; - if (isServerRevInconsistent) { + // Same content hash means the file has not changed since our last download. + // Safe to overwrite unconditionally. + const isServerUnchanged = freshRev === previousRev; + if (isServerUnchanged) { OpLog.warn( - 'FileBasedSyncAdapter: Rev unchanged after re-download, server has inconsistent timestamp handling. Force-uploading.', + 'FileBasedSyncAdapter: Rev unchanged after re-download, force-uploading.', ); } @@ -519,7 +520,7 @@ export class FileBasedSyncAdapterService { FILE_BASED_SYNC_CONSTANTS.SYNC_FILE, freshUploadData, freshRev, - isServerRevInconsistent, + isServerUnchanged, ); OpLog.normal(`FileBasedSyncAdapter: Retry ${attempt} upload successful`); return { finalSyncVersion: freshNewData.syncVersion }; @@ -527,7 +528,7 @@ export class FileBasedSyncAdapterService { if (!(retryErr instanceof UploadRevToMatchMismatchAPIError)) { throw retryErr; } - // If force-upload was used (isServerRevInconsistent), this shouldn't happen + // If force-upload was used (isServerUnchanged), this shouldn't happen // but if it does, let the loop continue previousRev = freshRev; } diff --git a/src/app/op-log/sync-providers/file-based/webdav/nextcloud.ts b/src/app/op-log/sync-providers/file-based/webdav/nextcloud.ts index 21160d43b4..744af81267 100644 --- a/src/app/op-log/sync-providers/file-based/webdav/nextcloud.ts +++ b/src/app/op-log/sync-providers/file-based/webdav/nextcloud.ts @@ -67,16 +67,9 @@ export class NextcloudProvider extends WebdavBaseProvider 'Nextcloud password is not configured. Please check your sync settings.', ); } - // Return a WebdavPrivateCfg with the auto-constructed baseUrl and - // pre-set Nextcloud capabilities to skip auto-detection on first sync return { ...cfg, baseUrl: this._buildNextcloudBaseUrl(cfg), - serverCapabilities: { - supportsETags: true, - supportsIfHeader: true, - supportsLastModified: true, - }, }; } diff --git a/src/app/op-log/sync-providers/file-based/webdav/webdav-api.spec.ts b/src/app/op-log/sync-providers/file-based/webdav/webdav-api.spec.ts index cee99fb666..6dcf6d8083 100644 --- a/src/app/op-log/sync-providers/file-based/webdav/webdav-api.spec.ts +++ b/src/app/op-log/sync-providers/file-based/webdav/webdav-api.spec.ts @@ -6,10 +6,10 @@ import { WebdavXmlParser } from './webdav-xml-parser'; import { HttpNotOkAPIError, InvalidDataSPError, - NoRevAPIError, RemoteFileChangedUnexpectedly, RemoteFileNotFoundAPIError, } from '../../../core/errors/sync-errors'; +import { md5HashSync } from '../../../../util/md5-hash'; describe('WebdavApi', () => { let api: WebdavApi; @@ -61,7 +61,7 @@ describe('WebdavApi', () => { }; mockXmlParser.parseMultiplePropsFromXml.and.returnValue([mockFileMeta]); - const result = await api.getFileMeta('/test.txt', null); + const result = await api.getFileMeta('/test.txt'); expect(mockHttpAdapter.request).toHaveBeenCalledWith({ url: 'http://example.com/webdav/test.txt', @@ -89,7 +89,7 @@ describe('WebdavApi', () => { }; mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - await expectAsync(api.getFileMeta('/test.txt', null)).toBeRejectedWith( + await expectAsync(api.getFileMeta('/test.txt')).toBeRejectedWith( jasmine.any(RemoteFileNotFoundAPIError), ); }); @@ -103,7 +103,7 @@ describe('WebdavApi', () => { mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); mockXmlParser.parseMultiplePropsFromXml.and.returnValue([]); - await expectAsync(api.getFileMeta('/test.txt', null)).toBeRejectedWith( + await expectAsync(api.getFileMeta('/test.txt')).toBeRejectedWith( jasmine.any(RemoteFileNotFoundAPIError), ); }); @@ -128,7 +128,7 @@ describe('WebdavApi', () => { }, ]); - await api.getFileMeta('/folder/file with spaces.txt', null); + await api.getFileMeta('/folder/file with spaces.txt'); expect(mockHttpAdapter.request).toHaveBeenCalledWith( jasmine.objectContaining({ @@ -136,51 +136,21 @@ describe('WebdavApi', () => { }), ); }); - - it('should fall back to HEAD metadata when PROPFIND fails and fallback is enabled', async () => { - const mockResponse = { - status: 500, - headers: {}, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - const headMeta = { - filename: 'test.txt', - basename: 'test.txt', - lastmod: 'Wed, 15 Jan 2025 10:00:00 GMT', - size: 42, - type: 'file', - etag: 'Wed, 15 Jan 2025 10:00:00 GMT', - data: {}, - path: '/test.txt', - }; - const headSpy = spyOn(api, '_getFileMetaViaHead').and.returnValue( - Promise.resolve(headMeta), - ); - - const result = await api.getFileMeta('/test.txt', null, true); - - expect(headSpy).toHaveBeenCalledWith('http://example.com/webdav/test.txt'); - expect(result).toEqual(headMeta); - }); }); describe('download', () => { - it('should download file successfully', async () => { + it('should return MD5 hash of content as rev', async () => { + const content = 'file content'; + const expectedHash = md5HashSync(content); const mockResponse = { status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 10:00:00 GMT', - }, - data: 'file content', + headers: {}, + data: content, }; mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); mockXmlParser.validateResponseContent.and.stub(); - const result = await api.download({ - path: '/test.txt', - }); + const result = await api.download({ path: '/test.txt' }); expect(mockHttpAdapter.request).toHaveBeenCalledWith( jasmine.objectContaining({ @@ -190,601 +160,275 @@ describe('WebdavApi', () => { ); expect(mockXmlParser.validateResponseContent).toHaveBeenCalledWith( - 'file content', + content, '/test.txt', 'download', 'file content', ); - expect(result).toEqual( - jasmine.objectContaining({ - rev: 'Wed, 15 Jan 2025 10:00:00 GMT', - dataStr: 'file content', - lastModified: 'Wed, 15 Jan 2025 10:00:00 GMT', - }), - ); - expect(result.legacyRev).toBeUndefined(); - }); - - it('should return legacyRev when ETag is present', async () => { - const mockResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 10:00:00 GMT', - etag: '"abc123"', - }, - data: 'file content', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - mockXmlParser.validateResponseContent.and.stub(); - - const result = await api.download({ - path: '/test.txt', + expect(result).toEqual({ + rev: expectedHash, + dataStr: content, }); - - expect(result).toEqual( - jasmine.objectContaining({ - rev: 'Wed, 15 Jan 2025 10:00:00 GMT', - legacyRev: 'abc123', // Cleaned ETag - dataStr: 'file content', - lastModified: 'Wed, 15 Jan 2025 10:00:00 GMT', - }), - ); }); - it('should use last-modified as rev when no etag', async () => { - const mockResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 10:00:00 GMT', - }, - data: 'content', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); + it('should produce the same hash for the same content', async () => { + const content = 'identical content'; mockXmlParser.validateResponseContent.and.stub(); - const result = await api.download({ - path: '/test.txt', - }); + mockHttpAdapter.request.and.returnValue( + Promise.resolve({ status: 200, headers: {}, data: content }), + ); + const result1 = await api.download({ path: '/file1.txt' }); - expect(result.rev).toBe('Wed, 15 Jan 2025 10:00:00 GMT'); + mockHttpAdapter.request.and.returnValue( + Promise.resolve({ status: 200, headers: {}, data: content }), + ); + const result2 = await api.download({ path: '/file2.txt' }); + + expect(result1.rev).toBe(result2.rev); }); - it('should fetch metadata when Last-Modified header is missing but ETag is present', async () => { - const mockResponse = { - status: 200, - headers: { - etag: '"abc123"', - }, - data: 'file content', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); + it('should produce different hashes for different content', async () => { mockXmlParser.validateResponseContent.and.stub(); - spyOn(api, 'getFileMeta').and.returnValue( - Promise.resolve({ - filename: 'test.txt', - basename: 'test.txt', - lastmod: 'Wed, 15 Jan 2025 10:00:00 GMT', - size: 100, - type: 'file', - etag: 'Wed, 15 Jan 2025 10:00:00 GMT', - data: { - etag: '"meta-etag-should-not-override"', - }, - path: '/test.txt', - }), + mockHttpAdapter.request.and.returnValue( + Promise.resolve({ status: 200, headers: {}, data: 'content A' }), ); + const result1 = await api.download({ path: '/file.txt' }); - const result = await api.download({ - path: '/test.txt', - }); + mockHttpAdapter.request.and.returnValue( + Promise.resolve({ status: 200, headers: {}, data: 'content B' }), + ); + const result2 = await api.download({ path: '/file.txt' }); - expect(api.getFileMeta).toHaveBeenCalledWith('/test.txt', null, true); - expect(result.rev).toBe('Wed, 15 Jan 2025 10:00:00 GMT'); - expect(result.legacyRev).toBe('abc123'); - expect(result.lastModified).toBe('Wed, 15 Jan 2025 10:00:00 GMT'); + expect(result1.rev).not.toBe(result2.rev); }); - it('should set legacyRev from metadata when GET response omits both headers', async () => { - const mockResponse = { - status: 200, - headers: {}, - data: 'file content', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); + it('should produce identical rev when downloading the same unchanged file twice', async () => { + const content = '{"tasks":[],"projects":[]}'; mockXmlParser.validateResponseContent.and.stub(); - spyOn(api, 'getFileMeta').and.returnValue( - Promise.resolve({ - filename: 'test.txt', - basename: 'test.txt', - lastmod: 'Wed, 15 Jan 2025 10:00:00 GMT', - size: 100, - type: 'file', - etag: 'Wed, 15 Jan 2025 10:00:00 GMT', - data: { - etag: '"propfind-etag-456"', - }, - path: '/test.txt', - }), + mockHttpAdapter.request.and.returnValue( + Promise.resolve({ status: 200, headers: {}, data: content }), ); + const result1 = await api.download({ path: '/sync/sync-data.json' }); - const result = await api.download({ - path: '/test.txt', - }); - - expect(result.rev).toBe('Wed, 15 Jan 2025 10:00:00 GMT'); - expect(result.legacyRev).toBe('propfind-etag-456'); - expect(result.lastModified).toBe('Wed, 15 Jan 2025 10:00:00 GMT'); - }); - - it('should throw NoRevAPIError if metadata fallback cannot provide a revision', async () => { - const mockResponse = { - status: 200, - headers: {}, - data: 'file content', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - mockXmlParser.validateResponseContent.and.stub(); - - spyOn(api, 'getFileMeta').and.returnValue( - Promise.resolve({ - filename: 'test.txt', - basename: 'test.txt', - lastmod: '', - size: 0, - type: 'file', - etag: '', - data: {}, - path: '/test.txt', - }), + mockHttpAdapter.request.and.returnValue( + Promise.resolve({ status: 200, headers: {}, data: content }), ); + const result2 = await api.download({ path: '/sync/sync-data.json' }); - await expectAsync( - api.download({ - path: '/test.txt', - }), - ).toBeRejectedWith(jasmine.any(NoRevAPIError)); - }); - - it('should use ETag as fallback revision when metadata fallback fails', async () => { - const mockResponse = { - status: 200, - headers: { - etag: '"etag-fallback-123"', - }, - data: 'file content', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - mockXmlParser.validateResponseContent.and.stub(); - - // Simulate metadata fallback failing - spyOn(api, 'getFileMeta').and.returnValue( - Promise.reject(new Error('PROPFIND failed')), - ); - - const result = await api.download({ - path: '/test.txt', - }); - - // Should use ETag as fallback revision - expect(result.rev).toBe('etag-fallback-123'); - expect(result.legacyRev).toBe('etag-fallback-123'); - expect(result.dataStr).toBe('file content'); + expect(result1.rev).toBe(result2.rev); + expect(typeof result1.rev).toBe('string'); + expect(result1.rev.length).toBeGreaterThan(0); }); it('should throw InvalidDataSPError when response body is empty', async () => { const mockResponse = { status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 10:00:00 GMT', - }, + headers: {}, data: '', }; mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - await expectAsync( - api.download({ - path: '/test.txt', - }), - ).toBeRejectedWith(jasmine.any(InvalidDataSPError)); + await expectAsync(api.download({ path: '/test.txt' })).toBeRejectedWith( + jasmine.any(InvalidDataSPError), + ); // validateResponseContent should NOT be called when empty body is detected expect(mockXmlParser.validateResponseContent).not.toHaveBeenCalled(); }); - // Test removed: If-None-Match header functionality has been removed - // Test removed: If-Modified-Since header functionality has been removed - // Test removed: If-Modified-Since header functionality has been removed - // Test removed: 304 Not Modified handling has been removed - // Test removed: 304 Not Modified handling has been removed - // Test removed: localRev parameter has been removed from download method - // Test removed: localRev parameter has been removed from download method + it('should call validateResponseContent to detect HTML error pages', async () => { + const htmlContent = 'Error'; + const mockResponse = { + status: 200, + headers: {}, + data: htmlContent, + }; + mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); + mockXmlParser.validateResponseContent.and.throwError('HTML error page detected'); + + await expectAsync(api.download({ path: '/test.txt' })).toBeRejected(); + + expect(mockXmlParser.validateResponseContent).toHaveBeenCalledWith( + htmlContent, + '/test.txt', + 'download', + 'file content', + ); + }); }); describe('upload', () => { - it('should upload file successfully', async () => { + it('should upload without expectedRev using plain PUT and return hash of uploaded data', async () => { + const uploadData = 'new content'; + const expectedHash = md5HashSync(uploadData); const mockResponse = { status: 201, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT', - }, + headers: {}, data: '', }; mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); const result = await api.upload({ path: '/test.txt', - data: 'new content', + data: uploadData, expectedRev: null, }); + // Should only make one request (the PUT) + expect(mockHttpAdapter.request).toHaveBeenCalledTimes(1); expect(mockHttpAdapter.request).toHaveBeenCalledWith( jasmine.objectContaining({ url: 'http://example.com/webdav/test.txt', method: 'PUT', - body: 'new content', + body: uploadData, headers: jasmine.objectContaining({ 'Content-Type': 'application/octet-stream', }), }), ); - expect(result).toEqual( - jasmine.objectContaining({ - rev: 'Wed, 15 Jan 2025 11:00:00 GMT', - lastModified: 'Wed, 15 Jan 2025 11:00:00 GMT', - }), - ); - expect(result.legacyRev).toBeUndefined(); + expect(result).toEqual({ rev: expectedHash }); }); - it('should return legacyRev when ETag is present in upload response', async () => { - const mockResponse = { - status: 201, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT', - etag: '"newrev123"', - }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); + it('should do GET-compare-PUT when expectedRev is provided', async () => { + const existingContent = 'existing content'; + const existingHash = md5HashSync(existingContent); + const uploadData = 'new content'; + const expectedUploadHash = md5HashSync(uploadData); + + let callCount = 0; + mockHttpAdapter.request.and.callFake(() => { + callCount++; + if (callCount === 1) { + // First call: GET to check current content + return Promise.resolve({ + status: 200, + headers: {}, + data: existingContent, + }); + } + // Second call: PUT to upload + return Promise.resolve({ status: 201, headers: {}, data: '' }); + }); const result = await api.upload({ path: '/test.txt', - data: 'new content', - expectedRev: null, + data: uploadData, + expectedRev: existingHash, }); - expect(result).toEqual( - jasmine.objectContaining({ - rev: 'Wed, 15 Jan 2025 11:00:00 GMT', - legacyRev: 'newrev123', // Cleaned ETag - lastModified: 'Wed, 15 Jan 2025 11:00:00 GMT', + expect(mockHttpAdapter.request).toHaveBeenCalledTimes(2); + // First call should be GET + expect(mockHttpAdapter.request.calls.argsFor(0)[0]).toEqual( + jasmine.objectContaining({ method: 'GET' }), + ); + // Second call should be PUT + expect(mockHttpAdapter.request.calls.argsFor(1)[0]).toEqual( + jasmine.objectContaining({ method: 'PUT', body: uploadData }), + ); + + expect(result).toEqual({ rev: expectedUploadHash }); + }); + + it('should throw RemoteFileChangedUnexpectedly when content hash mismatches expectedRev', async () => { + const existingContent = 'modified content on remote'; + const staleHash = md5HashSync('original content'); + + mockHttpAdapter.request.and.returnValue( + Promise.resolve({ + status: 200, + headers: {}, + data: existingContent, }), ); + + await expectAsync( + api.upload({ + path: '/test.txt', + data: 'new content', + expectedRev: staleHash, + }), + ).toBeRejectedWith(jasmine.any(RemoteFileChangedUnexpectedly)); + + // Should have only made the GET request, not the PUT + expect(mockHttpAdapter.request).toHaveBeenCalledTimes(1); + expect(mockHttpAdapter.request.calls.argsFor(0)[0]).toEqual( + jasmine.objectContaining({ method: 'GET' }), + ); }); - it('should NOT send If-Unmodified-Since when expectedRev is null', async () => { - const mockResponse = { - status: 200, - headers: { 'last-modified': 'Thu, 01 Jan 2025 00:00:00 GMT' }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); + it('should skip GET check when isForceOverwrite is true', async () => { + const uploadData = 'force overwrite content'; + const expectedHash = md5HashSync(uploadData); - await api.upload({ path: '/test.json', data: 'test', expectedRev: null }); + mockHttpAdapter.request.and.returnValue( + Promise.resolve({ status: 201, headers: {}, data: '' }), + ); - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - expect(requestArgs).toBeDefined(); - expect(requestArgs.headers['If-Unmodified-Since']).toBeUndefined(); - }); - - it('should NOT send If-Unmodified-Since when expectedRev is undefined', async () => { - const mockResponse = { - status: 200, - headers: { 'last-modified': 'Thu, 01 Jan 2025 00:00:00 GMT' }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - await api.upload({ path: '/test.json', data: 'test' }); - - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - expect(requestArgs).toBeDefined(); - expect(requestArgs.headers['If-Unmodified-Since']).toBeUndefined(); - }); - - it('should NOT send If-Unmodified-Since when isForceOverwrite is true', async () => { - const mockResponse = { - status: 200, - headers: { 'last-modified': 'Thu, 01 Jan 2025 00:00:00 GMT' }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - await api.upload({ - path: '/test.json', - data: 'test', - expectedRev: 'Thu, 01 Jan 2024 00:00:00 GMT', + const result = await api.upload({ + path: '/test.txt', + data: uploadData, + expectedRev: 'some-old-rev', isForceOverwrite: true, }); - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - expect(requestArgs).toBeDefined(); - expect(requestArgs.headers['If-Unmodified-Since']).toBeUndefined(); - }); - - it('should handle conditional upload with date string', async () => { - const mockResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT', - }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: 'Wed, 15 Jan 2025 10:00:00 GMT', - }); - - expect(mockHttpAdapter.request).toHaveBeenCalledWith( - jasmine.objectContaining({ - headers: jasmine.objectContaining({ - 'If-Unmodified-Since': jasmine.any(String), - }), - }), + // Should only make one request (the PUT), no GET + expect(mockHttpAdapter.request).toHaveBeenCalledTimes(1); + expect(mockHttpAdapter.request.calls.argsFor(0)[0]).toEqual( + jasmine.objectContaining({ method: 'PUT' }), ); + expect(result).toEqual({ rev: expectedHash }); }); - it('should handle conditional upload with ISO date string', async () => { - const mockResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT', - }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); + it('should proceed with PUT when GET returns 404 (new file)', async () => { + const uploadData = 'new file content'; + const expectedHash = md5HashSync(uploadData); - const isoDate = '2022-01-15T12:00:00.000Z'; // ISO date string - - await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: isoDate, + let callCount = 0; + mockHttpAdapter.request.and.callFake(() => { + callCount++; + if (callCount === 1) { + // GET returns 404 via RemoteFileNotFoundAPIError + return Promise.reject(new RemoteFileNotFoundAPIError('/test.txt')); + } + // PUT succeeds + return Promise.resolve({ status: 201, headers: {}, data: '' }); }); - expect(mockHttpAdapter.request).toHaveBeenCalledWith( - jasmine.objectContaining({ - headers: jasmine.objectContaining({ - 'If-Unmodified-Since': jasmine.any(String), - }), - }), - ); - }); - - it('should add 1-second buffer to If-Unmodified-Since header to handle sub-second precision', async () => { - const mockResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT', - }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - // Use a known date: Wed, 15 Jan 2025 10:00:00 GMT - const expectedRev = 'Wed, 15 Jan 2025 10:00:00 GMT'; - // The buffered date should be: Wed, 15 Jan 2025 10:00:01 GMT - - await api.upload({ + const result = await api.upload({ path: '/test.txt', - data: 'new content', - expectedRev, + data: uploadData, + expectedRev: 'some-rev', }); - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - // The header should have 1 second added (10:00:01 instead of 10:00:00) - expect(requestArgs.headers['If-Unmodified-Since']).toBe( - 'Wed, 15 Jan 2025 10:00:01 GMT', - ); + expect(mockHttpAdapter.request).toHaveBeenCalledTimes(2); + expect(result).toEqual({ rev: expectedHash }); }); - it('should NOT add buffer to If-Match header when expectedRev is an ETag', async () => { - const mockResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT', - }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - // ETag that is not a valid date - no buffer should be applied - const etag = 'abc123-etag-value'; - - await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: etag, - }); - - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - // ETag should be used as-is (with quotes), no time buffer - expect(requestArgs.headers['If-Match']).toBe('"abc123-etag-value"'); - expect(requestArgs.headers['If-Unmodified-Since']).toBeUndefined(); - }); - - it('should add 1-second buffer to ISO date format', async () => { - const mockResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT', - }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - // ISO date: 2025-01-15T10:00:00.000Z - const isoDate = '2025-01-15T10:00:00.000Z'; - - await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: isoDate, - }); - - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - // The header should have 1 second added (10:00:01 instead of 10:00:00) - expect(requestArgs.headers['If-Unmodified-Since']).toBe( - 'Wed, 15 Jan 2025 10:00:01 GMT', - ); - }); - - it('should handle date boundary rollover with 1-second buffer', async () => { - const mockResponse = { - status: 200, - headers: { - 'last-modified': 'Thu, 16 Jan 2025 00:00:00 GMT', - }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - // Date at 23:59:59 - adding 1 second should roll over to next day - const expectedRev = 'Wed, 15 Jan 2025 23:59:59 GMT'; - - await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev, - }); - - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - // Should roll over to 00:00:00 on the next day - expect(requestArgs.headers['If-Unmodified-Since']).toBe( - 'Thu, 16 Jan 2025 00:00:00 GMT', - ); - }); - - it('should use If-Match header when expectedRev is an ETag (not a valid date)', async () => { - const mockResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT', - }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - // ETag that is not a valid date - const etag = 'abc123-etag-value'; - - await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: etag, - }); - - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - expect(requestArgs.headers['If-Match']).toBe('"abc123-etag-value"'); - expect(requestArgs.headers['If-Unmodified-Since']).toBeUndefined(); - }); - - it('should properly quote ETag in If-Match header per RFC 7232', async () => { - const mockResponse = { - status: 200, - headers: { 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT' }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - // Unquoted ETag - await api.upload({ - path: '/test.txt', - data: 'content', - expectedRev: 'simple-etag', - }); - - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - expect(requestArgs.headers['If-Match']).toBe('"simple-etag"'); - }); - - it('should not double-quote already quoted ETags in If-Match header', async () => { - const mockResponse = { - status: 200, - headers: { 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT' }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - // Already quoted ETag - await api.upload({ - path: '/test.txt', - data: 'content', - expectedRev: '"already-quoted-etag"', - }); - - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - expect(requestArgs.headers['If-Match']).toBe('"already-quoted-etag"'); - }); - - it('should handle 412 Precondition Failed with ETag-based If-Match', async () => { - const errorResponse = new Response(null, { status: 412 }); - const error = new HttpNotOkAPIError(errorResponse); - mockHttpAdapter.request.and.returnValue(Promise.reject(error)); - - // Using ETag (not a date) should trigger If-Match - await expectAsync( - api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: 'etag-that-changed', - }), - ).toBeRejectedWith(jasmine.any(RemoteFileChangedUnexpectedly)); - }); - - it('should handle 412 Precondition Failed', async () => { - const errorResponse = new Response(null, { status: 412 }); - const error = new HttpNotOkAPIError(errorResponse); - mockHttpAdapter.request.and.returnValue(Promise.reject(error)); - - await expectAsync( - api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: 'oldrev', - }), - ).toBeRejectedWith(jasmine.any(RemoteFileChangedUnexpectedly)); - }); - - it('should handle 409 Conflict by creating parent directory', async () => { + it('should handle 409 Conflict by creating parent directory and retrying', async () => { + const uploadData = 'new content'; + const expectedHash = md5HashSync(uploadData); const errorResponse = new Response(null, { status: 409 }); const error = new HttpNotOkAPIError(errorResponse); - // First call fails with 409 - // Second call to create directory succeeds - // Third call to upload succeeds + // First call: PUT fails with 409 + // Second call: MKCOL to create directory succeeds + // Third call: PUT retry succeeds const mockResponses = [ Promise.reject(error), Promise.resolve({ status: 201, headers: {}, data: '' }), - Promise.resolve({ - status: 201, - headers: { 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT' }, - data: '', - }), + Promise.resolve({ status: 201, headers: {}, data: '' }), ]; let callCount = 0; mockHttpAdapter.request.and.callFake(() => mockResponses[callCount++]); const result = await api.upload({ path: '/folder/test.txt', - data: 'new content', + data: uploadData, expectedRev: null, }); @@ -796,157 +440,7 @@ describe('WebdavApi', () => { method: 'MKCOL', }), ); - expect(result.rev).toBe('Wed, 15 Jan 2025 11:00:00 GMT'); - }); - - it('should fetch metadata when no rev in response headers', async () => { - const mockUploadResponse = { - status: 201, - headers: {}, // No etag or last-modified - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockUploadResponse)); - - // Mock getFileMeta to be called after upload - spyOn(api, 'getFileMeta').and.returnValue( - Promise.resolve({ - filename: 'test.txt', - basename: 'test.txt', - lastmod: 'Wed, 15 Jan 2025 12:00:00 GMT', - size: 100, - type: 'file', - etag: 'Wed, 15 Jan 2025 12:00:00 GMT', // Using lastmod as etag - data: {}, - path: '/test.txt', - }), - ); - - const result = await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: null, - }); - - expect(api.getFileMeta).toHaveBeenCalledWith('/test.txt', null, true); - expect(result).toEqual( - jasmine.objectContaining({ - rev: 'Wed, 15 Jan 2025 12:00:00 GMT', - lastModified: 'Wed, 15 Jan 2025 12:00:00 GMT', - }), - ); - expect(result.legacyRev).toBeUndefined(); - }); - - it('should return legacyRev from HEAD request when PUT returns no headers', async () => { - // First request (PUT) returns no headers - const putResponse = { - status: 201, - headers: {}, - data: '', - }; - - // HEAD request returns both Last-Modified and ETag - const headResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 13:00:00 GMT', - etag: '"head-etag-123"', - }, - data: '', - }; - - mockHttpAdapter.request.and.callFake((params) => { - if (params.method === 'PUT') { - return Promise.resolve(putResponse); - } else if (params.method === 'HEAD') { - return Promise.resolve(headResponse); - } else { - return Promise.reject(new Error('Unexpected method')); - } - }); - - const result = await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: null, - }); - - expect(result).toEqual({ - rev: 'Wed, 15 Jan 2025 13:00:00 GMT', - legacyRev: 'head-etag-123', - lastModified: 'Wed, 15 Jan 2025 13:00:00 GMT', - }); - }); - - it('should extract legacyRev from PROPFIND meta when HEAD fails', async () => { - // PUT returns no headers - const putResponse = { - status: 201, - headers: {}, - data: '', - }; - - mockHttpAdapter.request.and.callFake((params) => { - if (params.method === 'PUT') { - return Promise.resolve(putResponse); - } else if (params.method === 'HEAD') { - return Promise.reject(new Error('HEAD failed')); - } else { - return Promise.reject(new Error('Unexpected method')); - } - }); - - // Mock getFileMeta to return data with etag - spyOn(api, 'getFileMeta').and.returnValue( - Promise.resolve({ - filename: 'test.txt', - basename: 'test.txt', - lastmod: 'Wed, 15 Jan 2025 14:00:00 GMT', - size: 100, - type: 'file', - etag: 'Wed, 15 Jan 2025 14:00:00 GMT', - data: { - etag: '"propfind-etag-456"', // Original ETag in data - }, - path: '/test.txt', - }), - ); - - const result = await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: null, - }); - - expect(result).toEqual({ - rev: 'Wed, 15 Jan 2025 14:00:00 GMT', - legacyRev: 'propfind-etag-456', - lastModified: 'Wed, 15 Jan 2025 14:00:00 GMT', - }); - }); - - it('should handle upload with ETag in initial response', async () => { - const mockResponse = { - status: 201, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 15:00:00 GMT', - ETag: '"W/\\"weak-etag-789\\""', // Weak ETag with nested quotes - }, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - const result = await api.upload({ - path: '/test.txt', - data: 'new content', - expectedRev: null, - }); - - expect(result).toEqual({ - rev: 'Wed, 15 Jan 2025 15:00:00 GMT', - legacyRev: 'W\\weak-etag-789\\', // Cleaned (removes / and " but not \) - lastModified: 'Wed, 15 Jan 2025 15:00:00 GMT', - }); + expect(result).toEqual({ rev: expectedHash }); }); it('should throw InvalidDataSPError when upload data is empty string', async () => { @@ -976,7 +470,7 @@ describe('WebdavApi', () => { }); describe('remove', () => { - it('should remove file successfully', async () => { + it('should remove file with plain DELETE', async () => { const mockResponse = { status: 204, headers: {}, @@ -993,182 +487,6 @@ describe('WebdavApi', () => { }), ); }); - - it('should handle conditional delete with date string', async () => { - const mockResponse = { - status: 204, - headers: {}, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - await api.remove('/test.txt', 'Wed, 15 Jan 2025 10:00:00 GMT'); - - expect(mockHttpAdapter.request).toHaveBeenCalledWith( - jasmine.objectContaining({ - url: 'http://example.com/webdav/test.txt', - method: 'DELETE', - headers: jasmine.objectContaining({ - 'If-Unmodified-Since': jasmine.any(String), - }), - }), - ); - }); - - it('should add 1-second buffer to If-Unmodified-Since header for delete', async () => { - const mockResponse = { - status: 204, - headers: {}, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - // Use a known date: Wed, 15 Jan 2025 10:00:00 GMT - const expectedRev = 'Wed, 15 Jan 2025 10:00:00 GMT'; - // The buffered date should be: Wed, 15 Jan 2025 10:00:01 GMT - - await api.remove('/test.txt', expectedRev); - - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - // The header should have 1 second added (10:00:01 instead of 10:00:00) - expect(requestArgs.headers['If-Unmodified-Since']).toBe( - 'Wed, 15 Jan 2025 10:00:01 GMT', - ); - }); - - it('should NOT set If-Unmodified-Since when delete expectedRev is not a valid date', async () => { - const mockResponse = { - status: 204, - headers: {}, - data: '', - }; - mockHttpAdapter.request.and.returnValue(Promise.resolve(mockResponse)); - - // ETag that is not a valid date - await api.remove('/test.txt', 'abc123-etag-value'); - - const requestArgs = mockHttpAdapter.request.calls.mostRecent()?.args[0] as any; - // No If-Unmodified-Since header should be set for non-date revisions - expect(requestArgs.headers['If-Unmodified-Since']).toBeUndefined(); - }); - }); - - describe('_cleanRev', () => { - it('should clean revision strings', () => { - expect((api as any)._cleanRev('"abc123"')).toBe('abc123'); - expect((api as any)._cleanRev('abc/123')).toBe('abc123'); - expect((api as any)._cleanRev('"abc/123"')).toBe('abc123'); - expect((api as any)._cleanRev('"abc123"')).toBe('abc123'); - expect((api as any)._cleanRev('')).toBe(''); - }); - - it('should handle various ETag formats', () => { - // Standard ETag with quotes - expect((api as any)._cleanRev('"12345"')).toBe('12345'); - - // Weak ETag - expect((api as any)._cleanRev('W/"weak-etag"')).toBe('Wweak-etag'); - - // ETag with escaped quotes - expect((api as any)._cleanRev('"escaped\\\\"quotes\\\\""')).toBe( - 'escaped\\\\quotes\\\\', - ); - - // ETag with HTML entities - expect((api as any)._cleanRev('"html-entity"')).toBe('html-entity'); - - // Complex ETag with multiple special characters - expect((api as any)._cleanRev('"complex/etag/with/slashes"')).toBe( - 'complexetagwithslashes', - ); - - // Empty or null cases - expect((api as any)._cleanRev(null)).toBe(''); - expect((api as any)._cleanRev(undefined)).toBe(''); - expect((api as any)._cleanRev(' ')).toBe(''); - - // ETag with surrounding whitespace - expect((api as any)._cleanRev(' "whitespace" ')).toBe('whitespace'); - }); - }); - - describe('_getFileMetaViaHead', () => { - it('should parse HEAD response into FileMeta data', async () => { - const mockHeadResponse = { - status: 200, - headers: { - 'last-modified': 'Wed, 15 Jan 2025 15:00:00 GMT', - 'content-length': '128', - 'content-type': 'application/json', - }, - data: '', - }; - const requestSpy = spyOn(api, '_makeRequest').and.returnValue( - Promise.resolve(mockHeadResponse), - ); - - const result = await (api as any)._getFileMetaViaHead( - 'http://example.com/webdav/test.json', - ); - - expect(requestSpy).toHaveBeenCalledWith({ - url: 'http://example.com/webdav/test.json', - method: 'HEAD', - }); - expect(result).toEqual({ - filename: 'test.json', - basename: 'test.json', - lastmod: 'Wed, 15 Jan 2025 15:00:00 GMT', - size: 128, - type: 'application/json', - etag: 'Wed, 15 Jan 2025 15:00:00 GMT', - data: { - 'content-type': 'application/json', - 'content-length': '128', - 'last-modified': 'Wed, 15 Jan 2025 15:00:00 GMT', - etag: '', - href: 'http://example.com/webdav/test.json', - }, - path: 'http://example.com/webdav/test.json', - }); - }); - - it('should throw InvalidDataSPError when both Last-Modified and ETag headers are missing', async () => { - spyOn(api, '_makeRequest').and.returnValue( - Promise.resolve({ - status: 200, - headers: {}, - data: '', - }), - ); - - await expectAsync( - (api as any)._getFileMetaViaHead('http://example.com/webdav/test.json'), - ).toBeRejectedWith(jasmine.any(InvalidDataSPError)); - }); - - it('should use ETag as fallback when Last-Modified header is missing', async () => { - const mockHeadResponse = { - status: 200, - headers: { - etag: '"abc123-etag"', - 'content-length': '256', - 'content-type': 'application/json', - }, - data: '', - }; - spyOn(api, '_makeRequest').and.returnValue(Promise.resolve(mockHeadResponse)); - - const result = await (api as any)._getFileMetaViaHead( - 'http://example.com/webdav/test.json', - ); - - // ETag should be cleaned and used as lastmod - expect(result.lastmod).toBe('abc123-etag'); - expect(result.etag).toBe('abc123-etag'); - expect(result.filename).toBe('test.json'); - expect(result.size).toBe(256); - }); }); describe('_buildFullPath', () => { @@ -1215,10 +533,8 @@ describe('WebdavApi', () => { }); it('should fallback gracefully for invalid URLs', () => { - // Fallback behavior verification const invalidBase = 'not-a-valid-url'; const path = '/file.txt'; - // The fallback just concatenates and tries to encode expect((api as any)._buildFullPath(invalidBase, path)).toBe( 'not-a-valid-url/file.txt', ); @@ -1227,7 +543,6 @@ describe('WebdavApi', () => { describe('listFiles', () => { it('should handle invalid status codes safely when creating error Response', async () => { - // Simulate a response with an invalid status (e.g., 0 from a network failure) const mockResponse = { status: 0, headers: {}, @@ -1278,7 +593,6 @@ describe('WebdavApi', () => { describe('_createDirectory', () => { it('should re-throw unexpected errors instead of swallowing them', async () => { - // Simulate a 403 Forbidden error on MKCOL const errorResponse = new Response(null, { status: 403 }); const error = new HttpNotOkAPIError(errorResponse); @@ -1286,12 +600,10 @@ describe('WebdavApi', () => { mockHttpAdapter.request.and.callFake((params) => { callCount++; if (params.method === 'PUT' && callCount === 1) { - // First upload fails with 409 Conflict const conflictResponse = new Response(null, { status: 409 }); return Promise.reject(new HttpNotOkAPIError(conflictResponse)); } if (params.method === 'MKCOL') { - // Directory creation fails with 403 Permission Denied return Promise.reject(error); } return Promise.resolve({ status: 200, headers: {}, data: '' }); @@ -1309,6 +621,8 @@ describe('WebdavApi', () => { it('should not throw for known "directory exists" status codes', async () => { const errorResponse405 = new Response(null, { status: 405 }); const error405 = new HttpNotOkAPIError(errorResponse405); + const uploadData = 'content'; + const expectedHash = md5HashSync(uploadData); let callCount = 0; mockHttpAdapter.request.and.callFake((params) => { @@ -1318,25 +632,21 @@ describe('WebdavApi', () => { return Promise.reject(new HttpNotOkAPIError(conflictResponse)); } if (params.method === 'MKCOL') { - // 405 means directory already exists — should not throw + // 405 means directory already exists - should not throw return Promise.reject(error405); } if (params.method === 'PUT') { - return Promise.resolve({ - status: 201, - headers: { 'last-modified': 'Wed, 15 Jan 2025 11:00:00 GMT' }, - data: '', - }); + return Promise.resolve({ status: 201, headers: {}, data: '' }); } return Promise.resolve({ status: 200, headers: {}, data: '' }); }); const result = await api.upload({ path: '/folder/test.txt', - data: 'content', + data: uploadData, expectedRev: null, }); - expect(result.rev).toBe('Wed, 15 Jan 2025 11:00:00 GMT'); + expect(result).toEqual({ rev: expectedHash }); }); }); @@ -1355,13 +665,13 @@ describe('WebdavApi', () => { lastmod: '', size: 0, type: 'file', - etag: 'Wed, 15 Jan 2025 10:00:00 GMT', // Using lastmod as etag + etag: '', data: {}, path: '/test.txt', }, ]); - await api.getFileMeta('/test.txt', null); + await api.getFileMeta('/test.txt'); expect(mockGetCfg).toHaveBeenCalled(); }); @@ -1370,196 +680,7 @@ describe('WebdavApi', () => { const error = new Error('Config error'); mockGetCfg.and.returnValue(Promise.reject(error)); - await expectAsync(api.getFileMeta('/test.txt', null)).toBeRejectedWith(error); - }); - }); - - describe('testConditionalHeaders', () => { - it('should return true when server returns 412 for old date', async () => { - let requestCount = 0; - mockHttpAdapter.request.and.callFake((params: any) => { - requestCount++; - if (params.method === 'PUT' && requestCount === 1) { - // First upload succeeds - return Promise.resolve({ - status: 200, - headers: { 'last-modified': 'Thu, 01 Jan 2025 00:00:00 GMT' }, - data: '', - }); - } - if (params.method === 'PROPFIND') { - // getFileMeta returns lastmod - return Promise.resolve({ - status: 207, - headers: {}, - data: '', - }); - } - if (params.method === 'PUT' && requestCount >= 2) { - // Second upload with old date fails with 412 - return Promise.reject( - new HttpNotOkAPIError(new Response(null, { status: 412 })), - ); - } - if (params.method === 'DELETE') { - // Cleanup succeeds - return Promise.resolve({ - status: 204, - headers: {}, - data: '', - }); - } - return Promise.resolve({ - status: 200, - headers: {}, - data: '', - }); - }); - - mockXmlParser.parseMultiplePropsFromXml.and.returnValue([ - { - filename: 'test.txt', - basename: 'test.txt', - lastmod: 'Thu, 01 Jan 2025 00:00:00 GMT', - size: 100, - type: 'file', - etag: '"abc123"', - data: {}, - path: '/test.txt', - }, - ]); - - const result = await api.testConditionalHeaders('/test-path'); - expect(result).toBe(true); - }); - - it('should return false when server ignores If-Unmodified-Since', async () => { - mockHttpAdapter.request.and.callFake((params: any) => { - if (params.method === 'PUT') { - // Both uploads succeed (server ignores conditional header) - return Promise.resolve({ - status: 200, - headers: { 'last-modified': 'Thu, 01 Jan 2025 00:00:00 GMT' }, - data: '', - }); - } - if (params.method === 'PROPFIND') { - return Promise.resolve({ - status: 207, - headers: {}, - data: '', - }); - } - if (params.method === 'DELETE') { - return Promise.resolve({ - status: 204, - headers: {}, - data: '', - }); - } - return Promise.resolve({ - status: 200, - headers: {}, - data: '', - }); - }); - - mockXmlParser.parseMultiplePropsFromXml.and.returnValue([ - { - filename: 'test.txt', - basename: 'test.txt', - lastmod: 'Thu, 01 Jan 2025 00:00:00 GMT', - size: 100, - type: 'file', - etag: '"abc123"', - data: {}, - path: '/test.txt', - }, - ]); - - const result = await api.testConditionalHeaders('/test-path'); - expect(result).toBe(false); - }); - - it('should return false when server does not return lastmod', async () => { - mockHttpAdapter.request.and.callFake((params: any) => { - if (params.method === 'PUT') { - return Promise.resolve({ - status: 200, - headers: {}, - data: '', - }); - } - if (params.method === 'PROPFIND') { - return Promise.resolve({ - status: 207, - headers: {}, - data: '', - }); - } - if (params.method === 'DELETE') { - return Promise.resolve({ - status: 204, - headers: {}, - data: '', - }); - } - return Promise.resolve({ - status: 200, - headers: {}, - data: '', - }); - }); - - // No lastmod in metadata - mockXmlParser.parseMultiplePropsFromXml.and.returnValue([ - { - filename: 'test.txt', - basename: 'test.txt', - lastmod: '', - size: 100, - type: 'file', - etag: '"abc123"', - data: {}, - path: '/test.txt', - }, - ]); - - const result = await api.testConditionalHeaders('/test-path'); - expect(result).toBe(false); - }); - - it('should clean up test file even on error', async () => { - let deleteWasCalled = false; - mockHttpAdapter.request.and.callFake((params: any) => { - if (params.method === 'PUT') { - return Promise.resolve({ - status: 200, - headers: { 'last-modified': 'Thu, 01 Jan 2025 00:00:00 GMT' }, - data: '', - }); - } - if (params.method === 'PROPFIND') { - // Error during metadata retrieval - return Promise.reject(new Error('Network error')); - } - if (params.method === 'DELETE') { - deleteWasCalled = true; - return Promise.resolve({ - status: 204, - headers: {}, - data: '', - }); - } - return Promise.resolve({ - status: 200, - headers: {}, - data: '', - }); - }); - - await expectAsync(api.testConditionalHeaders('/test-path')).toBeRejected(); - expect(deleteWasCalled).toBe(true); + await expectAsync(api.getFileMeta('/test.txt')).toBeRejectedWith(error); }); }); }); diff --git a/src/app/op-log/sync-providers/file-based/webdav/webdav-api.ts b/src/app/op-log/sync-providers/file-based/webdav/webdav-api.ts index 49171dd5e5..3b835ee29b 100644 --- a/src/app/op-log/sync-providers/file-based/webdav/webdav-api.ts +++ b/src/app/op-log/sync-providers/file-based/webdav/webdav-api.ts @@ -1,16 +1,16 @@ import { WebdavPrivateCfg } from './webdav.model'; -import { Log, SyncLog } from '../../../../core/log'; +import { SyncLog } from '../../../../core/log'; import { FileMeta, WebdavXmlParser } from './webdav-xml-parser'; import { WebDavHttpAdapter, WebDavHttpResponse } from './webdav-http-adapter'; import { HttpNotOkAPIError, InvalidDataSPError, MissingCredentialsSPError, - NoRevAPIError, RemoteFileChangedUnexpectedly, RemoteFileNotFoundAPIError, } from '../../../core/errors/sync-errors'; import { WebDavHttpHeader, WebDavHttpMethod, WebDavHttpStatus } from './webdav.const'; +import { md5HashSync } from '../../../../util/md5-hash'; export class WebdavApi { private static readonly L = 'WebdavApi'; @@ -19,10 +19,14 @@ export class WebdavApi { private directoryCreationQueue = new Map>(); constructor(private _getCfgOrError: () => Promise) { - this.xmlParser = new WebdavXmlParser((rev: string) => this._cleanRev(rev)); + this.xmlParser = new WebdavXmlParser(); this.httpAdapter = new WebDavHttpAdapter(); } + private _computeContentHash(data: string): string { + return md5HashSync(data); + } + // ============================== // File Operations // ============================== @@ -78,18 +82,14 @@ export class WebdavApi { } /** - * Retrieve metadata for a file or folder + * Retrieve metadata for a file or folder via PROPFIND. + * Used for testConnection() and listFiles(), not for revision tracking. */ - async getFileMeta( - path: string, - _localRev: string | null, - useGetFallback: boolean = false, - ): Promise { + async getFileMeta(path: string): Promise { const cfg = await this._getCfgOrError(); const fullPath = this._buildFullPath(cfg.baseUrl, path); try { - // Try PROPFIND first const response = await this._makeRequest({ url: fullPath, method: WebDavHttpMethod.PROPFIND, @@ -103,49 +103,20 @@ export class WebdavApi { if (response.status === WebDavHttpStatus.MULTI_STATUS) { const files = this.xmlParser.parseMultiplePropsFromXml(response.data, path); if (files && files.length > 0) { - const meta = files[0]; - SyncLog.verbose(`${WebdavApi.L}.getFileMeta() PROPFIND success for ${path}`, { - lastmod: meta.lastmod, - }); - return meta; + return files[0]; } } } catch (e) { - // If PROPFIND fails and fallback is enabled, try HEAD - if (useGetFallback) { - SyncLog.verbose( - `${WebdavApi.L}.getFileMeta() PROPFIND failed, trying HEAD fallback`, - e, - ); - try { - return await this._getFileMetaViaHead(fullPath); - } catch (headErr) { - SyncLog.warn( - `${WebdavApi.L}.getFileMeta() HEAD fallback failed for ${path}`, - headErr, - ); - // If HEAD also fails, throw the original error (or maybe the HEAD error?) - // Usually the original PROPFIND error is more informative about connectivity - } - } SyncLog.error(`${WebdavApi.L}.getFileMeta() error`, { path, error: e }); throw e; } - // If we get here, PROPFIND worked but returned no data (or not MULTI_STATUS) - // Try HEAD request as fallback if enabled - if (useGetFallback) { - return await this._getFileMetaViaHead(fullPath); - } - throw new RemoteFileNotFoundAPIError(path); } async download({ path }: { path: string }): Promise<{ rev: string; - legacyRev?: string; dataStr: string; - lastModified?: string; }> { const cfg = await this._getCfgOrError(); const fullPath = this._buildFullPath(cfg.baseUrl, path); @@ -171,64 +142,9 @@ export class WebdavApi { 'file content', ); - // Get revision from Last-Modified - let lastModified = - response.headers['last-modified'] || response.headers['Last-Modified']; - - // Get ETag for legacy compatibility - const etagHeader = response.headers['etag'] || response.headers['ETag']; - let legacyRev = etagHeader ? this._cleanRev(etagHeader) : undefined; - - let rev = lastModified || ''; - const isLastModifiedMissing = !lastModified; - const isLegacyRevMissing = !legacyRev; - - // Fallback: Some servers may omit Last-Modified on GET, so request metadata separately - if (isLastModifiedMissing) { - SyncLog.verbose( - `${WebdavApi.L}.download() missing Last-Modified header, trying metadata fallback for ${path}`, - ); - try { - const meta = await this.getFileMeta(path, null, true); - if (!lastModified && meta.lastmod) { - lastModified = meta.lastmod; - rev = lastModified; - } - if (isLegacyRevMissing) { - const metaEtag = meta.data?.etag; - if (metaEtag) { - legacyRev = this._cleanRev(metaEtag); - } - } - } catch (e) { - SyncLog.warn( - `${WebdavApi.L}.download() metadata fallback failed for ${path}`, - e, - ); - } - } - - // Fallback to ETag if Last-Modified is still not available - if (!rev && legacyRev) { - rev = legacyRev; - SyncLog.warn( - `${WebdavApi.L}.download() no Last-Modified for ${path}, using ETag as revision.`, - ); - } - - if (!rev) { - SyncLog.err( - `${WebdavApi.L}.download() no revision markers (Last-Modified or ETag) found for ${path}. ` + - `Check your WebDAV server or reverse proxy configuration.`, - ); - throw new NoRevAPIError(`No revision markers available for: ${path}`); - } - return { - rev, - legacyRev, + rev: this._computeContentHash(response.data), dataStr: response.data, - lastModified, }; } catch (e) { SyncLog.error(`${WebdavApi.L}.download() error`, { path, error: e }); @@ -246,7 +162,7 @@ export class WebdavApi { data: string; expectedRev?: string | null; isForceOverwrite?: boolean; - }): Promise<{ rev: string; legacyRev?: string; lastModified?: string }> { + }): Promise<{ rev: string }> { // Guard against empty upload data — prevents overwriting remote file with zero bytes. // This can happen when the Capacitor bridge drops the payload on Android. // Symmetric with the download guard at the download() method. @@ -260,71 +176,50 @@ export class WebdavApi { const fullPath = this._buildFullPath(cfg.baseUrl, path); try { - // Prepare headers for upload + // Application-level conflict detection: download current file and compare hash + if (!isForceOverwrite && expectedRev) { + try { + const currentResponse = await this._makeRequest({ + url: fullPath, + method: WebDavHttpMethod.GET, + }); + const currentHash = this._computeContentHash(currentResponse.data); + if (currentHash !== expectedRev) { + throw new RemoteFileChangedUnexpectedly( + `File ${path} was modified on remote (expected rev: ${expectedRev}, got: ${currentHash})`, + ); + } + } catch (e) { + // 404 means file doesn't exist yet — safe to proceed with upload + if (!(e instanceof RemoteFileNotFoundAPIError)) { + throw e; + } + } + } + const headers: Record = { [WebDavHttpHeader.CONTENT_TYPE]: 'application/octet-stream', }; - // Set conditional headers based on revision type - if (!isForceOverwrite && expectedRev) { - // Try to parse as date first - const parsedDate = new Date(expectedRev); - if (isNaN(parsedDate.getTime())) { - // Not a valid date - treat as ETag and use If-Match header - // ETags should be quoted per RFC 7232 - const quotedEtag = expectedRev.startsWith('"') - ? expectedRev - : `"${expectedRev}"`; - headers[WebDavHttpHeader.IF_MATCH] = quotedEtag; - Log.verbose(WebdavApi.L, 'Using If-Match with ETag', quotedEtag); - } else { - // Valid date - use If-Unmodified-Since header - // Add 1 second buffer to handle sub-second filesystem precision differences. - // Some WebDAV servers store mtimes with millisecond precision but HTTP headers - // only support second-level precision, causing false 412 Precondition Failed errors. - // See: https://github.com/super-productivity/super-productivity/issues/6218 - const bufferedDate = new Date(parsedDate.getTime() + 1000); - headers[WebDavHttpHeader.IF_UNMODIFIED_SINCE] = bufferedDate.toUTCString(); - Log.verbose( - WebdavApi.L, - 'Using If-Unmodified-Since (with 1s buffer)', - bufferedDate.toUTCString(), - ); - } - } - // Try to upload the file - let response: WebDavHttpResponse; try { - response = await this._makeRequest({ + await this._makeRequest({ url: fullPath, method: WebDavHttpMethod.PUT, body: data, headers, }); } catch (uploadError) { - // Check for 412 Precondition Failed - means file was modified if ( - uploadError instanceof HttpNotOkAPIError && - uploadError.response && - uploadError.response.status === WebDavHttpStatus.PRECONDITION_FAILED - ) { - throw new RemoteFileChangedUnexpectedly( - `File ${path} was modified on remote (expected rev: ${expectedRev})`, - ); - } - - if ( - // if we get a 404 on upload this also indicates that the directory does not exist (for nextcloud) + // 404 on upload indicates the directory does not exist (Nextcloud) uploadError instanceof RemoteFileNotFoundAPIError || (uploadError instanceof HttpNotOkAPIError && uploadError.response && - // If we get a 409 Conflict, it might be because parent directory doesn't exist + // 409 Conflict — parent directory doesn't exist uploadError.response.status === WebDavHttpStatus.CONFLICT) ) { SyncLog.debug( - `${WebdavApi.L}.upload() got 409 Conflict for ${fullPath}. ` + - `This often indicates the sync folder path is misconfigured. ` + + `${WebdavApi.L}.upload() got 404/409 for ${fullPath}. ` + `Attempting to create parent directory...`, ); @@ -333,14 +228,13 @@ export class WebdavApi { // Retry the upload try { - response = await this._makeRequest({ + await this._makeRequest({ url: fullPath, method: WebDavHttpMethod.PUT, body: data, headers, }); } catch (retryError) { - // If retry also fails with 409, log a helpful error message if ( retryError instanceof HttpNotOkAPIError && retryError.response && @@ -359,86 +253,21 @@ export class WebdavApi { } } - // Get the new revision from Last-Modified - const lastModified = - response.headers['last-modified'] || response.headers['Last-Modified']; - - // Get ETag for legacy compatibility - const etag = response.headers['etag'] || response.headers['ETag']; - const legacyRev = etag ? this._cleanRev(etag) : undefined; - - let rev = lastModified || ''; - - if (!rev) { - // Some WebDAV servers don't return Last-Modified on PUT - // Try to get it from a HEAD request first (cheaper than PROPFIND) - SyncLog.verbose( - `${WebdavApi.L}.upload() no Last-Modified in PUT response, fetching via HEAD`, - ); - try { - const headResponse = await this._makeRequest({ - url: fullPath, - method: WebDavHttpMethod.HEAD, - }); - const headLastMod = - headResponse.headers['last-modified'] || - headResponse.headers['Last-Modified']; - rev = headLastMod || ''; - - if (rev) { - // Try to get ETag from HEAD response for legacy compatibility - const headEtag = headResponse.headers['etag'] || headResponse.headers['ETag']; - const headLegacyRev = headEtag ? this._cleanRev(headEtag) : undefined; - return { rev, legacyRev: headLegacyRev, lastModified: rev }; - } - } catch (headError) { - SyncLog.verbose( - `${WebdavApi.L}.upload() HEAD request failed, falling back to PROPFIND`, - headError, - ); - } - - // If HEAD didn't work, fall back to PROPFIND - const meta = await this.getFileMeta(path, null, true); - // Extract original ETag from meta.data if available - const metaEtag = meta.data?.etag; - const metaLegacyRev = metaEtag ? this._cleanRev(metaEtag) : undefined; - return { - rev: meta.lastmod, - legacyRev: metaLegacyRev, - lastModified: meta.lastmod, - }; - } - - return { rev, legacyRev, lastModified }; + return { rev: this._computeContentHash(data) }; } catch (e) { SyncLog.error(`${WebdavApi.L}.upload() error`, { path, error: e }); throw e; } } - async remove(path: string, expectedRev?: string): Promise { + async remove(path: string): Promise { const cfg = await this._getCfgOrError(); const fullPath = this._buildFullPath(cfg.baseUrl, path); try { - const headers: Record = {}; - - if (expectedRev) { - // Try to parse as date for If-Unmodified-Since - const parsedDate = new Date(expectedRev); - if (!isNaN(parsedDate.getTime())) { - // Add 1 second buffer to handle sub-second filesystem precision differences. - // See: https://github.com/super-productivity/super-productivity/issues/6218 - const bufferedDate = new Date(parsedDate.getTime() + 1000); - headers[WebDavHttpHeader.IF_UNMODIFIED_SINCE] = bufferedDate.toUTCString(); - } - } - await this._makeRequest({ url: fullPath, method: WebDavHttpMethod.DELETE, - headers, }); SyncLog.verbose(`${WebdavApi.L}.remove() success for ${path}`); @@ -491,76 +320,6 @@ export class WebdavApi { } } - /** - * Tests if the WebDAV server properly supports If-Unmodified-Since conditional headers. - * - * This method: - * 1. Uploads a test file - * 2. Gets its last-modified timestamp - * 3. Tries to upload again with an old If-Unmodified-Since date (1 day before) - * 4. If the upload succeeds (when it should fail with 412), headers are NOT supported - * 5. If the upload fails with 412 Precondition Failed, headers ARE supported - * - * @param testPath - Path where the test file will be created (will be cleaned up) - * @returns true if conditional headers are properly supported, false otherwise - */ - async testConditionalHeaders(testPath: string): Promise { - const testContent = `test-${Date.now()}`; - SyncLog.normal( - `${WebdavApi.L}.testConditionalHeaders() testing with path: ${testPath}`, - ); - - try { - // Step 1: Upload test file (force overwrite to ensure it gets created) - await this.upload({ - path: testPath, - data: testContent, - isForceOverwrite: true, - }); - - // Step 2: Get its timestamp - const meta = await this.getFileMeta(testPath, null, true); - const currentRev = meta.lastmod; - - if (!currentRev) { - SyncLog.warn( - `${WebdavApi.L}.testConditionalHeaders() Server did not return lastmod - cannot test conditional headers`, - ); - return false; - } - - // Step 3: Try to upload with an OLD If-Unmodified-Since (1 day before) - const oldDate = new Date(new Date(currentRev).getTime() - 86400000).toUTCString(); - try { - await this.upload({ - path: testPath, - data: testContent + '-v2', - expectedRev: oldDate, - }); - // Upload succeeded when it should have failed with 412 - SyncLog.warn( - `${WebdavApi.L}.testConditionalHeaders() Server ignored If-Unmodified-Since header - conditional headers NOT supported`, - ); - return false; // Headers NOT supported - } catch (e) { - if (e instanceof RemoteFileChangedUnexpectedly) { - SyncLog.normal( - `${WebdavApi.L}.testConditionalHeaders() Server properly returned 412 - conditional headers ARE supported`, - ); - return true; // Headers ARE supported (got 412 as expected) - } - throw e; // Unexpected error - } - } finally { - // Clean up test file - try { - await this.remove(testPath); - } catch { - // Ignore cleanup errors - } - } - } - private async _makeRequest({ url, method, @@ -729,82 +488,4 @@ export class WebdavApi { return `${protocol}${normalizedBase}${normalizedPath}`; } } - - private _cleanRev(rev: string): string { - // Clean ETag values for legacy compatibility - // Remove quotes, slashes, and HTML entities - if (!rev) return ''; - return rev - .replace(/"/g, '') - .replace(/\//g, '') - .replace(/"/g, '') - .trim(); - } - - private async _getFileMetaViaHead(fullPath: string): Promise { - const response = await this._makeRequest({ - url: fullPath, - method: WebDavHttpMethod.HEAD, - }); - - // Safely access headers with null checks - const headers = response.headers || {}; - const lastModified = headers['last-modified'] || headers['Last-Modified'] || ''; - const contentLength = headers['content-length'] || headers['Content-Length'] || '0'; - const contentType = headers['content-type'] || headers['Content-Type'] || ''; - const etag = headers['etag'] || headers['ETag'] || ''; - - // Determine effective lastmod: prefer Last-Modified, fall back to ETag - let effectiveLastmod = lastModified; - if (!lastModified && etag) { - effectiveLastmod = this._cleanRev(etag); - SyncLog.warn( - `${WebdavApi.L}._getFileMetaViaHead() No Last-Modified header for ${fullPath}, using ETag as revision. ` + - `This may indicate a reverse proxy or server configuration issue.`, - ); - } - - if (!effectiveLastmod) { - throw new InvalidDataSPError( - `No Last-Modified or ETag headers in HEAD response for ${fullPath}. ` + - `Your WebDAV server or reverse proxy may be stripping headers. ` + - `Check server configuration or reverse proxy header forwarding settings.`, - ); - } - - // Extract filename from path - const filename = fullPath.split('/').pop() || ''; - - // Safely parse content length with validation - let size = 0; - try { - const parsedSize = parseInt(contentLength, 10); - if (!isNaN(parsedSize) && parsedSize >= 0) { - size = parsedSize; - } - } catch (e) { - SyncLog.warn( - `${WebdavApi.L}._getFileMetaViaHead() invalid content-length: ${contentLength}`, - ); - } - - return { - filename, - basename: filename, - lastmod: effectiveLastmod, - size, - type: contentType || 'application/octet-stream', - etag: effectiveLastmod, // Use effective lastmod for consistency - data: { - /* eslint-disable @typescript-eslint/naming-convention */ - 'content-type': contentType, - 'content-length': contentLength, - 'last-modified': effectiveLastmod, - /* eslint-enable @typescript-eslint/naming-convention */ - etag: etag, - href: fullPath, - }, - path: fullPath, - }; - } } diff --git a/src/app/op-log/sync-providers/file-based/webdav/webdav-base-provider.ts b/src/app/op-log/sync-providers/file-based/webdav/webdav-base-provider.ts index 7e2487b7c7..2a73f47ca1 100644 --- a/src/app/op-log/sync-providers/file-based/webdav/webdav-base-provider.ts +++ b/src/app/op-log/sync-providers/file-based/webdav/webdav-base-provider.ts @@ -6,7 +6,6 @@ import { SyncCredentialStore } from '../../credential-store.service'; import { InvalidDataSPError, MissingCredentialsSPError, - NoRevAPIError, RemoteFileChangedUnexpectedly, UploadRevToMatchMismatchAPIError, } from '../../../core/errors/sync-errors'; @@ -69,11 +68,10 @@ export abstract class WebdavBaseProvider< async getFileRev( targetPath: string, - localRev: string | null, + _localRev: string | null, ): Promise<{ rev: string }> { - const { filePath } = await this._getConfigAndPath(targetPath); - const meta = await this._api.getFileMeta(filePath, localRev, true); - return { rev: meta.lastmod }; + const r = await this.downloadFile(targetPath); + return { rev: r.rev }; } async uploadFile( @@ -89,14 +87,14 @@ export abstract class WebdavBaseProvider< }); const { filePath } = await this._getConfigAndPath(targetPath); - let result; try { - result = await this._api.upload({ + const result = await this._api.upload({ path: filePath, data: dataStr, isForceOverwrite: isForceOverwrite, expectedRev: isForceOverwrite ? null : localRev, }); + return { rev: result.rev }; } catch (e) { // Translate RemoteFileChangedUnexpectedly to UploadRevToMatchMismatchAPIError // so the retry mechanism in FileBasedSyncAdapterService._uploadWithRetry() can handle it @@ -105,17 +103,9 @@ export abstract class WebdavBaseProvider< } throw e; } - - if (!result.rev) { - throw new NoRevAPIError(); - } - - return { rev: result.rev }; } - async downloadFile( - targetPath: string, - ): Promise<{ rev: string; legacyRev?: string; dataStr: string }> { + async downloadFile(targetPath: string): Promise<{ rev: string; dataStr: string }> { SyncLog.debug(this.logLabel, 'downloadFile', { targetPath }); const { filePath } = await this._getConfigAndPath(targetPath); @@ -126,11 +116,8 @@ export abstract class WebdavBaseProvider< if (result.dataStr == null) { throw new InvalidDataSPError(targetPath); } - if (typeof result.rev !== 'string') { - throw new NoRevAPIError(); - } - return { rev: result.rev, legacyRev: result.legacyRev, dataStr: result.dataStr }; + return { rev: result.rev, dataStr: result.dataStr }; } async removeFile(targetPath: string): Promise { diff --git a/src/app/op-log/sync-providers/file-based/webdav/webdav-xml-parser.spec.ts b/src/app/op-log/sync-providers/file-based/webdav/webdav-xml-parser.spec.ts index 3394a8e766..1ee1f65eb1 100644 --- a/src/app/op-log/sync-providers/file-based/webdav/webdav-xml-parser.spec.ts +++ b/src/app/op-log/sync-providers/file-based/webdav/webdav-xml-parser.spec.ts @@ -4,7 +4,7 @@ describe('WebdavXmlParser', () => { let parser: WebdavXmlParser; beforeEach(() => { - parser = new WebdavXmlParser((rev: string) => rev.replace(/"/g, '')); + parser = new WebdavXmlParser(); }); describe('PROPFIND_XML', () => { @@ -201,10 +201,7 @@ describe('WebdavXmlParser', () => { }); it('should use lastmod as etag when present', () => { - const cleanRevFn = jasmine - .createSpy('cleanRevFn') - .and.callFake((rev: string) => rev.replace(/"/g, '').toUpperCase()); - const customParser = new WebdavXmlParser(cleanRevFn); + const customParser = new WebdavXmlParser(); const xml = ` @@ -221,7 +218,6 @@ describe('WebdavXmlParser', () => { `; const results = customParser.parseMultiplePropsFromXml(xml, '/test.txt'); - // cleanRevFn should no longer be called for the etag expect(results[0].etag).toBe('Wed, 15 Jan 2025 10:00:00 GMT'); }); diff --git a/src/app/op-log/sync-providers/file-based/webdav/webdav-xml-parser.ts b/src/app/op-log/sync-providers/file-based/webdav/webdav-xml-parser.ts index 6eeed0e2b4..827fba95f3 100644 --- a/src/app/op-log/sync-providers/file-based/webdav/webdav-xml-parser.ts +++ b/src/app/op-log/sync-providers/file-based/webdav/webdav-xml-parser.ts @@ -29,7 +29,7 @@ export class WebdavXmlParser { `; - constructor(private _cleanRev: (rev: string) => string) {} + constructor() {} /** * Validates that response content is not an HTML error page diff --git a/src/app/op-log/sync-providers/file-based/webdav/webdav.const.ts b/src/app/op-log/sync-providers/file-based/webdav/webdav.const.ts index 609bae4420..ccef448f14 100644 --- a/src/app/op-log/sync-providers/file-based/webdav/webdav.const.ts +++ b/src/app/op-log/sync-providers/file-based/webdav/webdav.const.ts @@ -12,7 +12,6 @@ export const WebDavHttpStatus = { NOT_FOUND: 404, METHOD_NOT_ALLOWED: 405, CONFLICT: 409, - PRECONDITION_FAILED: 412, TOO_MANY_REQUESTS: 429, INTERNAL_SERVER_ERROR: 500, } as const; @@ -24,7 +23,6 @@ export const WebDavHttpMethod = { GET: 'GET', PUT: 'PUT', DELETE: 'DELETE', - HEAD: 'HEAD', PROPFIND: 'PROPFIND', MKCOL: 'MKCOL', } as const; @@ -35,13 +33,6 @@ export const WebDavHttpMethod = { export const WebDavHttpHeader = { AUTHORIZATION: 'Authorization', CONTENT_TYPE: 'Content-Type', - ETAG: 'ETag', - IF_MATCH: 'If-Match', - IF_NONE_MATCH: 'If-None-Match', - IF_MODIFIED_SINCE: 'If-Modified-Since', - IF_UNMODIFIED_SINCE: 'If-Unmodified-Since', - LAST_MODIFIED: 'Last-Modified', CONTENT_LENGTH: 'Content-Length', DEPTH: 'Depth', - RANGE: 'Range', } as const; diff --git a/src/app/op-log/sync-providers/file-based/webdav/webdav.model.ts b/src/app/op-log/sync-providers/file-based/webdav/webdav.model.ts index b2d15ef35b..0edc130767 100644 --- a/src/app/op-log/sync-providers/file-based/webdav/webdav.model.ts +++ b/src/app/op-log/sync-providers/file-based/webdav/webdav.model.ts @@ -1,14 +1,5 @@ import { SyncProviderPrivateCfgBase } from '../../../core/types/sync.types'; -export interface WebdavServerCapabilities { - /** Whether the server supports ETag headers for versioning */ - supportsETags: boolean; - /** Whether the server supports WebDAV If headers (RFC 4918) */ - supportsIfHeader: boolean; - /** Whether the server supports Last-Modified headers for versioning */ - supportsLastModified: boolean; -} - export interface WebdavPrivateCfg extends SyncProviderPrivateCfgBase { baseUrl: string; userName: string; @@ -16,48 +7,4 @@ export interface WebdavPrivateCfg extends SyncProviderPrivateCfgBase { // Optional access token for Bearer auth (e.g. SuperSync) accessToken?: string; syncFolderPath?: string; - - /** - * Server capabilities configuration. If not provided, capabilities will be - * detected automatically on first use. Providing this configuration can - * improve performance by skipping detection and ensure consistent behavior. - * - * Recommended settings for common servers: - * - Nextcloud/ownCloud: { supportsETags: true, supportsIfHeader: true, supportsLastModified: true } - * - Apache mod_dav: { supportsETags: true, supportsIfHeader: false, supportsLastModified: true } - * - Basic WebDAV: { supportsETags: false, supportsIfHeader: false, supportsLastModified: true } - */ - serverCapabilities?: WebdavServerCapabilities; - - /** - * Force the use of Last-Modified headers instead of ETags, even if ETags are available. - * This can be useful for testing fallback behavior or working with servers that have - * unreliable ETag implementations. - */ - preferLastModified?: boolean; - - /** - * Compatibility mode for servers with limited WebDAV support. - * When enabled, disables conditional operations and safe creation mechanisms. - * Use only for very basic WebDAV servers that don't support any conditional headers. - */ - basicCompatibilityMode?: boolean; - - /** - * Maximum number of retry attempts for capability detection and fallback operations. - * Default: 2 - */ - maxRetries?: number; -} - -/** - * Server type enum for common WebDAV implementations - */ -export enum WebdavServerType { - NEXTCLOUD = 'nextcloud', - OWNCLOUD = 'owncloud', - APACHE_MOD_DAV = 'apache_mod_dav', - NGINX_DAV = 'nginx_dav', - BASIC_WEBDAV = 'basic_webdav', - CUSTOM = 'custom', } diff --git a/src/app/op-log/sync-providers/provider.interface.ts b/src/app/op-log/sync-providers/provider.interface.ts index c80b8538f5..442ae4bb6d 100644 --- a/src/app/op-log/sync-providers/provider.interface.ts +++ b/src/app/op-log/sync-providers/provider.interface.ts @@ -148,7 +148,6 @@ export const isFileSyncProvider = ( export interface FileRevResponse { /** The current revision identifier for the file */ rev: string; - legacyRev?: string; } /** diff --git a/src/app/pages/config-page/config-page.component.spec.ts b/src/app/pages/config-page/config-page.component.spec.ts index 41b52cc615..190defe759 100644 --- a/src/app/pages/config-page/config-page.component.spec.ts +++ b/src/app/pages/config-page/config-page.component.spec.ts @@ -84,9 +84,6 @@ describe('ConfigPageComponent', () => { fullUrl: 'https://webdav.example.com/sp-test', }), ); - spyOn(WebdavApi.prototype, 'testConditionalHeaders').and.returnValue( - Promise.resolve(true), - ); const webDavCfg = { baseUrl: 'https://webdav.example.com', diff --git a/src/app/pages/config-page/config-page.component.ts b/src/app/pages/config-page/config-page.component.ts index fb59e276ab..30374439cb 100644 --- a/src/app/pages/config-page/config-page.component.ts +++ b/src/app/pages/config-page/config-page.component.ts @@ -61,7 +61,6 @@ import { DialogDisableProfilesConfirmationComponent } from '../../features/user- import { DialogRestorePointComponent } from '../../imex/sync/dialog-restore-point/dialog-restore-point.component'; import { SyncProviderId } from '../../op-log/sync-providers/provider.const'; import { DialogConfirmComponent } from '../../ui/dialog-confirm/dialog-confirm.component'; -import { LS } from '../../core/persistence/storage-keys.const'; import { MatTab, MatTabGroup, MatTabLabel } from '@angular/material/tabs'; import { MatIcon } from '@angular/material/icon'; import { MatTooltip } from '@angular/material/tooltip'; @@ -324,45 +323,6 @@ export class ConfigPageComponent implements OnInit, OnDestroy { true, ); } - - // Test conditional header support - const testPath = `${webDavCfg.syncFolderPath || '/'}/.sp-header-test-${Date.now()}`; - try { - const supportsHeaders = - await api.testConditionalHeaders(testPath); - - if ( - !supportsHeaders && - !localStorage.getItem( - LS.WEBDAV_CONDITIONAL_HEADER_WARNING_DISMISSED, - ) - ) { - const dialogRef = this._matDialog.open(DialogConfirmComponent, { - data: { - title: - T.F.SYNC.FORM.WEB_DAV.CONDITIONAL_HEADER_WARNING_TITLE, - message: - T.F.SYNC.FORM.WEB_DAV.CONDITIONAL_HEADER_WARNING_MSG, - okTxt: T.G.OK, - hideCancelButton: true, - showDontShowAgain: true, - }, - }); - const res = await firstValueFrom(dialogRef.afterClosed()); - if (res?.dontShowAgain) { - localStorage.setItem( - LS.WEBDAV_CONDITIONAL_HEADER_WARNING_DISMISSED, - 'true', - ); - } - } - } catch (headerTestError) { - // Ignore header test errors - connection test was successful - Log.warn( - 'WebDAV conditional header test failed:', - headerTestError, - ); - } } else { this._snackService.open({ type: 'ERROR', diff --git a/src/app/t.const.ts b/src/app/t.const.ts index 2ecf083a0e..59f628f94f 100644 --- a/src/app/t.const.ts +++ b/src/app/t.const.ts @@ -1334,10 +1334,6 @@ const T = { }, TITLE: 'F.SYNC.FORM.TITLE', WEB_DAV: { - CONDITIONAL_HEADER_WARNING_MSG: - 'F.SYNC.FORM.WEB_DAV.CONDITIONAL_HEADER_WARNING_MSG', - CONDITIONAL_HEADER_WARNING_TITLE: - 'F.SYNC.FORM.WEB_DAV.CONDITIONAL_HEADER_WARNING_TITLE', CORS_INFO: 'F.SYNC.FORM.WEB_DAV.CORS_INFO', D_SYNC_FOLDER_PATH: 'F.SYNC.FORM.WEB_DAV.D_SYNC_FOLDER_PATH', INFO: 'F.SYNC.FORM.WEB_DAV.INFO', diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index 9bcf419c8c..b57bfd157a 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -1297,8 +1297,6 @@ }, "TITLE": "Sync", "WEB_DAV": { - "CONDITIONAL_HEADER_WARNING_MSG": "Your WebDAV server does not support conditional headers (If-Unmodified-Since). This means Super Productivity cannot detect concurrent changes from other devices during upload.

Sync will still work, but there is a small risk of data conflicts if you make changes on multiple devices simultaneously.", - "CONDITIONAL_HEADER_WARNING_TITLE": "Reduced Sync Safety", "CORS_INFO": "Making it work in a web browser: You need to allow Super Productivity to make CORS requests to your server. For Nextcloud, one approach is allowing \"https://app.super-productivity.com\" via the webapppassword app. Refer to this GitHub thread for more information.", "D_SYNC_FOLDER_PATH": "Path relative to the WebDAV server root where sync files will be stored (e.g. '/super-productivity' or '/'). This is NOT your server's internal directory path.", "INFO": "Warning: Generic WebDAV sync is provided as-is without support. If you are using Nextcloud, please use the Nextcloud sync option instead.",