From eda01fe188baaf6d860a67cbd4cbad54bb5f9426 Mon Sep 17 00:00:00 2001 From: Denys Zhuravel Date: Tue, 14 Jun 2022 15:31:36 +0000 Subject: [PATCH 1/6] Set the ExternalAttributes to the values from the file. We need to set the external attributes to the *exact* values that the files have. Otherwise if we create a ZipArchive entry and set something there this would get added to the file values and this is not what we want. --- .../IO/Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs index d2e4f44a6751af..7aca1d0566810c 100644 --- a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs +++ b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs @@ -10,7 +10,7 @@ static partial void SetExternalAttributes(FileStream fs, ZipArchiveEntry entry) Interop.Sys.FileStatus status; Interop.CheckIo(Interop.Sys.FStat(fs.SafeFileHandle, out status), fs.Name); - entry.ExternalAttributes |= status.Mode << 16; + entry.ExternalAttributes = status.Mode << 16; } } } From 632b85867b9994c6ac98d1caa7ad8f6ca09aa21f Mon Sep 17 00:00:00 2001 From: Denys Zhuravel Date: Mon, 27 Jun 2022 20:44:03 +0000 Subject: [PATCH 2/6] Add an assert of te SetExternalAttributes --- .../Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs index 7aca1d0566810c..32aa2571f2f3fb 100644 --- a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs +++ b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchive.Create.Unix.cs @@ -1,12 +1,16 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; + namespace System.IO.Compression { public static partial class ZipFileExtensions { static partial void SetExternalAttributes(FileStream fs, ZipArchiveEntry entry) { + Debug.Assert(!OperatingSystem.IsWindows()); + Interop.Sys.FileStatus status; Interop.CheckIo(Interop.Sys.FStat(fs.SafeFileHandle, out status), fs.Name); From b3976f1f3efc0117521bfa0fc937790c132f9825 Mon Sep 17 00:00:00 2001 From: Denys Zhuravel Date: Mon, 27 Jun 2022 20:46:35 +0000 Subject: [PATCH 3/6] Move the constants into separate files --- .../src/System.IO.Compression.csproj | 2 ++ .../IO/Compression/ZipArchiveEntry.Unix.cs | 2 -- .../IO/Compression/ZipArchiveEntry.Windows.cs | 2 -- .../System/IO/Compression/ZipArchiveEntry.cs | 2 ++ .../ZipArchiveEntryConstants.Unix.cs | 25 +++++++++++++++++++ .../ZipArchiveEntryConstants.Windows.cs | 22 ++++++++++++++++ 6 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs create mode 100644 src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs diff --git a/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj b/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj index 7d2ba356485680..81c00261df49cd 100644 --- a/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj +++ b/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj @@ -45,12 +45,14 @@ + + /// To get the file name of a ZipArchiveEntry, we should be parsing the FullName based /// on the path specifications and requirements of the OS that ZipArchive was created on. diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Windows.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Windows.cs index 730ade4b35081c..c617e00543bec6 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Windows.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Windows.cs @@ -5,8 +5,6 @@ namespace System.IO.Compression { public partial class ZipArchiveEntry { - internal const ZipVersionMadeByPlatform CurrentZipPlatform = ZipVersionMadeByPlatform.Windows; - /// /// To get the file name of a ZipArchiveEntry, we should be parsing the FullName based /// on the path specifications and requirements of the OS that ZipArchive was created on. diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs index f58bd778a1c026..710fc659efeb1f 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs @@ -9,6 +9,8 @@ using System.Threading; using System.Threading.Tasks; +using static System.IO.Compression.ZipArchiveEntryConstants; + namespace System.IO.Compression { // The disposable fields that this class owns get disposed when the ZipArchive it belongs to gets disposed diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs new file mode 100644 index 00000000000000..8da9934e06a0e7 --- /dev/null +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.IO.Compression +{ + internal static partial class ZipArchiveEntryConstants + { + internal const ZipVersionMadeByPlatform CurrentZipPlatform = ZipVersionMadeByPlatform.Unix; + + /// + /// The default external file attributes are used to support zip archives on multiple platforms. + /// + internal const UnixFileMode DefaultFileEntryPermissions = UnixFileMode.UserRead | UnixFileMode.UserWrite + | UnixFileMode.GroupRead + | UnixFileMode.OtherRead; + + /// + /// The default external directory attributes are used to support zip archives on multiple platforms. + /// Directories on Unix require the execute permissions to get into them. + /// + internal const UnixFileMode DefaultDirectoryEntryPermissions = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute + | UnixFileMode.GroupRead | UnixFileMode.GroupExecute + | UnixFileMode.OtherRead | UnixFileMode.OtherExecute; + } +} diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs new file mode 100644 index 00000000000000..6f8f104d35ed32 --- /dev/null +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.IO.Compression +{ + internal static partial class ZipArchiveEntryConstants + { + internal const ZipVersionMadeByPlatform CurrentZipPlatform = ZipVersionMadeByPlatform.Windows; + + /// + /// The default external file attributes are used to support zip archives on multiple platforms. + /// Since Windows doesn't use file permissions, there's no default value needed. + /// + internal const UnixFileMode DefaultFileEntryPermissions = UnixFileMode.None; + + /// + /// The default external directory attributes are used to support zip archives on multiple platforms. + /// Since Windows doesn't use file permissions, there's no default value needed. + /// + internal const UnixFileMode DefaultDirectoryEntryPermissions = UnixFileMode.None; + } +} From 139850e65ce09d9eb36409d3c5b577234437d8b9 Mon Sep 17 00:00:00 2001 From: Denys Zhuravel Date: Mon, 27 Jun 2022 20:47:00 +0000 Subject: [PATCH 4/6] Add a test verifying the new behavior --- .../tests/System.IO.Compression.Tests.csproj | 4 ++++ .../tests/ZipArchive/zip_CreateTests.Unix.cs | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 src/libraries/System.IO.Compression/tests/ZipArchive/zip_CreateTests.Unix.cs diff --git a/src/libraries/System.IO.Compression/tests/System.IO.Compression.Tests.csproj b/src/libraries/System.IO.Compression/tests/System.IO.Compression.Tests.csproj index adac7a348c6922..229119e3aa37cf 100644 --- a/src/libraries/System.IO.Compression/tests/System.IO.Compression.Tests.csproj +++ b/src/libraries/System.IO.Compression/tests/System.IO.Compression.Tests.csproj @@ -41,6 +41,10 @@ + + + + diff --git a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_CreateTests.Unix.cs b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_CreateTests.Unix.cs new file mode 100644 index 00000000000000..121ffada3054ee --- /dev/null +++ b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_CreateTests.Unix.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Threading.Tasks; +using Xunit; + +namespace System.IO.Compression.Tests +{ + public partial class zip_CreateTests : ZipFileTestBase + { + [Theory] + [InlineData("folder/", 493 << 16)] + [InlineData("folder/file", 420 << 16)] + [InlineData("folder\\file", 420 << 16)] + public static void Verify_Default_Permissions_Are_Applied_For_Entries(string path, int permissions) + { + using var archive = new ZipArchive(new MemoryStream(), ZipArchiveMode.Create, false); + var newEntry = archive.CreateEntry(path); + Assert.Equal(newEntry.ExternalAttributes, permissions); + } + } +} From 424127d6ccc95bb49ad69e01e26bbaeb5631af62 Mon Sep 17 00:00:00 2001 From: Denys Zhuravel Date: Mon, 27 Jun 2022 20:47:59 +0000 Subject: [PATCH 5/6] Use the new default permissions on the ZipArchiveEntry --- .../src/System/IO/Compression/ZipArchiveEntry.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs index 710fc659efeb1f..b75af75204aef8 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs @@ -117,7 +117,11 @@ internal ZipArchiveEntry(ZipArchive archive, string entryName) _compressedSize = 0; // we don't know these yet _uncompressedSize = 0; - _externalFileAttr = 0; + UnixFileMode defaultEntryPermissions = entryName.EndsWith(Path.DirectorySeparatorChar) || entryName.EndsWith(Path.AltDirectorySeparatorChar) + ? DefaultDirectoryEntryPermissions + : DefaultFileEntryPermissions; + _externalFileAttr = (uint)(defaultEntryPermissions) << 16; + _offsetOfLocalHeader = 0; _storedOffsetOfCompressedData = null; _crc32 = 0; From 5c8920b20194dbdac30777945cc2e3d8b8776b3d Mon Sep 17 00:00:00 2001 From: Denys Zhuravel Date: Mon, 27 Jun 2022 20:56:26 +0000 Subject: [PATCH 6/6] Move the zip constants to the common directory Move the new zip constants to the common folder and use the constants files in both IO.Compression and IO.Compression.ZipFile --- .../System/IO/Compression/ZipArchiveEntryConstants.Unix.cs | 2 -- .../IO/Compression/ZipArchiveEntryConstants.Windows.cs | 2 -- .../src/System.IO.Compression.ZipFile.csproj | 1 + .../ZipFileExtensions.ZipArchive.Create.Unix.cs | 7 +++++++ .../System.IO.Compression/src/System.IO.Compression.csproj | 4 ++-- .../src/System/IO/Compression/ZipArchiveEntry.Unix.cs | 2 ++ .../src/System/IO/Compression/ZipArchiveEntry.Windows.cs | 2 ++ 7 files changed, 14 insertions(+), 6 deletions(-) rename src/libraries/{System.IO.Compression => Common}/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs (92%) rename src/libraries/{System.IO.Compression => Common}/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs (89%) diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs b/src/libraries/Common/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs similarity index 92% rename from src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs rename to src/libraries/Common/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs index 8da9934e06a0e7..6097961679a85c 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs +++ b/src/libraries/Common/src/System/IO/Compression/ZipArchiveEntryConstants.Unix.cs @@ -5,8 +5,6 @@ namespace System.IO.Compression { internal static partial class ZipArchiveEntryConstants { - internal const ZipVersionMadeByPlatform CurrentZipPlatform = ZipVersionMadeByPlatform.Unix; - /// /// The default external file attributes are used to support zip archives on multiple platforms. /// diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs b/src/libraries/Common/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs similarity index 89% rename from src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs rename to src/libraries/Common/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs index 6f8f104d35ed32..5f685313e7bc5c 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs +++ b/src/libraries/Common/src/System/IO/Compression/ZipArchiveEntryConstants.Windows.cs @@ -5,8 +5,6 @@ namespace System.IO.Compression { internal static partial class ZipArchiveEntryConstants { - internal const ZipVersionMadeByPlatform CurrentZipPlatform = ZipVersionMadeByPlatform.Windows; - /// /// The default external file attributes are used to support zip archives on multiple platforms. /// Since Windows doesn't use file permissions, there's no default value needed. diff --git a/src/libraries/System.IO.Compression.ZipFile/src/System.IO.Compression.ZipFile.csproj b/src/libraries/System.IO.Compression.ZipFile/src/System.IO.Compression.ZipFile.csproj index 3287498e587c16..b573949d699958 100644 --- a/src/libraries/System.IO.Compression.ZipFile/src/System.IO.Compression.ZipFile.csproj +++ b/src/libraries/System.IO.Compression.ZipFile/src/System.IO.Compression.ZipFile.csproj @@ -25,6 +25,7 @@ + - + - + /// To get the file name of a ZipArchiveEntry, we should be parsing the FullName based /// on the path specifications and requirements of the OS that ZipArchive was created on. diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Windows.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Windows.cs index c617e00543bec6..730ade4b35081c 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Windows.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Windows.cs @@ -5,6 +5,8 @@ namespace System.IO.Compression { public partial class ZipArchiveEntry { + internal const ZipVersionMadeByPlatform CurrentZipPlatform = ZipVersionMadeByPlatform.Windows; + /// /// To get the file name of a ZipArchiveEntry, we should be parsing the FullName based /// on the path specifications and requirements of the OS that ZipArchive was created on.