Implement UnixFileMode APIs (#69980)

* Implement UnixFileMode APIs on Unix. * Throw PNSE on Windows, add UnsupportedOSPlatform. * Fix API compat issue. * Borrow a few things from SafeFileHandle API PR to this compiles. * Fix System.IO.FileSystem.AccessControl compilation. * Add xml docs. * Replace Interop.Sys.Permissions to System.IO.UnixFileMode. * Throw PNSE immediately on Windows. * Add ODE to xml docs of methods that accept a handle. * Don't throw (PNSE) from FileSystemInfo.UnixFileMode getter on Windows. * Minor style fix. * Get rid of some casts. Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> * Add tests for creating a file/directory with UnixFileMode. * Some CI envs don't have a umask exe, try retrieving via a shell builtin. * Update expected test mode values. * Fix OSX * Fix Windows build. * Add ArgumentException tests. * Fix Windows build. * Add get/set tests. * Update test for Windows. * Make setters target link instead of link target. * Linux: fix SetUnixFileMode * Fix OSX compilation. * Try make all tests pass in CI. * For link, operate on target permissions. * Skip tests on Browser. * Add tests for 'Get' that doesn't use a 'Set' first. * Don't perform exist check for handles. * Fix Get test for wasm. * Review xml comments. * Add comment to test. * GetUnixFileMode for handle won't throw UnauthorizedAccessException. * Apply suggestions from code review Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> * PR feedback. * Update enum doc to say 'owner' instead of 'user'. * Use UnixFileMode in library. * Use UnixFileMode in library tests. * Fix Windows build. * Fix missing FileAccess when changing to FileStreamOptions API. * PR feedback. * Fix Argument_InvalidUnixCreateMode message. Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com> Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
dotnet · Jun 23, 2022 · ddc4f95 · ddc4f95
1 parent 9e8c7a8
commit ddc4f95
Show file tree

Hide file tree

Showing 58 changed files with 1,144 additions and 217 deletions.
diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Permissions.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Permissions.cs
diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
@@ -276,8 +276,6 @@
  Link="Common\Interop\Unix\System.Native\Interop.Access.cs" />
  <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs"
  Link="Common\Interop\Unix\Interop.Stat.cs" />
- <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Permissions.cs"
- Link="Common\Interop\Unix\Interop.Permissions.cs" />
  <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs"
  Link="Common\Interop\Unix\Interop.GetEUid.cs" />
  <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.IsMemberOfGroup.cs"

diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs
@@ -763,10 +763,12 @@ private static bool IsExecutable(string fullPath)
  return false;
  }
 
- Interop.Sys.Permissions permissions = ((Interop.Sys.Permissions)fileinfo.Mode) & Interop.Sys.Permissions.S_IXUGO;
+ const UnixFileMode AllExecute = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute;
+
+ UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute;
 
  // Avoid checking user/group when permission.
- if (permissions == Interop.Sys.Permissions.S_IXUGO)
+ if (permissions == AllExecute)
  {
  return true;
  }
@@ -785,11 +787,11 @@ private static bool IsExecutable(string fullPath)
  if (euid == fileinfo.Uid)
  {
  // We own the file.
- return (permissions & Interop.Sys.Permissions.S_IXUSR) != 0;
+ return (permissions & UnixFileMode.UserExecute) != 0;
  }
 
- bool groupCanExecute = (permissions & Interop.Sys.Permissions.S_IXGRP) != 0;
- bool otherCanExecute = (permissions & Interop.Sys.Permissions.S_IXOTH) != 0;
+ bool groupCanExecute = (permissions & UnixFileMode.GroupExecute) != 0;
+ bool otherCanExecute = (permissions & UnixFileMode.OtherExecute) != 0;
 
  // Avoid group check when group and other have same permissions.
  if (groupCanExecute == otherCanExecute)

diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
@@ -167,7 +167,7 @@ public void ProcessNameMatchesScriptName()
  string scriptName = GetTestFileName();
  string filename = Path.Combine(TestDirectory, scriptName);
  File.WriteAllText(filename, $"#!/bin/sh\nsleep 600\n"); // sleep 10 min.
- ChMod(filename, "744"); // set x-bit
+ File.SetUnixFileMode(filename, ExecutablePermissions);
 
  using (var process = Process.Start(new ProcessStartInfo { FileName = filename }))
  {
@@ -235,7 +235,7 @@ public void ProcessStart_UseShellExecute_OnUnix_FallsBackWhenNotRealExecutable()
  // Create a file that has the x-bit set, but which isn't a valid script.
  string filename = WriteScriptFile(TestDirectory, GetTestFileName(), returnValue: 0);
  File.WriteAllText(filename, $"not a script");
- ChMod(filename, "744"); // set x-bit
+ File.SetUnixFileMode(filename, ExecutablePermissions);
 
  RemoteInvokeOptions options = new RemoteInvokeOptions();
  options.StartInfo.EnvironmentVariables["PATH"] = path;
@@ -508,7 +508,7 @@ public void TestStartOnUnixWithBadPermissions()
  {
  string path = GetTestFilePath();
  File.Create(path).Dispose();
- ChMod(path, "644");
+ File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserWrite);
 
  Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(path));
  Assert.NotEqual(0, e.NativeErrorCode);
@@ -520,7 +520,7 @@ public void TestStartOnUnixWithBadFormat()
  {
  string path = GetTestFilePath();
  File.Create(path).Dispose();
- ChMod(path, "744"); // set x-bit
+ File.SetUnixFileMode(path, ExecutablePermissions);
 
  Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(path));
  Assert.NotEqual(0, e.NativeErrorCode);
@@ -936,14 +936,6 @@ private static int GetWaitStateReferenceCount(object waitState)
  return (int)referenCountField.GetValue(waitState);
  }
 
- [DllImport("libc")]
- private static extern int chmod(string path, int mode);
-
- private static void ChMod(string filename, string mode)
- {
- Assert.Equal(0, chmod(filename, Convert.ToInt32(mode, 8)));
- }
-
  [DllImport("libc")]
  private static extern uint geteuid();
  [DllImport("libc")]
@@ -1001,7 +993,7 @@ private string WriteScriptFile(string directory, string name, int returnValue)
  {
  string filename = Path.Combine(directory, name);
  File.WriteAllText(filename, $"#!/bin/sh\nexit {returnValue}\n");
- ChMod(filename, "744"); // set x-bit
+ File.SetUnixFileMode(filename, ExecutablePermissions);
  return filename;
  }
 

diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs
@@ -15,8 +15,5 @@ private string WriteScriptFile(string directory, string name, int returnValue)
  File.WriteAllText(filename, $"exit {returnValue}");
  return filename;
  }
-
- private static void ChMod(string filename, string mode)
- => throw new PlatformNotSupportedException();
  }
 }
diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs
@@ -23,6 +23,10 @@ namespace System.Diagnostics.Tests
 {
  public partial class ProcessTests : ProcessTestBase
  {
+ const UnixFileMode ExecutablePermissions = UnixFileMode.UserRead | UnixFileMode.UserExecute | UnixFileMode.UserWrite |
+ UnixFileMode.GroupRead | UnixFileMode.GroupExecute |
+ UnixFileMode.GroupRead | UnixFileMode.GroupExecute;
+
  private class FinalizingProcess : Process
  {
  public static volatile bool WasFinalized;
@@ -2193,7 +2197,7 @@ public void LongProcessNamesAreSupported()
  // Instead of using sleep directly, we wrap it with a script.
  sleepPath = GetTestFilePath();
  File.WriteAllText(sleepPath, $"#!/bin/sh\nsleep 600\n"); // sleep 10 min.
- ChMod(sleepPath, "744");
+ File.SetUnixFileMode(sleepPath, ExecutablePermissions);
  }
  else
  {

diff --git a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj
@@ -63,7 +63,6 @@
  <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetGroupName.cs" Link="Common\Interop\Unix\System.Native\Interop.GetGroupName.cs" />
  <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Link.cs" Link="Common\Interop\Unix\System.Native\Interop.Link.cs" />
  <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.MkFifo.cs" Link="Common\Interop\Unix\System.Native\Interop.MkFifo.cs" />
- <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Permissions.cs" Link="Common\Interop\Unix\Interop.Permissions.cs" />
  <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs" Link="Common\Interop\Unix\Interop.Stat.cs" />
  <Compile Include="$(CommonPath)System\IO\Archiving.Utils.Unix.cs" Link="Common\System\IO\Archiving.Utils.Unix.cs" />
  </ItemGroup>

diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Unix.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Unix.cs
@@ -3,6 +3,7 @@
 
 using System.Diagnostics;
 using Microsoft.Win32.SafeHandles;
+using System.IO;
 
 namespace System.Formats.Tar
 {
@@ -53,7 +54,7 @@ partial void SetModeOnFile(SafeFileHandle handle, string destinationFileName)
  // If the permissions weren't set at all, don't write the file's permissions.
  if (permissions != 0)
  {
- Interop.CheckIo(Interop.Sys.FChMod(handle, permissions), destinationFileName);
+ File.SetUnixFileMode(handle, (UnixFileMode)permissions);
  }
  }
  }

diff --git a/...ssion.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.Unix.cs b/...ssion.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.Unix.cs
@@ -22,7 +22,9 @@ static partial void ExtractExternalAttributes(FileStream fs, ZipArchiveEntry ent
  // include the permissions, or was made on Windows.
  if (permissions != 0)
  {
- Interop.CheckIo(Interop.Sys.FChMod(fs.SafeFileHandle, permissions), fs.Name);
+#pragma warning disable CA1416 // Validate platform compatibility
+ File.SetUnixFileMode(fs.SafeFileHandle, (UnixFileMode)permissions);
+#pragma warning restore CA1416
  }
  }
  }

diff --git a/src/libraries/System.IO.FileSystem/tests/Base/AllGetSetAttributes.cs b/src/libraries/System.IO.FileSystem/tests/Base/AllGetSetAttributes.cs
@@ -59,7 +59,15 @@ public void SymLinksReflectSymLinkAttributes()
  try
  {
  Assert.Equal(FileAttributes.ReadOnly, FileAttributes.ReadOnly & GetAttributes(path));
- Assert.NotEqual(FileAttributes.ReadOnly, FileAttributes.ReadOnly & GetAttributes(linkPath));
+ if (OperatingSystem.IsWindows())
+ {
+ Assert.NotEqual(FileAttributes.ReadOnly, FileAttributes.ReadOnly & GetAttributes(linkPath)); 
+ }
+ else
+ {
+ // On Unix, Get/SetAttributes FileAttributes.ReadOnly operates on the target of the link.
+ Assert.Equal(FileAttributes.ReadOnly, FileAttributes.ReadOnly & GetAttributes(linkPath)); 
+ }
  }
  finally
  {