Skip to content

Commit 9327b65

Browse files
camilasancamilasan
authored
Merge pull request #10135 from nextcloud/bugfix/fpext-invalidate-lock-tokens
fix(file-provider): invalidate lock tokens when file paths change.
2 parents f6d4f94 + 94a6df4 commit 9327b65

8 files changed

Lines changed: 435 additions & 6 deletions

File tree

‎shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager+Directories.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ public extension FilesDatabaseManager {
151151
of: oldDirectoryServerUrl, with: newDirectoryServerUrl
152152
)
153153
childItem.serverUrl = movedServerUrl
154+
childItem.lockToken = nil
154155
database.add(childItem, update: .all)
155156
logger.debug(
156157
"""

‎shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,10 +277,11 @@ public final class FilesDatabaseManager: Sendable {
277277
for var updatedMetadata in updatedMetadatas {
278278
if let existingMetadata = existingMetadatas.first(where: { $0.ocId == updatedMetadata.ocId }) {
279279
if existingMetadata.status == Status.normal.rawValue, !existingMetadata.isInSameDatabaseStoreableRemoteState(updatedMetadata) {
280-
if updatedMetadata.directory,
281-
updatedMetadata.serverUrl != existingMetadata.serverUrl ||
282-
updatedMetadata.fileName != existingMetadata.fileName
283-
{
280+
let pathChanged =
281+
updatedMetadata.serverUrl != existingMetadata.serverUrl ||
282+
updatedMetadata.fileName != existingMetadata.fileName
283+
284+
if updatedMetadata.directory, pathChanged {
284285
directoriesNeedingRename.append(updatedMetadata)
285286
}
286287

@@ -290,7 +291,7 @@ public final class FilesDatabaseManager: Sendable {
290291

291292
updatedMetadata.visitedDirectory = existingMetadata.visitedDirectory
292293
updatedMetadata.keepDownloaded = existingMetadata.keepDownloaded
293-
updatedMetadata.lockToken = existingMetadata.lockToken
294+
updatedMetadata.lockToken = pathChanged ? nil : existingMetadata.lockToken
294295

295296
returningUpdatedMetadatas.append(updatedMetadata)
296297

@@ -651,6 +652,7 @@ public final class FilesDatabaseManager: Sendable {
651652
itemMetadata.fileName = newFileName
652653
itemMetadata.fileNameView = newFileName
653654
itemMetadata.serverUrl = newServerUrl
655+
itemMetadata.lockToken = nil
654656

655657
database.add(itemMetadata, update: .all)
656658

‎shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Extensions/NKError+Extensions.swift‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ extension NKError {
2828
errorCode == 404
2929
}
3030

31+
var isPreconditionFailedError: Bool {
32+
errorCode == 412
33+
}
34+
35+
var isLockedError: Bool {
36+
errorCode == 423
37+
}
38+
3139
var isNoChangesError: Bool {
3240
errorCode == NKError.noChangesErrorCode
3341
}
@@ -49,7 +57,6 @@ extension NKError {
4957
} else if isNotFoundError {
5058
NSFileProviderError(.noSuchItem)
5159
} else if isCouldntConnectError {
52-
// Provide something the file provider can do something with
5360
NSFileProviderError(.serverUnreachable)
5461
} else if isUnauthenticatedError || isUnauthorizedError {
5562
NSFileProviderError(.notAuthenticated)

‎shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,37 @@ public extension Item {
168168
"""
169169
)
170170

171+
if error.isPreconditionFailedError || error.isLockedError {
172+
logger.info("Clearing stale lock token after lock/precondition error.", [.item: itemIdentifier])
173+
metadata.lockToken = nil
174+
// Signal re-enumeration: if the parent was also renamed (causing the
175+
// precondition failure), the working set check will update the path
176+
// before the system retries.
177+
if let domain, let manager = NSFileProviderManager(for: domain) {
178+
Task {
179+
try? await manager.signalEnumerator(for: .workingSet)
180+
}
181+
}
182+
}
183+
184+
// Remote path gone — parent renamed on another client while the file was
185+
// open. Clear any stale lock token and signal the working set enumerator
186+
// so the system discovers the new path before retrying. Return
187+
// cannotSynchronize rather than noSuchItem: the file still exists on the
188+
// server at a different location.
189+
if error.isNotFoundError {
190+
metadata.lockToken = nil
191+
metadata.status = Status.uploadError.rawValue
192+
metadata.sessionError = error.errorDescription
193+
dbManager.addItemMetadata(metadata)
194+
if let domain, let manager = NSFileProviderManager(for: domain) {
195+
Task {
196+
try? await manager.signalEnumerator(for: .workingSet)
197+
}
198+
}
199+
return (nil, NSFileProviderError(.cannotSynchronize))
200+
}
201+
171202
metadata.status = Status.uploadError.rawValue
172203
metadata.sessionError = error.errorDescription
173204
dbManager.addItemMetadata(metadata)

‎shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,10 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable {
587587
/// exercising the locally-known-mtime fallback in the production code.
588588
public var uploadReturnsNilDate: Bool = false
589589

590+
/// When set, every upload call returns this error immediately without touching the mock tree.
591+
/// Use this to simulate server-side upload rejections (e.g. 404 path gone, 507 quota).
592+
public var uploadError: NKError?
593+
590594
/// Handler to track enumerate calls
591595
public var enumerateCallHandler: ((String, EnumerateDepth, Bool, [String], Data?, Account, NKRequestOptions, @escaping (URLSessionTask) -> Void) -> Void)?
592596

@@ -760,6 +764,10 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable {
760764
response: HTTPURLResponse?,
761765
remoteError: NKError
762766
) {
767+
if let uploadError {
768+
return (account.ncKitAccount, nil, nil, nil, 0, nil, uploadError)
769+
}
770+
763771
var itemName: String
764772
do {
765773
itemName = try name(from: remotePath)

‎shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,144 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase {
330330
XCTAssertEqual(remoteItem.data, originalRemoteData)
331331
}
332332

333+
/// When the server returns 404 during an upload (the parent folder was renamed
334+
/// on another client while the file was open), the extension must:
335+
/// - clear the stale lock token so the next attempt goes without an If: header
336+
/// - return cannotSynchronize, not noSuchItem — the file still exists at a new path
337+
///
338+
/// Regression test for the race condition described in nextcloud/desktop#9987.
339+
func testModifyWith404ClearsLockTokenAndReturnsCannotSynchronize() async throws {
340+
let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem)
341+
remoteInterface.uploadError = NKError(statusCode: 404, fallbackDescription: "Not Found")
342+
343+
var itemMetadata = remoteItem.toItemMetadata(account: Self.account)
344+
itemMetadata.lockToken = "opaquelocktoken:stale-token-from-old-path"
345+
itemMetadata.uploaded = true
346+
itemMetadata.downloaded = true
347+
Self.dbManager.addItemMetadata(itemMetadata)
348+
349+
let newContentsUrl = FileManager.default.temporaryDirectory
350+
.appendingPathComponent("modify-404-test")
351+
try "Updated content".write(to: newContentsUrl, atomically: true, encoding: .utf8)
352+
353+
let item = Item(
354+
metadata: itemMetadata,
355+
parentItemIdentifier: .rootContainer,
356+
account: Self.account,
357+
remoteInterface: remoteInterface,
358+
dbManager: Self.dbManager
359+
)
360+
let targetItem = Item(
361+
metadata: itemMetadata,
362+
parentItemIdentifier: .rootContainer,
363+
account: Self.account,
364+
remoteInterface: remoteInterface,
365+
dbManager: Self.dbManager
366+
)
367+
368+
let (modifiedItem, error) = await item.modify(
369+
itemTarget: targetItem,
370+
changedFields: [.contents, .contentModificationDate],
371+
contents: newContentsUrl,
372+
dbManager: Self.dbManager
373+
)
374+
375+
XCTAssertNil(modifiedItem)
376+
XCTAssertEqual(
377+
(error as? NSFileProviderError)?.code, .cannotSynchronize,
378+
"404 during modify must surface as cannotSynchronize, not noSuchItem"
379+
)
380+
let updatedMetadata = Self.dbManager.itemMetadata(ocId: itemMetadata.ocId)
381+
XCTAssertNil(
382+
updatedMetadata?.lockToken,
383+
"Lock token must be cleared when the upload path is gone"
384+
)
385+
}
386+
387+
func testModifyWith412ClearsLockToken() async throws {
388+
let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem)
389+
remoteInterface.uploadError = NKError(statusCode: 412, fallbackDescription: "Precondition Failed")
390+
391+
var itemMetadata = remoteItem.toItemMetadata(account: Self.account)
392+
itemMetadata.lockToken = "opaquelocktoken:stale-token"
393+
itemMetadata.uploaded = true
394+
itemMetadata.downloaded = true
395+
Self.dbManager.addItemMetadata(itemMetadata)
396+
397+
let newContentsUrl = FileManager.default.temporaryDirectory
398+
.appendingPathComponent("modify-412-test")
399+
try "Updated content".write(to: newContentsUrl, atomically: true, encoding: .utf8)
400+
401+
let item = Item(
402+
metadata: itemMetadata,
403+
parentItemIdentifier: .rootContainer,
404+
account: Self.account,
405+
remoteInterface: remoteInterface,
406+
dbManager: Self.dbManager
407+
)
408+
let targetItem = Item(
409+
metadata: itemMetadata,
410+
parentItemIdentifier: .rootContainer,
411+
account: Self.account,
412+
remoteInterface: remoteInterface,
413+
dbManager: Self.dbManager
414+
)
415+
416+
let (modifiedItem, error) = await item.modify(
417+
itemTarget: targetItem,
418+
changedFields: [.contents, .contentModificationDate],
419+
contents: newContentsUrl,
420+
dbManager: Self.dbManager
421+
)
422+
423+
XCTAssertNil(modifiedItem)
424+
XCTAssertEqual((error as? NSFileProviderError)?.code, .cannotSynchronize)
425+
let updatedMetadata = Self.dbManager.itemMetadata(ocId: itemMetadata.ocId)
426+
XCTAssertNil(updatedMetadata?.lockToken, "Stale lock token must be cleared on 412.")
427+
}
428+
429+
func testModifyWith423ClearsLockToken() async throws {
430+
let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem)
431+
remoteInterface.uploadError = NKError(statusCode: 423, fallbackDescription: "Locked")
432+
433+
var itemMetadata = remoteItem.toItemMetadata(account: Self.account)
434+
itemMetadata.lockToken = "opaquelocktoken:stale-token"
435+
itemMetadata.uploaded = true
436+
itemMetadata.downloaded = true
437+
Self.dbManager.addItemMetadata(itemMetadata)
438+
439+
let newContentsUrl = FileManager.default.temporaryDirectory
440+
.appendingPathComponent("modify-423-test")
441+
try "Updated content".write(to: newContentsUrl, atomically: true, encoding: .utf8)
442+
443+
let item = Item(
444+
metadata: itemMetadata,
445+
parentItemIdentifier: .rootContainer,
446+
account: Self.account,
447+
remoteInterface: remoteInterface,
448+
dbManager: Self.dbManager
449+
)
450+
let targetItem = Item(
451+
metadata: itemMetadata,
452+
parentItemIdentifier: .rootContainer,
453+
account: Self.account,
454+
remoteInterface: remoteInterface,
455+
dbManager: Self.dbManager
456+
)
457+
458+
let (modifiedItem, error) = await item.modify(
459+
itemTarget: targetItem,
460+
changedFields: [.contents, .contentModificationDate],
461+
contents: newContentsUrl,
462+
dbManager: Self.dbManager
463+
)
464+
465+
XCTAssertNil(modifiedItem)
466+
XCTAssertEqual((error as? NSFileProviderError)?.code, .cannotSynchronize)
467+
let updatedMetadata = Self.dbManager.itemMetadata(ocId: itemMetadata.ocId)
468+
XCTAssertNil(updatedMetadata?.lockToken, "Stale lock token must be cleared on 423.")
469+
}
470+
333471
func testModifyFolder() async throws {
334472
let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem)
335473

0 commit comments

Comments
 (0)