From 7d626816b48deaeae58a704be0fab46a36b6a07e Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 25 Apr 2023 11:04:20 +0200 Subject: [PATCH 1/4] Zip.Unix: don't hang when creating zip file from directory with named pipe. When we open the named pipe for reading, that call will block indefinitely when there is no writer to provide data. And if we manage to read data from a pipe, that data was probably meant for someone else. Rather than opening named pipes, throw an IOException indicating they are not supported. And like the named pipes, this also treat character devices, block devices, and sockets as unsupported types. --- .../Common/src/System/IO/Archiving.Utils.cs | 4 +- .../src/Resources/Strings.resx | 3 ++ .../src/System.IO.Compression.ZipFile.csproj | 2 + .../IO/Compression/ZipFile.Create.Unix.cs | 37 ++++++++++++++ .../IO/Compression/ZipFile.Create.Windows.cs | 15 ++++++ .../System/IO/Compression/ZipFile.Create.cs | 48 ++++++++++++------- .../tests/ZipFile.Unix.cs | 28 ++--------- 7 files changed, 94 insertions(+), 43 deletions(-) create mode 100644 src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.Unix.cs create mode 100644 src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.Windows.cs diff --git a/src/libraries/Common/src/System/IO/Archiving.Utils.cs b/src/libraries/Common/src/System/IO/Archiving.Utils.cs index 23cc77a30c75f..8c15f6302bce9 100644 --- a/src/libraries/Common/src/System/IO/Archiving.Utils.cs +++ b/src/libraries/Common/src/System/IO/Archiving.Utils.cs @@ -26,9 +26,9 @@ public static void EnsureCapacity(ref char[] buffer, int min) } } - public static bool IsDirEmpty(DirectoryInfo possiblyEmptyDir) + public static bool IsDirEmpty(string directoryFullName) { - using (IEnumerator enumerator = Directory.EnumerateFileSystemEntries(possiblyEmptyDir.FullName).GetEnumerator()) + using (IEnumerator enumerator = Directory.EnumerateFileSystemEntries(directoryFullName).GetEnumerator()) return !enumerator.MoveNext(); } diff --git a/src/libraries/System.IO.Compression.ZipFile/src/Resources/Strings.resx b/src/libraries/System.IO.Compression.ZipFile/src/Resources/Strings.resx index b8f50177b395e..ae33cedfbc854 100644 --- a/src/libraries/System.IO.Compression.ZipFile/src/Resources/Strings.resx +++ b/src/libraries/System.IO.Compression.ZipFile/src/Resources/Strings.resx @@ -99,4 +99,7 @@ Access to the path '{0}' is denied. + + The file '{0}' is a type of file not supported for zip archiving. + 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 94999191663fa..caa16a7c12db3 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 @@ -20,9 +20,11 @@ + + CreateEnumerableForCreate(string directoryFullPath) + => new FileSystemEnumerable<(string, CreateEntryType)>(directoryFullPath, + static (ref FileSystemEntry entry) => + { + string fullPath = entry.ToFullPath(); + + int type; + if (entry.IsDirectory) // entry is a directory, or a link to a directory. + { + type = Interop.Sys.FileTypes.S_IFDIR; + } + else + { + // Use 'stat' to follow links. + Interop.CheckIo(Interop.Sys.Stat(fullPath, out Interop.Sys.FileStatus status), fullPath); + type = (status.Mode & Interop.Sys.FileTypes.S_IFMT); + } + + return type switch + { + Interop.Sys.FileTypes.S_IFREG => (fullPath, CreateEntryType.File), + Interop.Sys.FileTypes.S_IFDIR => (fullPath, CreateEntryType.Directory), + _ => (fullPath, CreateEntryType.Unsupported) + }; + }, + new EnumerationOptions { RecurseSubdirectories = true, AttributesToSkip = 0, IgnoreInaccessible = false }); + } +} diff --git a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.Windows.cs b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.Windows.cs new file mode 100644 index 0000000000000..fb36a47353937 --- /dev/null +++ b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.Windows.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO.Enumeration; + +namespace System.IO.Compression +{ + public static partial class ZipFile + { + private static FileSystemEnumerable<(string, CreateEntryType)> CreateEnumerableForCreate(string directoryFullPath) + => new FileSystemEnumerable<(string, CreateEntryType)>(directoryFullPath, + static (ref FileSystemEntry entry) => (entry.ToFullPath(), entry.IsDirectory ? CreateEntryType.Directory : CreateEntryType.File), + new EnumerationOptions { RecurseSubdirectories = true, AttributesToSkip = 0, IgnoreInaccessible = false }); + } +} diff --git a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.cs b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.cs index 70894d2b74f7e..4f9915e7540ac 100644 --- a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.cs +++ b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Text; +using System.IO.Enumeration; namespace System.IO.Compression { @@ -375,26 +376,34 @@ private static void DoCreateFromDirectory(string sourceDirectoryName, string des if (includeBaseDirectory && di.Parent != null) basePath = di.Parent.FullName; - foreach (FileSystemInfo file in di.EnumerateFileSystemInfos("*", SearchOption.AllDirectories)) + FileSystemEnumerable<(string, CreateEntryType)> fse = CreateEnumerableForCreate(di.FullName); + + foreach ((string fullPath, CreateEntryType type) in fse) { directoryIsEmpty = false; - if (file is FileInfo) - { - // Create entry for file: - string entryName = ArchivingUtils.EntryFromPath(file.FullName.AsSpan(basePath.Length)); - ZipFileExtensions.DoCreateEntryFromFile(archive, file.FullName, entryName, compressionLevel); - } - else + switch (type) { - // Entry marking an empty dir: - if (file is DirectoryInfo possiblyEmpty && ArchivingUtils.IsDirEmpty(possiblyEmpty)) - { - // FullName never returns a directory separator character on the end, - // but Zip archives require it to specify an explicit directory: - string entryName = ArchivingUtils.EntryFromPath(file.FullName.AsSpan(basePath.Length), appendPathSeparator: true); - archive.CreateEntry(entryName); - } + case CreateEntryType.File: + { + // Create entry for file: + string entryName = ArchivingUtils.EntryFromPath(fullPath.AsSpan(basePath.Length)); + ZipFileExtensions.DoCreateEntryFromFile(archive, fullPath, entryName, compressionLevel); + } + break; + case CreateEntryType.Directory: + if (ArchivingUtils.IsDirEmpty(fullPath)) + { + // Create entry marking an empty dir: + // FullName never returns a directory separator character on the end, + // but Zip archives require it to specify an explicit directory: + string entryName = ArchivingUtils.EntryFromPath(fullPath.AsSpan(basePath.Length), appendPathSeparator: true); + archive.CreateEntry(entryName); + } + break; + case CreateEntryType.Unsupported: + default: + throw new IOException(SR.Format(SR.ZipUnsupportedFile, fullPath)); } } @@ -403,5 +412,12 @@ private static void DoCreateFromDirectory(string sourceDirectoryName, string des archive.CreateEntry(ArchivingUtils.EntryFromPath(di.Name, appendPathSeparator: true)); } } + + private enum CreateEntryType + { + File, + Directory, + Unsupported + } } } diff --git a/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs b/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs index 25c41ead025d1..7ab70b65fca66 100644 --- a/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs +++ b/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs @@ -184,9 +184,9 @@ public void UnixExtractFilePermissionsCompat(string zipName, string expectedPerm } [Fact] - [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.tvOS & ~TestPlatforms.iOS)] + [PlatformSpecific(TestPlatforms.AnyUnix)] [SkipOnPlatform(TestPlatforms.LinuxBionic, "Bionic is not normal Linux, has no normal file permissions")] - public async Task CanZipNamedPipe() + public void ZipNamedPipeIsNotSupported() { string destPath = Path.Combine(TestDirectory, "dest.zip"); @@ -195,29 +195,7 @@ public async Task CanZipNamedPipe() Directory.CreateDirectory(subFolderPath); // mandatory before calling mkfifo Assert.Equal(0, mkfifo(fifoPath, 438 /* 666 in octal */)); - byte[] contentBytes = { 1, 2, 3, 4, 5 }; - - await Task.WhenAll( - Task.Run(() => - { - using FileStream fs = new (fifoPath, FileMode.Open, FileAccess.Write, FileShare.Read, bufferSize: 0); - foreach (byte content in contentBytes) - { - fs.WriteByte(content); - } - }), - Task.Run(() => - { - ZipFile.CreateFromDirectory(subFolderPath, destPath); - - using ZipArchive zippedFolder = ZipFile.OpenRead(destPath); - using Stream unzippedPipe = zippedFolder.Entries.Single().Open(); - - byte[] readBytes = new byte[contentBytes.Length]; - Assert.Equal(contentBytes.Length, unzippedPipe.Read(readBytes)); - Assert.Equal(contentBytes, readBytes); - Assert.Equal(0, unzippedPipe.Read(readBytes)); // EOF - })); + Assert.Throws(() => ZipFile.CreateFromDirectory(subFolderPath, destPath)); } private static string GetExpectedPermissions(string expectedPermissions) From 144a0b40fbe826a806afbee2996d81e28d99f702 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 25 Apr 2023 13:17:21 +0200 Subject: [PATCH 2/4] Skip test on browser. --- .../System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs b/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs index 7ab70b65fca66..3dca3b7ff6c7f 100644 --- a/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs +++ b/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs @@ -184,7 +184,7 @@ public void UnixExtractFilePermissionsCompat(string zipName, string expectedPerm } [Fact] - [PlatformSpecific(TestPlatforms.AnyUnix)] + [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)] // browser doesn't have libc mkfifo. [SkipOnPlatform(TestPlatforms.LinuxBionic, "Bionic is not normal Linux, has no normal file permissions")] public void ZipNamedPipeIsNotSupported() { From 54818026f345fddc66fd9386eef05f2e43390c57 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 4 May 2023 08:51:34 +0200 Subject: [PATCH 3/4] Tar,Zip: update unsupported file type error message. --- src/libraries/System.Formats.Tar/src/Resources/Strings.resx | 2 +- .../System.IO.Compression.ZipFile/src/Resources/Strings.resx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/Resources/Strings.resx b/src/libraries/System.Formats.Tar/src/Resources/Strings.resx index 4da0897395633..0ece0c6c56152 100644 --- a/src/libraries/System.Formats.Tar/src/Resources/Strings.resx +++ b/src/libraries/System.Formats.Tar/src/Resources/Strings.resx @@ -244,7 +244,7 @@ A metadata entry of type '{0}' was unexpectedly found after a metadata entry of type '{1}'. - The file '{0}' is a type of file not supported for tar archiving. + The file type of '{0}' is not supported for tar archiving. Access to the path is denied. diff --git a/src/libraries/System.IO.Compression.ZipFile/src/Resources/Strings.resx b/src/libraries/System.IO.Compression.ZipFile/src/Resources/Strings.resx index ae33cedfbc854..3770da259211a 100644 --- a/src/libraries/System.IO.Compression.ZipFile/src/Resources/Strings.resx +++ b/src/libraries/System.IO.Compression.ZipFile/src/Resources/Strings.resx @@ -100,6 +100,6 @@ Access to the path '{0}' is denied. - The file '{0}' is a type of file not supported for zip archiving. + The file type of '{0}' is not supported for zip archiving. From bb5c8ed73006ef26a3d229a048c2eab1c6b00bad Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 9 May 2023 09:14:57 +0200 Subject: [PATCH 4/4] Skip tests on tvOS/iOS. --- .../System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs b/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs index 3dca3b7ff6c7f..b2e59d9270d97 100644 --- a/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs +++ b/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs @@ -184,7 +184,7 @@ public void UnixExtractFilePermissionsCompat(string zipName, string expectedPerm } [Fact] - [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)] // browser doesn't have libc mkfifo. + [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.tvOS & ~TestPlatforms.iOS)] // browser doesn't have libc mkfifo. tvOS/iOS return an error for mkfifo. [SkipOnPlatform(TestPlatforms.LinuxBionic, "Bionic is not normal Linux, has no normal file permissions")] public void ZipNamedPipeIsNotSupported() {