diff --git a/Sources/Foundation/NSData.swift b/Sources/Foundation/NSData.swift index 63c80619c8..4fd966176f 100644 --- a/Sources/Foundation/NSData.swift +++ b/Sources/Foundation/NSData.swift @@ -434,8 +434,7 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding { } let fm = FileManager.default - // The destination file path may not exist so provide a default file permissions of RW user only - let permissions = (try? fm._permissionsOfItem(atPath: path)) ?? 0o600 + let permissions = try? fm._permissionsOfItem(atPath: path) if writeOptionsMask.contains(.atomic) { let (newFD, auxFilePath) = try _NSCreateTemporaryFile(path) @@ -446,7 +445,9 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding { // requires that there be no open handles to the file fh.closeFile() try _NSCleanupTemporaryFile(auxFilePath, path) - try fm.setAttributes([.posixPermissions: NSNumber(value: permissions)], ofItemAtPath: path) + if let permissions = permissions { + try fm.setAttributes([.posixPermissions: NSNumber(value: permissions)], ofItemAtPath: path) + } } catch { let savedErrno = errno try? fm.removeItem(atPath: auxFilePath) @@ -457,11 +458,15 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding { if writeOptionsMask.contains(.withoutOverwriting) { flags |= O_EXCL } + let createMode = Int(S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) - guard let fh = FileHandle(path: path, flags: flags, createMode: permissions) else { + guard let fh = FileHandle(path: path, flags: flags, createMode: createMode) else { throw _NSErrorWithErrno(errno, reading: false, path: path) } try doWrite(fh) + if let permissions = permissions { + try fm.setAttributes([.posixPermissions: NSNumber(value: permissions)], ofItemAtPath: path) + } } } diff --git a/Sources/Foundation/NSPathUtilities.swift b/Sources/Foundation/NSPathUtilities.swift index b0021d0c00..25fb7a57a3 100644 --- a/Sources/Foundation/NSPathUtilities.swift +++ b/Sources/Foundation/NSPathUtilities.swift @@ -760,7 +760,10 @@ internal func _NSCreateTemporaryFile(_ filePath: String) throws -> (Int32, Strin let maxLength = Int(PATH_MAX) + 1 var buf = [Int8](repeating: 0, count: maxLength) let _ = template._nsObject.getFileSystemRepresentation(&buf, maxLength: maxLength) - let fd = mkstemp(&buf) + guard let name = mktemp(&buf) else { + throw _NSErrorWithErrno(errno, reading: false, path: filePath) + } + let fd = open(name, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) if fd == -1 { throw _NSErrorWithErrno(errno, reading: false, path: filePath) } diff --git a/Tests/Foundation/Tests/TestNSData.swift b/Tests/Foundation/Tests/TestNSData.swift index e166953366..813b578ba6 100644 --- a/Tests/Foundation/Tests/TestNSData.swift +++ b/Tests/Foundation/Tests/TestNSData.swift @@ -7,6 +7,16 @@ // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors // +import CoreFoundation + +#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT + #if canImport(SwiftFoundation) && !DEPLOYMENT_RUNTIME_OBJC + @testable import SwiftFoundation + #else + @testable import Foundation + #endif +#endif + class TestNSData: LoopbackServerTest { class AllOnesImmutableData : NSData { @@ -223,6 +233,8 @@ class TestNSData: LoopbackServerTest { ("test_limitDebugDescription", test_limitDebugDescription), ("test_edgeDebugDescription", test_edgeDebugDescription), ("test_writeToURLOptions", test_writeToURLOptions), + ("test_writeToURLPermissions", test_writeToURLPermissions), + ("test_writeToURLPermissionsWithAtomic", test_writeToURLPermissionsWithAtomic), ("test_edgeNoCopyDescription", test_edgeNoCopyDescription), ("test_initializeWithBase64EncodedDataGetsDecodedData", test_initializeWithBase64EncodedDataGetsDecodedData), ("test_initializeWithBase64EncodedDataWithNonBase64CharacterIsNil", test_initializeWithBase64EncodedDataWithNonBase64CharacterIsNil), @@ -551,6 +563,51 @@ class TestNSData: LoopbackServerTest { } } +#if !os(Windows) + // NOTE: `umask(3)` is process global. Therefore, the behavior is unknown if `withUmask(_:_:)` is used simultaniously. + private func withUmask(_ mode: mode_t, _ block: () -> Void) { + let original = umask(mode) + block() + umask(original) + } +#endif + + func test_writeToURLPermissions() { +#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT && !os(Windows) + withUmask(0) { + do { + let data = Data() + let url = URL(fileURLWithPath: NSTemporaryDirectory() + "meow") + try data.write(to: url) + let fileManager = FileManager.default + let permission = try fileManager._permissionsOfItem(atPath: url.path) + XCTAssertEqual(0o666, permission) + try! fileManager.removeItem(atPath: url.path) + } catch { + XCTFail() + } + } +#endif + } + + func test_writeToURLPermissionsWithAtomic() { +#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT && !os(Windows) + withUmask(0) { + do { + let data = Data() + let url = URL(fileURLWithPath: NSTemporaryDirectory() + "meow") + try data.write(to: url, options: .atomic) + let fileManager = FileManager.default + let permission = try fileManager._permissionsOfItem(atPath: url.path) + XCTAssertEqual(0o666, permission) + try! fileManager.removeItem(atPath: url.path) + } catch { + XCTFail() + } + } +#endif + } + func test_emptyDescription() { let expected = "<>"