Skip to content

Commit

Permalink
ZipFile UnixCreateSetsPermissionsInExternalAttributes test fails on A…
Browse files Browse the repository at this point in the history
…pple OSes (#69583)

* ZipFile UnixCreateSetsPermissionsInExternalAttributes test fails on Apple OSes

There are scenarios where chmod succeeds, but the OS clears certain bits, like S_ISGID and S_ISUID. When this happens, the ZipFile tests fail because the .zip file doesn't contain the expected ExternalAttributes.

To fix this, read the file mode after calling chmod, and update the expected file mode so the tests still pass.

Fix #68293
Fix #60581

* Fix the tests to expect the correct file name
  • Loading branch information
eerhardt authored May 20, 2022
1 parent e7886b2 commit 6069680
Showing 1 changed file with 33 additions and 12 deletions.
45 changes: 33 additions & 12 deletions src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,21 @@ namespace System.IO.Compression.Tests
public partial class ZipFile_Unix : ZipFileTestBase
{
[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/68293", TestPlatforms.OSX)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/60581", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)]
public void UnixCreateSetsPermissionsInExternalAttributes()
{
// '7600' tests that S_ISUID, S_ISGID, and S_ISVTX bits get preserved in ExternalAttributes
string[] testPermissions = new[] { "777", "755", "644", "600", "7600" };

using (var tempFolder = new TempDirectory(Path.Combine(GetTestFilePath(), "testFolder")))
{
foreach (string permission in testPermissions)
{
CreateFile(tempFolder.Path, permission);
}
string[] expectedPermissions = CreateFiles(tempFolder.Path, testPermissions);

string archivePath = GetTestFilePath();
ZipFile.CreateFromDirectory(tempFolder.Path, archivePath);

using (ZipArchive archive = ZipFile.OpenRead(archivePath))
{
Assert.Equal(5, archive.Entries.Count);
Assert.Equal(expectedPermissions.Length, archive.Entries.Count);

foreach (ZipArchiveEntry entry in archive.Entries)
{
Expand All @@ -50,7 +45,7 @@ void EnsureExternalAttributes(string permissions, ZipArchiveEntry entry)
{
ZipFile.ExtractToDirectory(archivePath, extractFolder.Path);

foreach (string permission in testPermissions)
foreach (string permission in expectedPermissions)
{
string filename = Path.Combine(extractFolder.Path, permission + ".txt");
Assert.True(File.Exists(filename));
Expand Down Expand Up @@ -96,12 +91,38 @@ public void UnixExtractSetsFilePermissionsFromExternalAttributes()
}
}

private static void CreateFile(string folderPath, string permissions)
private static string[] CreateFiles(string folderPath, string[] testPermissions)
{
string filename = Path.Combine(folderPath, $"{permissions}.txt");
File.WriteAllText(filename, "contents");
string[] expectedPermissions = new string[testPermissions.Length];

for (int i = 0; i < testPermissions.Length; i++)
{
string permissions = testPermissions[i];
string filename = Path.Combine(folderPath, $"{permissions}.txt");
File.WriteAllText(filename, "contents");

Assert.Equal(0, Interop.Sys.ChMod(filename, Convert.ToInt32(permissions, 8)));

// In some environments, the file mode may be modified by the OS.
// See the Rationale section of https://linux.die.net/man/3/chmod.

Assert.Equal(0, Interop.Sys.ChMod(filename, Convert.ToInt32(permissions, 8)));
// To workaround this, read the file mode back, and if it has changed, update the file name
// since the name is used to compare the file mode.
Interop.Sys.FileStatus status;
Assert.Equal(0, Interop.Sys.Stat(filename, out status));
string updatedPermissions = Convert.ToString(status.Mode & 0xFFF, 8);
if (updatedPermissions != permissions)
{
string newFileName = Path.Combine(folderPath, $"{updatedPermissions}.txt");
File.Move(filename, newFileName);

permissions = updatedPermissions;
}

expectedPermissions[i] = permissions;
}

return expectedPermissions;
}

private static void EnsureFilePermissions(string filename, string permissions)
Expand Down

0 comments on commit 6069680

Please sign in to comment.