From b0acc3cbe215758fbc4b5909ad69f3ba7e90fa00 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 16 Aug 2022 11:15:29 +0200 Subject: [PATCH 1/7] Directory.CreateDirectory: create missing parents using default UnixFileMode. --- .../src/System/Formats/Tar/TarHelpers.Unix.cs | 17 +++++++++++------ .../CreateDirectory_UnixFileMode.Unix.cs | 15 +++++++++++++++ .../src/System/IO/FileSystem.Unix.cs | 5 +++-- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs index f684dd78fde1c..88554a6fb1e1b 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs @@ -92,7 +92,7 @@ internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool o } else if (overwriteMetadata || pendingModes.ContainsKey(fullPath)) { - pendingModes[fullPath] = mode.Value; + pendingModes[fullPath] = mode.Value; } } return; @@ -113,18 +113,23 @@ internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool o mode = ExtractPermissions; } + // Create missing parents with ExtractPermissions and add them to be set to + // DefaultDirectoryMode if they are not present in the archive later. string parentDir = Path.GetDirectoryName(fullPath)!; string rootDir = Path.GetPathRoot(parentDir)!; - bool hasMissingParents = false; + Stack? missingParents = null; for (string dir = parentDir; dir != rootDir && !Directory.Exists(dir); dir = Path.GetDirectoryName(dir)!) { pendingModes.Add(dir, DefaultDirectoryMode); - hasMissingParents = true; + missingParents ??= new Stack(); + missingParents.Push(dir); } - - if (hasMissingParents) + if (missingParents is not null) { - Directory.CreateDirectory(parentDir, ExtractPermissions); + while (missingParents.TryPop(out string? dir)) + { + Directory.CreateDirectory(dir, ExtractPermissions); + } } Directory.CreateDirectory(fullPath, mode.Value); diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/CreateDirectory_UnixFileMode.Unix.cs b/src/libraries/System.IO.FileSystem/tests/Directory/CreateDirectory_UnixFileMode.Unix.cs index 73c8f587bcced..5627974f47420 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/CreateDirectory_UnixFileMode.Unix.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/CreateDirectory_UnixFileMode.Unix.cs @@ -39,6 +39,21 @@ public void CreateDoesntChangeExistingMode() Assert.Equal(initialMode, sameDir.UnixFileMode); } + [Fact] + public void MissingParentsHaveDefaultPermissions() + { + string parent = GetRandomDirPath(); + string child = Path.Combine(parent, "child"); + + const UnixFileMode childMode = UnixFileMode.UserRead | UnixFileMode.UserExecute; + DirectoryInfo childDir = Directory.CreateDirectory(child, childMode); + + Assert.Equal(childMode, childDir.UnixFileMode); + + UnixFileMode defaultPermissions = Directory.CreateDirectory(GetRandomDirPath()).UnixFileMode; + Assert.Equal(defaultPermissions, File.GetUnixFileMode(parent)); + } + [Theory] [InlineData((UnixFileMode)(1 << 12), false)] [InlineData((UnixFileMode)(1 << 12), true)] diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index 67237805c3a74..f538524471a91 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -328,7 +328,7 @@ private static void CreateParentsAndDirectory(string fullPath, UnixFileMode unix } ReadOnlySpan mkdirPath = fullPath.AsSpan(0, i); - int result = Interop.Sys.MkDir(mkdirPath, (int)unixCreateMode); + int result = Interop.Sys.MkDir(mkdirPath, (int)DefaultUnixCreateDirectoryMode); if (result == 0) { break; // Created parent. @@ -360,7 +360,8 @@ private static void CreateParentsAndDirectory(string fullPath, UnixFileMode unix for (i = stackDir.Length - 1; i >= 0; i--) { ReadOnlySpan mkdirPath = fullPath.AsSpan(0, stackDir[i]); - int result = Interop.Sys.MkDir(mkdirPath, (int)unixCreateMode); + UnixFileMode mode = i == 0 ? unixCreateMode : DefaultUnixCreateDirectoryMode; + int result = Interop.Sys.MkDir(mkdirPath, (int)mode); if (result < 0) { Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); From 253e68abaae9f19d2907c4255059b618e4eea720 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 17 Aug 2022 14:22:51 +0200 Subject: [PATCH 2/7] Simplify TarHelpers.CreateDirectory. --- .../src/System/Formats/Tar/TarHelpers.Unix.cs | 67 +++++-------------- .../TarFile.ExtractToDirectory.File.Tests.cs | 10 +-- ...File.ExtractToDirectoryAsync.File.Tests.cs | 35 ++++++---- .../System.Formats.Tar/tests/TarTestsBase.cs | 7 ++ 4 files changed, 49 insertions(+), 70 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs index 88554a6fb1e1b..187e43ce150d1 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs @@ -47,50 +47,33 @@ public int Compare (string? x, string? y) private static UnixFileMode UMask => s_umask.Value; - /* - Tar files are usually ordered: parent directories come before their child entries. - - They may be unordered. In that case we need to create parent directories before - we know the proper permissions for these directories. - - We create these directories with restrictive permissions. If we encounter an entry for - the directory later, we store the mode to apply it later. - - If the archive doesn't have an entry for the parent directory, we use the default mask. - - The pending modes to be applied are tracked through a reverse-sorted dictionary. - The reverse order is needed to apply permissions to children before their parent. - Otherwise we may apply a restrictive mask to the parent, that prevents us from - changing a child. - */ - + // Use a reverse-sorted dictionary to apply permission to children before their parents. + // Otherwise we may apply a restrictive mask to the parent, that prevents us from changing a child. internal static SortedDictionary? CreatePendingModesDictionary() => new SortedDictionary(s_reverseStringComparer); internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool overwriteMetadata, SortedDictionary? pendingModes) { - // Restrictive mask for creating the missing parent directories while extracting. + // Minimal permissions required for extracting. const UnixFileMode ExtractPermissions = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute; Debug.Assert(pendingModes is not null); if (Directory.Exists(fullPath)) { - // Apply permissions to an existing directory when we're overwriting metadata - // or the directory was created as a missing parent (stored in pendingModes). - if (mode.HasValue) + // Apply permissions to an existing directory when we're overwriting metadata. + if (mode.HasValue && overwriteMetadata) { + // Ensure we have sufficient permissions to extract in the directory. bool hasExtractPermissions = (mode.Value & ExtractPermissions) == ExtractPermissions; if (hasExtractPermissions) { - bool removed = pendingModes.Remove(fullPath); - if (overwriteMetadata || removed) - { - UnixFileMode umask = UMask; - File.SetUnixFileMode(fullPath, mode.Value & ~umask); - } + pendingModes.Remove(fullPath); + + UnixFileMode umask = UMask; + File.SetUnixFileMode(fullPath, mode.Value & ~umask); } - else if (overwriteMetadata || pendingModes.ContainsKey(fullPath)) + else { pendingModes[fullPath] = mode.Value; } @@ -98,6 +81,7 @@ internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool o return; } + // Missing parents are created using default permissions. if (mode.HasValue) { // Ensure we have sufficient permissions to extract in the directory. @@ -106,33 +90,14 @@ internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool o pendingModes[fullPath] = mode.Value; mode = ExtractPermissions; } - } - else - { - pendingModes.Add(fullPath, DefaultDirectoryMode); - mode = ExtractPermissions; - } - // Create missing parents with ExtractPermissions and add them to be set to - // DefaultDirectoryMode if they are not present in the archive later. - string parentDir = Path.GetDirectoryName(fullPath)!; - string rootDir = Path.GetPathRoot(parentDir)!; - Stack? missingParents = null; - for (string dir = parentDir; dir != rootDir && !Directory.Exists(dir); dir = Path.GetDirectoryName(dir)!) - { - pendingModes.Add(dir, DefaultDirectoryMode); - missingParents ??= new Stack(); - missingParents.Push(dir); + Directory.CreateDirectory(fullPath, mode.Value); } - if (missingParents is not null) + else { - while (missingParents.TryPop(out string? dir)) - { - Directory.CreateDirectory(dir, ExtractPermissions); - } + // Create directory using default permissions. + Directory.CreateDirectory(fullPath); } - - Directory.CreateDirectory(fullPath, mode.Value); } internal static void SetPendingModes(SortedDictionary? pendingModes) diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs index f741926cdd8ef..02f5c534fc252 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs @@ -216,20 +216,16 @@ public void UnixFileModes(bool overwrite) Assert.True(File.Exists(filePath), $"{filePath}' does not exist."); AssertFileModeEquals(filePath, TestPermission2); - // Missing parents are created with DefaultDirectoryMode. - // The mode is not set when overwrite == true if there is no entry and the directory exists before extracting. + // Missing parents are created with CreateDirectoryDefaultMode. Assert.True(Directory.Exists(missingParentPath), $"{missingParentPath}' does not exist."); - if (!overwrite) - { - AssertFileModeEquals(missingParentPath, DefaultDirectoryMode); - } + AssertFileModeEquals(missingParentPath, CreateDirectoryDefaultMode); Assert.True(Directory.Exists(missingParentDirPath), $"{missingParentDirPath}' does not exist."); AssertFileModeEquals(missingParentDirPath, TestPermission3); // Directory modes that are out-of-order are still applied. Assert.True(Directory.Exists(outOfOrderDirPath), $"{outOfOrderDirPath}' does not exist."); - AssertFileModeEquals(outOfOrderDirPath, TestPermission4); + AssertFileModeEquals(outOfOrderDirPath, overwrite ? TestPermission4 : CreateDirectoryDefaultMode); } [Theory] diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs index 01d2457018ce2..384ed00353ada 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs @@ -191,8 +191,10 @@ public async Task ExtractArchiveWithEntriesThatStartWithSlashDotPrefix_Async() } } - [Fact] - public async Task UnixFileModes_Async() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task UnixFileModes_Async(bool overwrite) { using TempDirectory source = new TempDirectory(); using TempDirectory destination = new TempDirectory(); @@ -223,29 +225,38 @@ public async Task UnixFileModes_Async() writer.WriteEntry(outOfOrderDir); } - await TarFile.ExtractToDirectoryAsync(archivePath, destination.Path, overwriteFiles: false); - string dirPath = Path.Join(destination.Path, "dir"); - Assert.True(Directory.Exists(dirPath), $"{dirPath}' does not exist."); + string filePath = Path.Join(destination.Path, "file"); + string missingParentPath = Path.Join(destination.Path, "missing_parent"); + string missingParentDirPath = Path.Join(missingParentPath, "dir"); + string outOfOrderDirPath = Path.Join(destination.Path, "out_of_order_parent"); + + if (overwrite) + { + File.OpenWrite(filePath).Dispose(); + Directory.CreateDirectory(dirPath); + Directory.CreateDirectory(missingParentDirPath); + Directory.CreateDirectory(outOfOrderDirPath); + } + + await TarFile.ExtractToDirectoryAsync(archivePath, destination.Path, overwriteFiles: overwrite); + + Assert.True(Directory.Exists(dirPath), $"{dirPath}' does not exist."); AssertFileModeEquals(dirPath, TestPermission1); - string filePath = Path.Join(destination.Path, "file"); Assert.True(File.Exists(filePath), $"{filePath}' does not exist."); AssertFileModeEquals(filePath, TestPermission2); - // Missing parents are created with DefaultDirectoryMode. - string missingParentPath = Path.Join(destination.Path, "missing_parent"); + // Missing parents are created with CreateDirectoryDefaultMode. Assert.True(Directory.Exists(missingParentPath), $"{missingParentPath}' does not exist."); - AssertFileModeEquals(missingParentPath, DefaultDirectoryMode); + AssertFileModeEquals(missingParentPath, CreateDirectoryDefaultMode); - string missingParentDirPath = Path.Join(missingParentPath, "dir"); Assert.True(Directory.Exists(missingParentDirPath), $"{missingParentDirPath}' does not exist."); AssertFileModeEquals(missingParentDirPath, TestPermission3); // Directory modes that are out-of-order are still applied. - string outOfOrderDirPath = Path.Join(destination.Path, "out_of_order_parent"); Assert.True(Directory.Exists(outOfOrderDirPath), $"{outOfOrderDirPath}' does not exist."); - AssertFileModeEquals(outOfOrderDirPath, TestPermission4); + AssertFileModeEquals(outOfOrderDirPath, overwrite ? TestPermission4 : CreateDirectoryDefaultMode); } [Fact] diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index b167a980acd2b..77a1f03b721d2 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -17,6 +17,8 @@ public abstract partial class TarTestsBase : FileCleanupTestBase protected const UnixFileMode DefaultFileMode = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.GroupRead | UnixFileMode.OtherRead; // 644 in octal, internally used as default protected const UnixFileMode DefaultDirectoryMode = DefaultFileMode | UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; // 755 in octal, internally used as default + protected readonly UnixFileMode CreateDirectoryDefaultMode; // Mode of directories created using Directory.CreateDirectory(string). + // Mode assumed for files and directories on Windows. protected const UnixFileMode DefaultWindowsMode = DefaultFileMode | UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; // 755 in octal, internally used as default @@ -115,6 +117,11 @@ public enum TestTarFormat protected static bool IsNotLinuxBionic => !PlatformDetection.IsLinuxBionic; + protected TarTestsBase() + { + CreateDirectoryDefaultMode = PlatformDetection.IsWindows ? UnixFileMode.None : Directory.CreateDirectory(GetRandomDirPath()).UnixFileMode; + } + protected static string GetTestCaseUnarchivedFolderPath(string testCaseName) => Path.Join(Directory.GetCurrentDirectory(), "unarchived", testCaseName); From daed79e774ddcf5d1ce02ab03087fda45a01f7a7 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 18 Aug 2022 09:52:42 +0200 Subject: [PATCH 3/7] PR feedback. --- .../src/System/Formats/Tar/TarHelpers.Unix.cs | 3 +-- src/libraries/System.Formats.Tar/tests/TarTestsBase.cs | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs index 187e43ce150d1..e03b649acd0c7 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs @@ -81,7 +81,7 @@ internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool o return; } - // Missing parents are created using default permissions. + // If there are missing parents, Directory.CreateDirectory will create them using default permissions. if (mode.HasValue) { // Ensure we have sufficient permissions to extract in the directory. @@ -95,7 +95,6 @@ internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool o } else { - // Create directory using default permissions. Directory.CreateDirectory(fullPath); } } diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index 77a1f03b721d2..278e50edab27e 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -119,7 +119,7 @@ public enum TestTarFormat protected TarTestsBase() { - CreateDirectoryDefaultMode = PlatformDetection.IsWindows ? UnixFileMode.None : Directory.CreateDirectory(GetRandomDirPath()).UnixFileMode; + CreateDirectoryDefaultMode = Directory.CreateDirectory(GetRandomDirPath()).UnixFileMode; // '0777 & ~umask' } protected static string GetTestCaseUnarchivedFolderPath(string testCaseName) => @@ -416,7 +416,8 @@ protected static void AssertEntryModeFromFileSystemEquals(TarEntry entry, UnixFi protected static void AssertFileModeEquals(string path, UnixFileMode mode) { - if (!PlatformDetection.IsWindows) + if (!PlatformDetection.IsWindows && + !PlatformDetection.IsAndroid) // Android may change the requested permissions. { Assert.Equal(mode, File.GetUnixFileMode(path)); } From 03858aa070e138813d34718bf97886deb481ca66 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 19 Aug 2022 06:32:02 +0200 Subject: [PATCH 4/7] AssertFileModeEquals: don't skip on Android. --- .../System.Formats.Tar/tests/TarTestsBase.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index 278e50edab27e..04d034ce8764f 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -18,6 +18,7 @@ public abstract partial class TarTestsBase : FileCleanupTestBase protected const UnixFileMode DefaultDirectoryMode = DefaultFileMode | UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; // 755 in octal, internally used as default protected readonly UnixFileMode CreateDirectoryDefaultMode; // Mode of directories created using Directory.CreateDirectory(string). + protected readonly UnixFileMode UMask; // Mode assumed for files and directories on Windows. protected const UnixFileMode DefaultWindowsMode = DefaultFileMode | UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; // 755 in octal, internally used as default @@ -120,6 +121,7 @@ public enum TestTarFormat protected TarTestsBase() { CreateDirectoryDefaultMode = Directory.CreateDirectory(GetRandomDirPath()).UnixFileMode; // '0777 & ~umask' + UMask = ~CreateDirectoryDefaultMode & (UnixFileMode)Convert.ToInt32("777", 8); } protected static string GetTestCaseUnarchivedFolderPath(string testCaseName) => @@ -414,12 +416,20 @@ protected static void AssertEntryModeFromFileSystemEquals(TarEntry entry, UnixFi Assert.Equal(fileMode, entry.Mode); } - protected static void AssertFileModeEquals(string path, UnixFileMode mode) + protected void AssertFileModeEquals(string path, UnixFileMode archiveMode) { - if (!PlatformDetection.IsWindows && - !PlatformDetection.IsAndroid) // Android may change the requested permissions. + if (!PlatformDetection.IsWindows) { - Assert.Equal(mode, File.GetUnixFileMode(path)); + UnixFileMode expectedMode = archiveMode & ~UMask; + + UnixFileMode actualMode = File.GetUnixFileMode(path); + // Ignore SetGroup: it may have been added to preserve group ownership. + if ((expectedMode & UnixFileMode.SetGroup) == 0) + { + actualMode &= ~UnixFileMode.SetGroup; + } + + Assert.Equal(expectedMode, actualMode); } } From 7f3d1bd1fd07b0205bf2ef888e02b2582d308ad0 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 19 Aug 2022 06:39:32 +0200 Subject: [PATCH 5/7] Always overwite directory metadata. --- .../System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs | 8 ++++---- .../src/System/Formats/Tar/TarHelpers.Unix.cs | 6 +++--- .../TarFile/TarFile.ExtractToDirectory.File.Tests.cs | 2 +- .../TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs index 00c790762e4b6..27fe4af8701a9 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs @@ -287,12 +287,12 @@ internal void ExtractRelativeToDirectory(string destinationDirectoryPath, bool o if (EntryType == TarEntryType.Directory) { - TarHelpers.CreateDirectory(fileDestinationPath, Mode, overwrite, pendingModes); + TarHelpers.CreateDirectory(fileDestinationPath, Mode, pendingModes); } else { // If it is a file, create containing directory. - TarHelpers.CreateDirectory(Path.GetDirectoryName(fileDestinationPath)!, mode: null, overwrite, pendingModes); + TarHelpers.CreateDirectory(Path.GetDirectoryName(fileDestinationPath)!, mode: null, pendingModes); ExtractToFileInternal(fileDestinationPath, linkTargetPath, overwrite); } } @@ -309,13 +309,13 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b if (EntryType == TarEntryType.Directory) { - TarHelpers.CreateDirectory(fileDestinationPath, Mode, overwrite, pendingModes); + TarHelpers.CreateDirectory(fileDestinationPath, Mode, pendingModes); return Task.CompletedTask; } else { // If it is a file, create containing directory. - TarHelpers.CreateDirectory(Path.GetDirectoryName(fileDestinationPath)!, mode: null, overwrite, pendingModes); + TarHelpers.CreateDirectory(Path.GetDirectoryName(fileDestinationPath)!, mode: null, pendingModes); return ExtractToFileInternalAsync(fileDestinationPath, linkTargetPath, overwrite, cancellationToken); } } diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs index e03b649acd0c7..3e0a0b814484d 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Unix.cs @@ -52,7 +52,7 @@ public int Compare (string? x, string? y) internal static SortedDictionary? CreatePendingModesDictionary() => new SortedDictionary(s_reverseStringComparer); - internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool overwriteMetadata, SortedDictionary? pendingModes) + internal static void CreateDirectory(string fullPath, UnixFileMode? mode, SortedDictionary? pendingModes) { // Minimal permissions required for extracting. const UnixFileMode ExtractPermissions = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute; @@ -61,8 +61,8 @@ internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool o if (Directory.Exists(fullPath)) { - // Apply permissions to an existing directory when we're overwriting metadata. - if (mode.HasValue && overwriteMetadata) + // Apply permissions to an existing directory. + if (mode.HasValue) { // Ensure we have sufficient permissions to extract in the directory. bool hasExtractPermissions = (mode.Value & ExtractPermissions) == ExtractPermissions; diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs index 02f5c534fc252..ee00ccfaf6ccb 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs @@ -225,7 +225,7 @@ public void UnixFileModes(bool overwrite) // Directory modes that are out-of-order are still applied. Assert.True(Directory.Exists(outOfOrderDirPath), $"{outOfOrderDirPath}' does not exist."); - AssertFileModeEquals(outOfOrderDirPath, overwrite ? TestPermission4 : CreateDirectoryDefaultMode); + AssertFileModeEquals(outOfOrderDirPath, TestPermission4); } [Theory] diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs index 384ed00353ada..d51f099a6c1ce 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs @@ -256,7 +256,7 @@ public async Task UnixFileModes_Async(bool overwrite) // Directory modes that are out-of-order are still applied. Assert.True(Directory.Exists(outOfOrderDirPath), $"{outOfOrderDirPath}' does not exist."); - AssertFileModeEquals(outOfOrderDirPath, overwrite ? TestPermission4 : CreateDirectoryDefaultMode); + AssertFileModeEquals(outOfOrderDirPath, TestPermission4); } [Fact] From 5271907a8d79b34239fa216ccde6d12918f2cfcf Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 19 Aug 2022 06:54:17 +0200 Subject: [PATCH 6/7] TarHelpers.Windows: remove CreateDirectory overwriteMetadata. --- .../src/System/Formats/Tar/TarHelpers.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Windows.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Windows.cs index e00f6476764ab..6569ff237dbf9 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Windows.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.Windows.cs @@ -13,7 +13,7 @@ internal static partial class TarHelpers internal static SortedDictionary? CreatePendingModesDictionary() => null; - internal static void CreateDirectory(string fullPath, UnixFileMode? mode, bool overwriteMetadata, SortedDictionary? pendingModes) + internal static void CreateDirectory(string fullPath, UnixFileMode? mode, SortedDictionary? pendingModes) => Directory.CreateDirectory(fullPath); internal static void SetPendingModes(SortedDictionary? pendingModes) From e1cf9d43604d94c782f0d8712417427879c6f9b6 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 19 Aug 2022 09:32:57 +0200 Subject: [PATCH 7/7] Fix Assert indentation. --- .../tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs index d51f099a6c1ce..905e9b61bfb8e 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs @@ -241,7 +241,7 @@ public async Task UnixFileModes_Async(bool overwrite) await TarFile.ExtractToDirectoryAsync(archivePath, destination.Path, overwriteFiles: overwrite); - Assert.True(Directory.Exists(dirPath), $"{dirPath}' does not exist."); + Assert.True(Directory.Exists(dirPath), $"{dirPath}' does not exist."); AssertFileModeEquals(dirPath, TestPermission1); Assert.True(File.Exists(filePath), $"{filePath}' does not exist.");