Skip to content

Create a file with permissions that respects umask #2851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions Sources/Foundation/NSData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion Sources/Foundation/NSPathUtilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
57 changes: 57 additions & 0 deletions Tests/Foundation/Tests/TestNSData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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 = "<>"

Expand Down