Skip to content

Commit caddcd4

Browse files
committed
Create a file with permissions that respects umask
**Problem** `Data.write(to:)` is a only method in the Foundation that can create a regular file. However, it ignores `umask` and always set 0600 permission unlike macOS Foundation, which respects process `umask`. **Solution** 1. With `.atomic` write option It uses `mkstemp(3)` in `_NSCreateTemporaryFile`, which is always creating a file with 0600 permission, if the system follows the latest POSIX specification or the permission is undefined. On macOS Foundation, therefore `_NSCreateTemporaryFile` uses `mktemp(3)` and `open(2)` instead to respect `umask`. 2. Without `.atomic` write option It uses `0o600` even if it uses `open(2)` that respects `umask`. Simply gives `0o666` instead. This is a bug caused by previous commit in swiftlang#1876. Swift JIRA is https://bugs.swift.org/browse/SR-13307.
1 parent 70ac8ed commit caddcd4

File tree

3 files changed

+74
-6
lines changed

3 files changed

+74
-6
lines changed

Sources/Foundation/NSData.swift

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -434,8 +434,7 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding {
434434
}
435435

436436
let fm = FileManager.default
437-
// The destination file path may not exist so provide a default file permissions of RW user only
438-
let permissions = (try? fm._permissionsOfItem(atPath: path)) ?? 0o600
437+
let permissions = try? fm._permissionsOfItem(atPath: path)
439438

440439
if writeOptionsMask.contains(.atomic) {
441440
let (newFD, auxFilePath) = try _NSCreateTemporaryFile(path)
@@ -446,7 +445,9 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding {
446445
// requires that there be no open handles to the file
447446
fh.closeFile()
448447
try _NSCleanupTemporaryFile(auxFilePath, path)
449-
try fm.setAttributes([.posixPermissions: NSNumber(value: permissions)], ofItemAtPath: path)
448+
if let permissions = permissions {
449+
try fm.setAttributes([.posixPermissions: NSNumber(value: permissions)], ofItemAtPath: path)
450+
}
450451
} catch {
451452
let savedErrno = errno
452453
try? fm.removeItem(atPath: auxFilePath)
@@ -457,11 +458,18 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding {
457458
if writeOptionsMask.contains(.withoutOverwriting) {
458459
flags |= O_EXCL
459460
}
460-
461-
guard let fh = FileHandle(path: path, flags: flags, createMode: permissions) else {
461+
#if os(Windows)
462+
let createMode = Int(ucrt.S_IREAD | ucrt.S_IWRITE)
463+
#else
464+
let createMode = Int(S_IREAD | S_IWRITE | S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH)
465+
#endif
466+
guard let fh = FileHandle(path: path, flags: flags, createMode: createMode) else {
462467
throw _NSErrorWithErrno(errno, reading: false, path: path)
463468
}
464469
try doWrite(fh)
470+
if let permissions = permissions {
471+
try fm.setAttributes([.posixPermissions: NSNumber(value: permissions)], ofItemAtPath: path)
472+
}
465473
}
466474
}
467475

Sources/Foundation/NSPathUtilities.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,10 @@ internal func _NSCreateTemporaryFile(_ filePath: String) throws -> (Int32, Strin
760760
let maxLength = Int(PATH_MAX) + 1
761761
var buf = [Int8](repeating: 0, count: maxLength)
762762
let _ = template._nsObject.getFileSystemRepresentation(&buf, maxLength: maxLength)
763-
let fd = mkstemp(&buf)
763+
guard let name = mktemp(&buf) else {
764+
throw _NSErrorWithErrno(errno, reading: false, path: filePath)
765+
}
766+
let fd = open(name, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH)
764767
if fd == -1 {
765768
throw _NSErrorWithErrno(errno, reading: false, path: filePath)
766769
}

Tests/Foundation/Tests/TestNSData.swift

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,16 @@
77
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
88
//
99

10+
import CoreFoundation
11+
12+
#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT
13+
#if canImport(SwiftFoundation) && !DEPLOYMENT_RUNTIME_OBJC
14+
@testable import SwiftFoundation
15+
#else
16+
@testable import Foundation
17+
#endif
18+
#endif
19+
1020
class TestNSData: LoopbackServerTest {
1121

1222
class AllOnesImmutableData : NSData {
@@ -223,6 +233,8 @@ class TestNSData: LoopbackServerTest {
223233
("test_limitDebugDescription", test_limitDebugDescription),
224234
("test_edgeDebugDescription", test_edgeDebugDescription),
225235
("test_writeToURLOptions", test_writeToURLOptions),
236+
("test_writeToURLPermissions", test_writeToURLPermissions),
237+
("test_writeToURLPermissionsWithAtomic", test_writeToURLPermissionsWithAtomic),
226238
("test_edgeNoCopyDescription", test_edgeNoCopyDescription),
227239
("test_initializeWithBase64EncodedDataGetsDecodedData", test_initializeWithBase64EncodedDataGetsDecodedData),
228240
("test_initializeWithBase64EncodedDataWithNonBase64CharacterIsNil", test_initializeWithBase64EncodedDataWithNonBase64CharacterIsNil),
@@ -551,6 +563,51 @@ class TestNSData: LoopbackServerTest {
551563
}
552564
}
553565

566+
#if !os(Windows)
567+
// NOTE: `umask(3)` is process global. Therefore, the behavior is unknown if `withUmask(_:_:)` is used simultaniously.
568+
private func withUmask(_ mode: mode_t, _ block: () -> Void) {
569+
let original = umask(mode)
570+
block()
571+
umask(original)
572+
}
573+
#endif
574+
575+
func test_writeToURLPermissions() {
576+
#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT && !os(Windows)
577+
withUmask(0) {
578+
do {
579+
let data = Data()
580+
let url = URL(fileURLWithPath: NSTemporaryDirectory() + "meow")
581+
try data.write(to: url)
582+
let fileManager = FileManager.default
583+
let permission = try fileManager._permissionsOfItem(atPath: url.path)
584+
XCTAssertEqual(0o666, permission)
585+
try! fileManager.removeItem(atPath: url.path)
586+
} catch {
587+
XCTFail()
588+
}
589+
}
590+
#endif
591+
}
592+
593+
func test_writeToURLPermissionsWithAtomic() {
594+
#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT && !os(Windows)
595+
withUmask(0) {
596+
do {
597+
let data = Data()
598+
let url = URL(fileURLWithPath: NSTemporaryDirectory() + "meow")
599+
try data.write(to: url, options: .atomic)
600+
let fileManager = FileManager.default
601+
let permission = try fileManager._permissionsOfItem(atPath: url.path)
602+
XCTAssertEqual(0o666, permission)
603+
try! fileManager.removeItem(atPath: url.path)
604+
} catch {
605+
XCTFail()
606+
}
607+
}
608+
#endif
609+
}
610+
554611
func test_emptyDescription() {
555612
let expected = "<>"
556613

0 commit comments

Comments
 (0)