FileManager "isDeletableFile" returns true when no file exists at path

[carlynorama]

Here's a little sample program, but when I try to delete this file I would have expected it to throw on isDeletableFile because no file by that name exists.

swift-driver version: 1.120.5 Apple Swift version 6.1 (swiftlang-6.1.0.110.21 clang-1700.0.13.3)
Target: arm64-apple-macosx15.0

enum TestingErrors:Error {
    case fileNotDeletable
}

let arguments = CommandLine.arguments

print("you just ran \(arguments[0]) with the following commands: \(arguments.dropFirst())")

let instruction = arguments.dropFirst().first
let filename = arguments.dropFirst(2).first

try delete(path:"junkFileName.txt")


public func delete(path: String) throws {
    guard FileManager.default.isDeletableFile(atPath: path) else {
        throw TestingErrors.fileNotDeletable
    }
    
    print("MESSAGE:", "says it's deletable: \(path))")
    
    try FileManager.default.removeItem(atPath: path)
}

Actual error message:

you just ran .build/arm64-apple-macosx/debug/wrangle with the following commands: []
MESSAGE: says it's deletable: junkFileName.txt)
Swift/ErrorType.swift:253: Fatal error: Error raised at top level: Error Domain=NSCocoaErrorDomain Code=4 "“junkFileName.txt” couldn’t be removed." UserInfo={NSUserStringVariant=(
    Remove
), NSFilePath=junkFileName.txt, NSURL=junkFileName.txt -- file:///$MYUSERINFO/FileWrangler/, NSUnderlyingError=0x15b60f8f0 {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"}}
zsh: trace trap  swift run

[jmschonfeld]

Indeed it looks like the behavior of this function does not necessarily check that the file exists, but rather just that the process has permission to delete a file (if it exists) at that path. It looks like (whether it is correct or not) this has been the behavior of this function for some time now. It seems plausible that maybe it should be changed to check if the file exists, but I'd also advise caution in this area to not rely on the result of a call to this followed by a call to actually deleting the file to always succeed because that would introduce a TOCTOU issue into your program (and applications that really want to delete the file should just call removeItem(atPath:) instead of checking isDeletableFile first).

[carlynorama]

The documentation says

true if the current process has delete privileges for the file at path; otherwise false if the process does not have delete privileges or the existence of the file could not be determined.

Which now implies to me that some existence check happened, but it only returns false if the result was nil? I thought it meant it would return false on the existence check returning anything but true. Some explicit warning in the doc's that it will return true if file was positively confirmed to already not exist would be helpful if the behavior stays the same.

I currently have

    public func delete(_ id: FWFile) async throws {
        let path = id.locationPath
            //let url = id.locationURL
        
        //note returns true if file does not exist. 
        //appears to be essentially a permissions check.  
        guard fileManager.isDeletableFile(atPath: path) else {
            throw FileServiceError("FileID not associated with deletable file.")
        }
        
        do {
            try fileManager.removeItem(atPath: path)
        } catch let error as CocoaError where error.code == .fileNoSuchFile {
            throw FileServiceError.noFileAtLocation(path)
        } catch {
            throw FileServiceError.caughtError(error)
        }
    }

Is what I'm hearing is that the guard checking for deletability is essentially redundant? Am I essentially running the same code twice? (Once called by isDeletable, second by removeItem)

FWIW: .isReadableFile and .isWritableFile I believe return false if the file does not exist.

[jmschonfeld]

Yes the isDeletableFile(atPath:) check is redundant, and also not recommended. Any situation where isDeletableFile would return false will simply throw an error in removeItem(atPath:), so there's no need to check first. Additionally, this could lead to a "time of check, time of use" (TOCTOU) issue because it is possible for the file system contents to change between the call to isDeletableFile(atPath:) and removeItem(atPath:). Instead of leaving the door open for that change to adversely impact your application, I'd recommend removing the call to isDeletableFile(atPath:) and instead just handling the errors thrown by removeItem(atPath:) which would indicate why the file couldn't be deleted.