Refactor FilePathTemp, add tests for permission code.

Broke `FilePathTemp.swift` into separate files for POSIX and
Windows.

Added tests for permissions code on Windows.

Added leading underscores to some function names.

Made some other functions `fileprivate`.

rdar://125087707

Author: al45tair

Sources/System/ErrnoWindows.swift

@@ -13,7 +13,7 @@ import WinSDK
 
 extension Errno {
   public init(windowsError: DWORD) {
-    self.init(rawValue: mapWindowsErrorToErrno(windowsError))
+    self.init(rawValue: _mapWindowsErrorToErrno(windowsError))
   }
 }
 

Sources/System/FileOperations.swift

@@ -135,8 +135,6 @@ extension FileDescriptor {
       if let permissions = permissions {
         return system_open(path, oFlag, permissions.rawValue)
       }
-      precondition(!options.contains(.create),
-        "Create must be given permissions")
       return system_open(path, oFlag)
     }
     return descOrError.map { FileDescriptor(rawValue: $0) }
@@ -506,3 +504,47 @@ extension FileDescriptor {
     }
   }
 }
+
+extension FilePermissions {
+  /// The file creation permission mask (aka "umask").
+  ///
+  /// Permissions set in this mask will be cleared by functions that create
+  /// files or directories.  Note that this mask is process-wide, and that
+  /// *getting* it is not thread safe.
+  @_alwaysEmitIntoClient
+  public static var creationMask: FilePermissions {
+    get {
+      let oldMask = _umask(0o22)
+      _ = _umask(oldMask)
+      return FilePermissions(rawValue: oldMask)
+    }
+    set {
+      _ = _umask(newValue.rawValue)
+    }
+  }
+
+  /// Change the file creation permission mask, run some code, then
+  /// restore it to its original value.
+  ///
+  /// - Parameters:
+  ///   - permissions: The new permission mask.
+  ///
+  /// This is more efficient than reading `creationMask` and restoring it
+  /// afterwards, because of the way reading the creation mask works.
+  @_alwaysEmitIntoClient
+  public static func withCreationMask<R>(
+    _ permissions: FilePermissions,
+    body: () throws -> R
+  ) rethrows -> R {
+    let oldMask = _umask(permissions.rawValue)
+    defer {
+      _ = _umask(oldMask)
+    }
+    return try body()
+  }
+
+  @usableFromInline
+  internal static func _umask(_ mode: CModeT) -> CModeT {
+    return system_umask(mode)
+  }
+}

Sources/System/FilePath/FilePathTemp.swift

@@ -9,244 +9,43 @@
 
 // MARK: - API
 
-public func withTemporaryPath<R>(
+/// Create a temporary path for the duration of the closure.
+///
+/// - Parameters:
+///   - basename: The base name for the temporary path.
+///   - body: The closure to execute.
+///
+/// Creates a temporary directory with a name based on the given `basename`,
+/// executes `body`, passing in the path of the created directory, then
+/// deletes the directory and all of its contents before returning.
+public func withTemporaryFilePath<R>(
   basename: FilePath.Component,
   _ body: (FilePath) throws -> R
 ) throws -> R {
   let temporaryDir = try createUniqueTemporaryDirectory(basename: basename)
   defer {
-    try? recursiveRemove(at: temporaryDir)
+    try? _recursiveRemove(at: temporaryDir)
   }
 
   return try body(temporaryDir)
 }
 
 // MARK: - Internals
 
-#if os(Windows)
-import WinSDK
-
-fileprivate func getTemporaryDirectory() throws -> FilePath {
-  return try withUnsafeTemporaryAllocation(of: CInterop.PlatformChar.self,
-                                           capacity: Int(MAX_PATH) + 1) {
-    buffer in
-
-    guard GetTempPath2W(DWORD(buffer.count), buffer.baseAddress) != 0 else {
-      throw Errno(windowsError: GetLastError())
-    }
-
-    return FilePath(SystemString(platformString: buffer.baseAddress!))
-  }
-}
-
-fileprivate func forEachFile(
-  at path: FilePath,
-  _ body: (WIN32_FIND_DATAW) throws -> ()
-) rethrows {
-  let searchPath = path.appending("\\*")
-
-  try searchPath.withPlatformString { szPath in
-    var findData = WIN32_FIND_DATAW()
-    let hFind = FindFirstFileW(szPath, &findData)
-    if hFind == INVALID_HANDLE_VALUE {
-      throw Errno(windowsError: GetLastError())
-    }
-    defer {
-      FindClose(hFind)
-    }
-
-    repeat {
-      // Skip . and ..
-      if findData.cFileName.0 == 46
-           && (findData.cFileName.1 == 0
-                 || (findData.cFileName.1 == 46
-                       && findData.cFileName.2 == 0)) {
-        continue
-      }
-
-      try body(findData)
-    } while FindNextFileW(hFind, &findData)
-  }
-}
-
-fileprivate func recursiveRemove(at path: FilePath) throws {
-  // First, deal with subdirectories
-  try forEachFile(at: path) { findData in
-    if (findData.dwFileAttributes & DWORD(FILE_ATTRIBUTE_DIRECTORY)) != 0 {
-      let name = withUnsafeBytes(of: findData.cFileName) {
-        return SystemString(platformString: $0.assumingMemoryBound(
-                              to: CInterop.PlatformChar.self).baseAddress!)
-      }
-      let component = FilePath.Component(name)!
-      let subpath = path.appending(component)
-
-      try recursiveRemove(at: subpath)
-    }
-  }
-
-  // Now delete everything else
-  try forEachFile(at: path) { findData in
-    let name = withUnsafeBytes(of: findData.cFileName) {
-      return SystemString(platformString: $0.assumingMemoryBound(
-                            to: CInterop.PlatformChar.self).baseAddress!)
-    }
-    let component = FilePath.Component(name)!
-    let subpath = path.appending(component)
-
-    if (findData.dwFileAttributes & DWORD(FILE_ATTRIBUTE_DIRECTORY)) == 0 {
-      try subpath.withPlatformString {
-        if !DeleteFileW($0) {
-          throw Errno(windowsError: GetLastError())
-        }
-      }
-    }
-  }
-
-  // Finally, delete the parent
-  try path.withPlatformString {
-    if !RemoveDirectoryW($0) {
-      throw Errno(windowsError: GetLastError())
-    }
-  }
-}
-
-#else
-fileprivate func getTemporaryDirectory() throws -> FilePath {
-  #if SYSTEM_PACKAGE_DARWIN
-  var capacity = 1024
-  while true {
-    let path: FilePath? = withUnsafeTemporaryAllocation(
-      of: CInterop.PlatformChar.self,
-      capacity: capacity
-    ) { buffer in
-      let len = system_confstr(SYSTEM_CS_DARWIN_USER_TEMP_DIR,
-                               buffer.baseAddress!,
-                               buffer.count)
-      if len == 0 {
-        // Fall back to "/tmp" if we can't read the temp directory
-        return "/tmp"
-      }
-      // If it was truncated, increase capaciy and try again
-      if len > buffer.count {
-        capacity = len
-        return nil
-      }
-      return FilePath(SystemString(platformString: buffer.baseAddress!))
-    }
-    if let path = path {
-      return path
-    }
-  }
-  #else
-  return "/tmp"
-  #endif
-}
-
-fileprivate func recursiveRemove(at path: FilePath) throws {
-  let dirfd = try FileDescriptor.open(path, .readOnly, options: .directory)
-  defer {
-    try? dirfd.close()
-  }
-
-  let dot: (CInterop.PlatformChar, CInterop.PlatformChar) = (46, 0)
-  try withUnsafeBytes(of: dot) {
-    try recursiveRemove(
-      in: dirfd.rawValue,
-      path: $0.assumingMemoryBound(to: CInterop.PlatformChar.self).baseAddress!
-    )
-  }
-
-  try path.withPlatformString {
-    if system_rmdir($0) != 0 {
-      throw Errno.current
-    }
-  }
-}
-
-fileprivate func impl_opendirat(
-  _ dirfd: CInt,
-  _ path: UnsafePointer<CInterop.PlatformChar>
-) -> UnsafeMutablePointer<system_DIR>? {
-  let fd = system_openat(dirfd, path,
-                         FileDescriptor.AccessMode.readOnly.rawValue
-                           | FileDescriptor.OpenOptions.directory.rawValue)
-  if fd < 0 {
-    return nil
-  }
-  return system_fdopendir(fd)
-}
-
-fileprivate func forEachFile(
-  in dirfd: CInt, path: UnsafePointer<CInterop.PlatformChar>,
-  _ body: (system_dirent) throws -> ()
-) throws {
-  guard let dir = impl_opendirat(dirfd, path) else {
-    throw Errno.current
-  }
-  defer {
-    _ = system_closedir(dir)
-  }
-
-  while let dirent = system_readdir(dir) {
-    // Skip . and ..
-    if dirent.pointee.d_name.0 == 46
-         && (dirent.pointee.d_name.1 == 0
-               || (dirent.pointee.d_name.1 == 46
-                     && dirent.pointee.d_name.2 == 0)) {
-      continue
-    }
-
-    try body(dirent.pointee)
-  }
-}
-
-internal func recursiveRemove(
-  in dirfd: CInt,
-  path: UnsafePointer<CInterop.PlatformChar>
-) throws {
-  // First, deal with subdirectories
-  try forEachFile(in: dirfd, path: path) { dirent in
-    if dirent.d_type == SYSTEM_DT_DIR {
-      try withUnsafeBytes(of: dirent.d_name) {
-        try recursiveRemove(
-          in: dirfd,
-          path: $0.assumingMemoryBound(to: CInterop.PlatformChar.self)
-            .baseAddress!
-        )
-      }
-    }
-  }
-
-  // Now delete the contents of this directory
-  try forEachFile(in: dirfd, path: path) { dirent in
-    let flag: CInt
-
-    if dirent.d_type == SYSTEM_DT_DIR {
-      flag = SYSTEM_AT_REMOVE_DIR
-    } else {
-      flag = 0
-    }
-
-    let result = withUnsafeBytes(of: dirent.d_name) {
-      system_unlinkat(dirfd,
-                      $0.assumingMemoryBound(to: CInterop.PlatformChar.self)
-                        .baseAddress!,
-                      flag)
-    }
-
-    if result != 0 {
-      throw Errno.current
-    }
-  }
-}
-#endif
-
 fileprivate let base64 = Array<UInt8>(
   "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_".utf8
 )
 
-fileprivate func makeTempDirectory(at: FilePath) throws -> Bool {
-  return try at.withPlatformString {
+/// Create a directory that is only accessible to the current user.
+///
+/// - Parameters:
+///   - path: The path of the directory to create.
+/// - Returns: `true` if a new directory was created.
+///
+/// This function will throw if there is an error, except if the error
+/// is that the directory exists, in which case it returns `false`.
+fileprivate func makeLockedDownDirectory(at path: FilePath) throws -> Bool {
+  return try path.withPlatformString {
     if system_mkdir($0, 0o700) == 0 {
       return true
     }
@@ -259,6 +58,11 @@ fileprivate func makeTempDirectory(at: FilePath) throws -> Bool {
   }
 }
 
+/// Generate a random string of base64 filename safe characters.
+///
+/// - Parameters:
+///   - length: The number of characters in the returned string.
+/// - Returns: A random string of length `length`.
 fileprivate func createRandomString(length: Int) -> String {
   return String(
     decoding: (0..<length).map{
@@ -268,16 +72,25 @@ fileprivate func createRandomString(length: Int) -> String {
   )
 }
 
-internal func createUniqueTemporaryDirectory(
+/// Given a base name, create a uniquely named temporary directory.
+///
+/// - Parameters:
+///   - basename: The base name for the new directory.
+/// - Returns: The path to the new directory.
+///
+/// Creates a directory in the system temporary directory whose name
+/// starts with `basename`, followed by a `.` and then a random
+/// string of characters.
+fileprivate func createUniqueTemporaryDirectory(
   basename: FilePath.Component
 ) throws -> FilePath {
-  var tempDir = try getTemporaryDirectory()
+  var tempDir = try _getTemporaryDirectory()
   tempDir.append(basename)
 
   while true {
     tempDir.extension = createRandomString(length: 16)
 
-    if try makeTempDirectory(at: tempDir) {
+    if try makeLockedDownDirectory(at: tempDir) {
       return tempDir
     }
   }