Improve temporary file location for atomic writes (#1477)

* (156721664) Temporary files for atomic writes should use _amkrtemp instead of mktemp when applicable

* Fix build failure

Author: jmschonfeld

Sources/FoundationEssentials/Data/Data+Writing.swift

@@ -157,9 +157,37 @@ private func createTemporaryFile(at destinationPath: String, inPath: PathOrURL,
 
     let pidString = String(ProcessInfo.processInfo.processIdentifier, radix: 16, uppercase: true)
     let template = directoryPath + prefix + pidString + ".XXXXXX"
-    var count = 0
     let maxCount = 7
-    repeat {
+    for _ in 0 ..< maxCount {
+#if FOUNDATION_FRAMEWORK
+        let (sandboxResult, amkrErrno) = inPath.withFileSystemRepresentation { inPathFileSystemRep -> ((Int32, String)?, Int32?) in
+            guard let inPathFileSystemRep else {
+                return (nil, nil)
+            }
+            // First, try _amkrtemp to carry over any sandbox extensions for inPath to the temporary file (even if the application isn't sandboxed)
+            guard let uniqueTempFile = _amkrtemp(inPathFileSystemRep) else {
+                return (nil, errno)
+            }
+            defer { free(uniqueTempFile) }
+            let fd = openFileDescriptorProtected(path: uniqueTempFile, flags: O_CREAT | O_EXCL | O_RDWR, options: options)
+            if fd >= 0 {
+                // Got a good fd
+                return ((fd, String(cString: uniqueTempFile)), nil)
+            }
+            return (nil, errno)
+        }
+        
+        // If _amkrtemp succeeded, return its result
+        if let sandboxResult {
+            return sandboxResult
+        }
+        // If _amkrtemp failed with EEXIST, just retry
+        if amkrErrno == EEXIST {
+            continue
+        }
+        // Otherwise, fall through to mktemp below
+#endif
+        
         let result = try template.withMutableFileSystemRepresentation { templateFileSystemRep -> (Int32, String)? in
             guard let templateFileSystemRep else {
                 throw CocoaError(.fileWriteInvalidFileName)
@@ -187,7 +215,12 @@ private func createTemporaryFile(at destinationPath: String, inPath: PathOrURL,
 
             // If the file exists, we repeat. Otherwise throw the error.
             if errno != EEXIST {
-                throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false, variant: variant)
+                #if FOUNDATION_FRAMEWORK
+                let debugDescription = "Creating a temporary file via mktemp failed. Creating the temporary file via _amkrtemp previously also failed with errno \(amkrErrno)"
+                #else
+                let debugDescription: String? = nil
+                #endif
+                throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false, variant: variant, debugDescription: debugDescription)
             }
 
             // Try again
@@ -196,14 +229,10 @@ private func createTemporaryFile(at destinationPath: String, inPath: PathOrURL,
 
         if let result {
             return result
-        } else {
-            count += 1
-            if count > maxCount {
-                // Prevent an infinite loop; even if the error is obscure
-                throw CocoaError(.fileWriteUnknown)
-            }
         }
-    } while true
+    }
+    // We hit max count, prevent an infinite loop; even if the error is obscure
+    throw CocoaError(.fileWriteUnknown)
 #endif // os(WASI)
 }
 

Sources/FoundationEssentials/Error/CocoaError+FilePath.swift

@@ -31,12 +31,12 @@ import WinSDK
 // MARK: - Error Creation with CocoaError.Code
 
 extension CocoaError {
-    static func errorWithFilePath(_ code: CocoaError.Code, _ path: String, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError {
-        CocoaError(code, path: path, variant: variant, source: source, destination: destination)
+    static func errorWithFilePath(_ code: CocoaError.Code, _ path: String, variant: String? = nil, source: String? = nil, destination: String? = nil, debugDescription: String? = nil) -> CocoaError {
+        CocoaError(code, path: path, variant: variant, source: source, destination: destination, debugDescription: debugDescription)
     }
 
-    static func errorWithFilePath(_ code: CocoaError.Code, _ url: URL, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError {
-        CocoaError(code, url: url, variant: variant, source: source, destination: destination)
+    static func errorWithFilePath(_ code: CocoaError.Code, _ url: URL, variant: String? = nil, source: String? = nil, destination: String? = nil, debugDescription: String? = nil) -> CocoaError {
+        CocoaError(code, url: url, variant: variant, source: source, destination: destination, debugDescription: debugDescription)
     }
 }
 
@@ -81,21 +81,21 @@ extension POSIXError {
 }
 
 extension CocoaError {
-    static func errorWithFilePath(_ pathOrURL: PathOrURL, errno: Int32, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError {
+    static func errorWithFilePath(_ pathOrURL: PathOrURL, errno: Int32, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil, debugDescription: String? = nil) -> CocoaError {
         switch pathOrURL {
         case .path(let path):
-            return Self.errorWithFilePath(path, errno: errno, reading: reading, variant: variant, source: source, destination: destination)
+            return Self.errorWithFilePath(path, errno: errno, reading: reading, variant: variant, source: source, destination: destination, debugDescription: debugDescription)
         case .url(let url):
-            return Self.errorWithFilePath(url, errno: errno, reading: reading, variant: variant, source: source, destination: destination)
+            return Self.errorWithFilePath(url, errno: errno, reading: reading, variant: variant, source: source, destination: destination, debugDescription: debugDescription)
         }
     }
 
-    static func errorWithFilePath(_ path: String, errno: Int32, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError {
-        CocoaError(Code(fileErrno: errno, reading: reading), path: path, underlying: POSIXError(errno: errno), variant: variant, source: source, destination: destination)
+    static func errorWithFilePath(_ path: String, errno: Int32, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil, debugDescription: String? = nil) -> CocoaError {
+        CocoaError(Code(fileErrno: errno, reading: reading), path: path, underlying: POSIXError(errno: errno), variant: variant, source: source, destination: destination, debugDescription: debugDescription)
     }
 
-    static func errorWithFilePath(_ url: URL, errno: Int32, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError {
-        CocoaError(Code(fileErrno: errno, reading: reading), url: url, underlying: POSIXError(errno: errno), variant: variant, source: source, destination: destination)
+    static func errorWithFilePath(_ url: URL, errno: Int32, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil, debugDescription: String? = nil) -> CocoaError {
+        CocoaError(Code(fileErrno: errno, reading: reading), url: url, underlying: POSIXError(errno: errno), variant: variant, source: source, destination: destination, debugDescription: debugDescription)
     }
 }
 
@@ -144,18 +144,18 @@ extension CocoaError.Code {
 }
 
 extension CocoaError {
-    static func errorWithFilePath(_ path: PathOrURL, win32 dwError: DWORD, reading: Bool) -> CocoaError {
+    static func errorWithFilePath(_ path: PathOrURL, win32 dwError: DWORD, reading: Bool, debugDescription: String? = nil) -> CocoaError {
         switch path {
         case let .path(path):
-            return CocoaError(.init(win32: dwError, reading: reading, emptyPath: path.isEmpty), path: path, underlying: Win32Error(dwError))
+            return CocoaError(.init(win32: dwError, reading: reading, emptyPath: path.isEmpty), path: path, underlying: Win32Error(dwError), debugDescription: debugDescription)
         case let .url(url):
             let pathStr = url.withUnsafeFileSystemRepresentation { String(cString: $0!) }
-            return CocoaError(.init(win32: dwError, reading: reading, emptyPath: pathStr.isEmpty), path: pathStr, url: url, underlying: Win32Error(dwError))
+            return CocoaError(.init(win32: dwError, reading: reading, emptyPath: pathStr.isEmpty), path: pathStr, url: url, underlying: Win32Error(dwError), debugDescription: debugDescription)
         }
     }
 
-    static func errorWithFilePath(_ path: String? = nil, win32 dwError: DWORD, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil) -> CocoaError {
-        return CocoaError(.init(win32: dwError, reading: reading, emptyPath: path?.isEmpty), path: path, underlying: Win32Error(dwError), variant: variant, source: source, destination: destination)
+    static func errorWithFilePath(_ path: String? = nil, win32 dwError: DWORD, reading: Bool, variant: String? = nil, source: String? = nil, destination: String? = nil, debugDescription: String? = nil) -> CocoaError {
+        return CocoaError(.init(win32: dwError, reading: reading, emptyPath: path?.isEmpty), path: path, underlying: Win32Error(dwError), variant: variant, source: source, destination: destination, debugDescription: debugDescription)
     }
 }
 #endif
@@ -190,7 +190,8 @@ extension CocoaError {
         underlying: (some Error)? = Optional<CocoaError>.none,
         variant: String? = nil,
         source: String? = nil,
-        destination: String? = nil
+        destination: String? = nil,
+        debugDescription: String? = nil
     ) {
         self.init(
             code,
@@ -199,7 +200,8 @@ extension CocoaError {
             underlying: underlying,
             variant: variant,
             source: source,
-            destination: destination
+            destination: destination,
+            debugDescription: debugDescription
         )
     }
 
@@ -209,7 +211,8 @@ extension CocoaError {
         underlying: (some Error)? = Optional<CocoaError>.none,
         variant: String? = nil,
         source: String? = nil,
-        destination: String? = nil
+        destination: String? = nil,
+        debugDescription: String? = nil
     ) {
         self.init(
             code,
@@ -218,7 +221,8 @@ extension CocoaError {
             underlying: underlying,
             variant: variant,
             source: source,
-            destination: destination
+            destination: destination,
+            debugDescription: debugDescription
         )
     }
 
@@ -229,10 +233,11 @@ extension CocoaError {
         underlying: (some Error)? = Optional<CocoaError>.none,
         variant: String? = nil,
         source: String? = nil,
-        destination: String? = nil
+        destination: String? = nil,
+        debugDescription: String? = nil
     ) {
         #if FOUNDATION_FRAMEWORK
-        self.init(_uncheckedNSError: NSError._cocoaError(withCode: code.rawValue, path: path, url: url, underlying: underlying, variant: variant, source: source, destination: destination) as NSError)
+        self.init(_uncheckedNSError: NSError._cocoaError(withCode: code.rawValue, path: path, url: url, underlying: underlying, variant: variant, source: source, destination: destination, debugDescription: debugDescription) as NSError)
         #else
         var userInfo: [String : Any] = [:]
         if let path {
@@ -253,6 +258,9 @@ extension CocoaError {
         if let variant {
             userInfo[NSUserStringVariantErrorKey] = [variant]
         }
+        if let debugDescription {
+            userInfo[NSDebugDescriptionErrorKey] = debugDescription
+        }
 
         self.init(code, userInfo: userInfo)
         #endif