Illegal characters in file path

ReleaseHistory.md

@@ -5,6 +5,7 @@
 * DEP: Remove spurious references to `System.Collections.Immutable`.
 * DEP: Update `Microsoft.Data.SqlClient` reference from 2.1.2 to 2.1.7 in `WorkItems` and `Sarif.Multitool.Library` to resolve [CVE-2024-0056](https://github.com/advisories/GHSA-98g6-xh36-x2p7).
 * DEP: Update `System.Data.SqlClient` reference from 4.8.5 to 4.8.6 in `WorkItems` to resolve [CVE-2024-0056](https://github.com/advisories/GHSA-98g6-xh36-x2p7).
+* BUG: Eliminate `ArgumentException: Illegal characters in path` in `ZipArchiveArtifact.GetArtifactData` when the archive entry contains an illegal file name or path character.
 * BUG: Update `Stack.Create` method to populate missing `PhysicalLocation` instances when stack frames reference relative file paths.
 * BUG: Fix `UnsupportedOperationException` in `ZipArchiveArtifact`.
 * BUG: Fix `MultithreadedAnalyzeCommandBase` to return rich return code with the `--rich-return-code` option.

src/Sarif/FileSystem.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Diagnostics.Contracts;
 using System.IO;
 using System.Reflection;
 using System.Text;
@@ -455,5 +456,41 @@ public string PathGetFileNameWithoutExtension(string path)
         {
             return Path.GetFileNameWithoutExtension(path);
         }
+
+        // Returns the extension of the given path. The returned value includes the
+        // period (".") character of the extension except when you have a terminal period when you get String.Empty, such as ".exe" or
+        // ".cpp". The returned value is null if the given path is
+        // null or if the given path does not include an extension.
+        //
+        public static string SafePathGetExtension(string path)
+        {
+            // https://referencesource.microsoft.com/#mscorlib/system/io/path.cs,f424e433705aeb09
+            if (path == null)
+            {
+                return null;
+            }
+
+            int length = path.Length;
+            for (int i = length; --i >= 0;)
+            {
+                char ch = path[i];
+                if (ch == '.')
+                {
+                    if (i != length - 1)
+                    {
+                        return path.Substring(i, length - i);
+                    }
+                    else
+                    {
+                        return string.Empty;
+                    }
+                }
+                if (ch == Path.DirectorySeparatorChar || ch == Path.AltDirectorySeparatorChar || ch == Path.VolumeSeparatorChar)
+                {
+                    break;
+                }
+            }
+            return string.Empty;
+        }
     }
 }

src/Sarif/ZipArchiveArtifact.cs

@@ -9,7 +9,6 @@
 
 namespace Microsoft.CodeAnalysis.Sarif
 {
-
     public class ZipArchiveArtifact : IEnumeratedArtifact
     {
         private readonly ISet<string> binaryExtensions;
@@ -91,7 +90,7 @@ public byte[] Bytes
                 {
                     if (this.contents == null && this.bytes == null)
                     {
-                        string extension = Path.GetExtension(Uri.ToString());
+                        string extension = FileSystem.SafePathGetExtension(Uri.ToString());
                         if (this.binaryExtensions.Contains(extension))
                         {
                             // The underlying System.IO.Compression.DeflateStream throws on reads to get_Length.

src/Test.UnitTests.Sarif/ArtifactProviderTests.cs

@@ -22,7 +22,7 @@ public class ArtifactProviderTests
         [Fact]
         public void MultithreadedZipArchiveArtifactProvider_RetrieveSizeInBytesBeforeRetrievingContents()
         {
-            string entryContents = $"{Guid.NewGuid}";
+            string entryContents = $"{Guid.NewGuid()}";
             ZipArchive zip = CreateZipArchiveWithTextContents("test.txt", entryContents);
             var artifactProvider = new MultithreadedZipArchiveArtifactProvider(zip, FileSystem.Instance);
 
@@ -125,7 +125,7 @@ private static ZipArchive CreateZipArchiveWithBinaryContents(string fileName, by
             using (var populateArchive = new ZipArchive(stream, ZipArchiveMode.Create, leaveOpen: true))
             {
                 ZipArchiveEntry entry = populateArchive.CreateEntry(fileName, CompressionLevel.NoCompression);
-                entry.Open().Write(bytes);
+                entry.Open().Write(bytes, 0, bytes.Length);
             }
             stream.Flush();
             stream.Position = 0;

src/Test.UnitTests.Sarif/EnumeratedArtifactTests.cs

@@ -40,7 +40,7 @@ public void EnumeratedArtifact_TextFile_OnDisk()
             using var tempFile = new TempFile();
             string filePath = tempFile.Name;
 
-            File.WriteAllText(filePath, $"{Guid.NewGuid}");
+            File.WriteAllText(filePath, $"{Guid.NewGuid()}");
 
             var artifact = new EnumeratedArtifact(new FileSystem())
             {
@@ -83,7 +83,7 @@ public void EnumeratedArtifact_TextFile_SeekableStream()
             using var tempFile = new TempFile();
             string filePath = tempFile.Name;
 
-            File.WriteAllText(filePath, $"{Guid.NewGuid}");
+            File.WriteAllText(filePath, $"{Guid.NewGuid()}");
 
             var artifact = new EnumeratedArtifact(new FileSystem())
             {

src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj

@@ -8,7 +8,7 @@
   <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), build.props))\build.props" />
 
   <PropertyGroup>
-    <TargetFrameworks>net6.0;</TargetFrameworks>
+    <TargetFrameworks>net472;</TargetFrameworks>
     <IsPackable>false</IsPackable>
     <IsTestProject>true</IsTestProject>
     <RootNamespace>Test.UnitTests.Sarif</RootNamespace>
@@ -356,4 +356,10 @@
   <ItemGroup>
     <EmbeddedResource Include="TestData\FileEncoding\Utf8.txt" />
   </ItemGroup>
+
+  <ItemGroup>
+    <Reference Include="System.Web">
+      <HintPath>C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.2\System.Web.dll</HintPath>
+    </Reference>
+  </ItemGroup>
 </Project>

src/Test.UnitTests.Sarif/ZipArchiveArtifactTests.cs

@@ -18,6 +18,20 @@ namespace Test.UnitTests.Sarif
 {
     public class ZipArchiveArtifactTests
     {
+        [Fact]
+        public void ZipArchiveArtifact_IllegalFileAndPathCharacters()
+        {
+            string text = $"{Guid.NewGuid()}";
+            byte[] contents = Encoding.UTF8.GetBytes(text);
+            string filePath = $"{Path.GetInvalidPathChars()[0]}{Path.GetInvalidFileNameChars()[1]}MyZippedFile.txt";
+            ZipArchive archive = EnumeratedArtifactTests.CreateZipArchive(filePath, contents);
+
+            foreach (IEnumeratedArtifact entry in new MultithreadedZipArchiveArtifactProvider(archive, new FileSystem()).Artifacts)
+            {
+                entry.IsBinary.Should().BeFalse();
+                entry.Contents.Should().BeEquivalentTo(text);
+            }
+        }
         [Fact]
         public void ZipArchiveArtifact_BytesAndContentsCannotBeSet()
         {
@@ -66,7 +80,7 @@ public void ZipArchiveArtifact_NonNullArchiveAndEntryAreRequired()
         }
 
         [Fact]
-        public void ZipArchiveArtifact_MixedTxtAndBinaryZipEntry()
+        public void ZipArchiveArtifact_MixedTextAndBinaryZipEntry()
         {
             string textData = Guid.NewGuid().ToString();