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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,31 @@ public extension Item {
"""
)

if let expectedSize = itemTemplate.documentSize??.int64Value, size != expectedSize {
logger.info(
"""
Created item upload reported as successful, but there are differences between
the received file size (\(Int(size ?? -1)))
and the original file size (\(itemTemplate.documentSize??.int64Value ?? 0))
"""
)
// Integrity check: the size the server reports it stored must match the bytes we
// uploaded. On a mismatch the transfer was torn/truncated (a dropped connection, or an app
// such as Adobe InDesign/Illustrator still writing the file). Refuse to record a clean
// creation: delete the partial remote object (best effort) so a retry starts clean, then
// return a *transient* error so the File Provider system re-drives the create instead of
// surfacing a broken file as synced.
//
// The error must be transient to get an automatic retry: per NSFileProviderReplicatedExtension,
// resolvable NSFileProviderError codes such as `.cannotSynchronize` make the system back off
// until the provider signals resolution, whereas "any other error … in NSCocoaErrorDomain" is
// retried. NOTE: a create retry may arrive with `.mayAlreadyExist`, which `create()` currently
// short-circuits — tracked as a separate follow-up.
let localAttributes = try? FileManager.default.attributesOfItem(atPath: localPath)

if let localSize = localAttributes?[.size] as? Int64, let uploadedSize = size, uploadedSize != localSize {
logger.error("Upload integrity check failed for created item: server stored \(uploadedSize) bytes but the local file is \(localSize) bytes. Removing the partial remote object and returning a transient error so the system retries.", [.name: itemTemplate.filename])
let (_, _, deleteError) = await remoteInterface.delete(remotePath: remotePath, account: account, options: .init(), taskHandler: { _ in })

if deleteError != .success {
logger.error("Could not remove partial remote object after integrity failure.", [.name: itemTemplate.filename, .error: deleteError])
}

return (nil, NSError(domain: NSCocoaErrorDomain, code: NSFileWriteUnknownError, userInfo: [
NSLocalizedDescriptionKey: "Upload integrity check failed: server stored \(uploadedSize) of \(localSize) bytes."
]))
}

let contentType: String = if itemTemplate.contentType == .aliasFile {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,15 +229,25 @@ public extension Item {
"""
)

// Integrity check: the size the server reports it stored must match the bytes we handed it.
// A mismatch means a torn/truncated transfer (a dropped connection, or an app such as Adobe
// InDesign/Illustrator still writing the file). Do NOT record it as a clean upload — return a
// *transient* error so the File Provider system re-drives the modification instead of
// committing a broken file. See F1 in the Adobe compatibility diagnosis.
//
// The error must be transient to get an automatic retry: per NSFileProviderReplicatedExtension,
// the resolvable NSFileProviderError codes (`.cannotSynchronize`, `.notAuthenticated`,
// `.excludedFromSync`, …) make the system back off until the provider calls
// `signalErrorResolved(_:)`. "Any other error … in NSCocoaErrorDomain" is treated as transient
// and retried. We leave the row in its `.uploading` state so the retry settles it to `.normal`.
let contentAttributes = try? FileManager.default.attributesOfItem(atPath: newContents.path)
if let expectedSize = contentAttributes?[.size] as? Int64, size != expectedSize {
logger.info(
"""
Item content modification upload reported as successful,
but there are differences between the received file size (\(size ?? -1))
and the original file size (\(documentSize?.int64Value ?? 0))
"""
)

if let localSize = contentAttributes?[.size] as? Int64, let uploadedSize = size, uploadedSize != localSize {
logger.error("Upload integrity check failed for item: server stored \(uploadedSize) bytes but the local file is \(localSize) bytes. Returning a transient error so the system retries.", [.name: filename, .item: ocId])

return (nil, NSError(domain: NSCocoaErrorDomain, code: NSFileWriteUnknownError, userInfo: [
NSLocalizedDescriptionKey: "Upload integrity check failed: server stored \(uploadedSize) of \(localSize) bytes."
]))
}

var newMetadata =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,14 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable {
/// Use this to simulate server-side upload rejections (e.g. 404 path gone, 507 quota).
public var uploadError: NKError?

/// When non-nil, the upload mock reports this as the stored size in its response, regardless
/// of the bytes actually written to the mock tree. Use this to simulate a torn/truncated
/// transfer (the server stores fewer/more bytes than the client sent) so the upload integrity
/// check in the production code can be exercised. The chunked upload path delegates to this
/// same `upload(...)`, so the override applies to both the single-shot and chunked paths.
/// `nil` echoes the real stored size.
public var uploadResponseSizeOverride: Int64?

/// Handler to track enumerate calls
public var enumerateCallHandler: ((String, EnumerateDepth, Bool, [String], Data?, Account, NKRequestOptions, @escaping (URLSessionTask) -> Void) -> Void)?

Expand Down Expand Up @@ -832,7 +840,7 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable {
item.identifier,
item.versionIdentifier,
responseDate,
item.size,
uploadResponseSizeOverride ?? item.size,
nil,
.success
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,12 @@ final class MockRemoteInterfaceTests: XCTestCase {
XCTAssertEqual(targetRootFile?.ocId, expectedRoot?.identifier)
XCTAssertEqual(targetRootFile?.fileName, NextcloudKit.shared.nkCommonInstance.rootFileName) // NextcloudKit gives the root dir this name
XCTAssertEqual(targetRootFile?.serverUrl, "https://mock.nc.com/remote.php/dav/files/testUserId") // NextcloudKit gives the root dir this url
XCTAssertEqual(targetRootFile?.date, expectedRoot?.creationDate)
// `toNKFile()` maps the item's `modificationDate` onto `NKFile.date`, so compare against
// that — not `creationDate`. `MockRemoteItem.init` defaults `creationDate` and
// `modificationDate` to two separate `Date()` calls, which are usually (but not always)
// identical; comparing `date` to `creationDate` therefore passed only when the clock did
// not tick between those two calls, and flaked on CI when it did.
XCTAssertEqual(targetRootFile?.date, expectedRoot?.modificationDate)
XCTAssertEqual(targetRootFile?.etag, expectedRoot?.versionIdentifier)

let resultChildren = await remoteInterface.enumerate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,53 @@ final class ItemCreateTests: NextcloudFileProviderKitTestCase {
XCTAssertTrue(createdItem.isUploaded)
}

/// Upload integrity guard (F1): when the server reports it stored a different number of bytes
/// than the local file contains, `create` must NOT record the item as a clean upload. It
/// returns a *transient* error (so the File Provider system automatically retries the create)
/// and best-effort removes the partial remote object, instead of surfacing a torn file as synced.
func testCreateFileFailsOnUploadSizeMismatch() async throws {
let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem)
var fileItemMetadata = SendableItemMetadata(
ocId: "file-id", fileName: "file", account: Self.account
)
fileItemMetadata.classFile = NKTypeClassFile.document.rawValue

let tempUrl = FileManager.default.temporaryDirectory
.appendingPathComponent("integrity-mismatch-create")
try Data("Hello world".utf8).write(to: tempUrl)

// Simulate the server storing fewer bytes than we sent (a torn transfer).
remoteInterface.uploadResponseSizeOverride = 3

let fileItemTemplate = Item(
metadata: fileItemMetadata,
parentItemIdentifier: .rootContainer,
account: Self.account,
remoteInterface: remoteInterface,
dbManager: Self.dbManager
)
let (createdItemMaybe, error) = await Item.create(
basedOn: fileItemTemplate,
contents: tempUrl,
account: Self.account,
remoteInterface: remoteInterface,
progress: Progress(),
dbManager: Self.dbManager,
log: FileProviderLogMock()
)

XCTAssertNil(createdItemMaybe)

// The error must be *transient* (NSCocoaErrorDomain) so the system automatically retries
// the create rather than backing off until the provider signals resolution.
let nsError = try XCTUnwrap(error as NSError?)
XCTAssertEqual(nsError.domain, NSCocoaErrorDomain)
XCTAssertEqual(nsError.code, NSFileWriteUnknownError)

// Nothing must be recorded as a clean upload for this item.
XCTAssertNil(Self.dbManager.itemMetadata(ocId: "file-id"))
}

func testCreateFile() async throws {
let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem)
var fileItemMetadata = SendableItemMetadata(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,66 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase {
XCTAssertEqual(remoteItem.data, originalRemoteData)
}

/// Upload integrity guard (F1): when the server reports it stored a different number of bytes
/// than the local file contains, the modify must NOT record the item as a clean upload. It
/// returns a *transient* error (so the File Provider system automatically retries the modify)
/// and leaves the row un-uploaded, instead of committing a truncated/torn file.
func testModifyFileFailsOnUploadSizeMismatch() async throws {
let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem)

var itemMetadata = remoteItem.toItemMetadata(account: Self.account)
itemMetadata.uploaded = true
itemMetadata.downloaded = true
Self.dbManager.addItemMetadata(itemMetadata)

let newContents = "Hello, New World!".data(using: .utf8)!
let newContentsUrl = FileManager.default.temporaryDirectory
.appendingPathComponent("integrity-mismatch-modify")
try newContents.write(to: newContentsUrl)

// Simulate the server storing fewer bytes than we sent (a torn transfer).
remoteInterface.uploadResponseSizeOverride = Int64(newContents.count - 1)

var targetItemMetadata = SendableItemMetadata(value: itemMetadata)
targetItemMetadata.size = Int64(newContents.count)

let item = Item(
metadata: itemMetadata,
parentItemIdentifier: .rootContainer,
account: Self.account,
remoteInterface: remoteInterface,
dbManager: Self.dbManager
)
let targetItem = Item(
metadata: targetItemMetadata,
parentItemIdentifier: .rootContainer,
account: Self.account,
remoteInterface: remoteInterface,
dbManager: Self.dbManager
)

let (modifiedItem, error) = await item.modify(
itemTarget: targetItem,
changedFields: [.contents],
contents: newContentsUrl,
dbManager: Self.dbManager
)

XCTAssertNil(modifiedItem)

// The error must be *transient* (NSCocoaErrorDomain, outside the resolvable
// NSFileProviderError set) so the system automatically retries the modify rather than
// backing off until the provider signals resolution.
let nsError = try XCTUnwrap(error as NSError?)
XCTAssertEqual(nsError.domain, NSCocoaErrorDomain)
XCTAssertEqual(nsError.code, NSFileWriteUnknownError)

// The item must not be recorded as a clean upload; it stays pending for the retry.
let dbItem = try XCTUnwrap(Self.dbManager.itemMetadata(ocId: itemMetadata.ocId))
XCTAssertFalse(dbItem.uploaded)
XCTAssertNotEqual(dbItem.status, Status.normal.rawValue)
}

/// When the server returns 404 during an upload (the parent folder was renamed
/// on another client while the file was open), the extension must:
/// - clear the stale lock token so the next attempt goes without an If: header
Expand Down
Loading