From 87b5c70ff1a2fe81395817c01736156f20b891db Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Thu, 20 Aug 2020 16:54:09 -0700 Subject: [PATCH] Revert "Create a file with permissions that respects umask" --- Sources/Foundation/NSData.swift | 13 ++---- Sources/Foundation/NSPathUtilities.swift | 5 +-- Tests/Foundation/Tests/TestNSData.swift | 57 ------------------------ 3 files changed, 5 insertions(+), 70 deletions(-) diff --git a/Sources/Foundation/NSData.swift b/Sources/Foundation/NSData.swift index 4fd966176f..63c80619c8 100644 --- a/Sources/Foundation/NSData.swift +++ b/Sources/Foundation/NSData.swift @@ -434,7 +434,8 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding { } let fm = FileManager.default - let permissions = try? fm._permissionsOfItem(atPath: path) + // 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 if writeOptionsMask.contains(.atomic) { let (newFD, auxFilePath) = try _NSCreateTemporaryFile(path) @@ -445,9 +446,7 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding { // requires that there be no open handles to the file fh.closeFile() try _NSCleanupTemporaryFile(auxFilePath, path) - if let permissions = permissions { - try fm.setAttributes([.posixPermissions: NSNumber(value: permissions)], ofItemAtPath: path) - } + try fm.setAttributes([.posixPermissions: NSNumber(value: permissions)], ofItemAtPath: path) } catch { let savedErrno = errno try? fm.removeItem(atPath: auxFilePath) @@ -458,15 +457,11 @@ 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: createMode) else { + guard let fh = FileHandle(path: path, flags: flags, createMode: permissions) 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 25fb7a57a3..b0021d0c00 100644 --- a/Sources/Foundation/NSPathUtilities.swift +++ b/Sources/Foundation/NSPathUtilities.swift @@ -760,10 +760,7 @@ 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) - 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) + let fd = mkstemp(&buf) 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 813b578ba6..e166953366 100644 --- a/Tests/Foundation/Tests/TestNSData.swift +++ b/Tests/Foundation/Tests/TestNSData.swift @@ -7,16 +7,6 @@ // 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 { @@ -233,8 +223,6 @@ 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), @@ -563,51 +551,6 @@ 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 = "<>"