Address suggestions

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Buffers;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Text;
@@ -236,7 +237,6 @@ internal static byte[] GetEncodedTruncatedBytesFromString(string? text, Encoding
                 return bytes[0..totalCodePoints];
             }
 
-
             bytes = encoding.GetBytes(text);
             return maxBytes < bytes.Length ? bytes[0..maxBytes] : bytes;
         }

src/libraries/System.IO.Compression/tests/System.IO.Compression.Tests.csproj

@@ -27,6 +27,7 @@
     <Compile Include="ZipArchive\zip_ReadTests.cs" />
     <Compile Include="ZipArchive\zip_UpdateTests.cs" />
     <Compile Include="ZipArchive\zip_UpdateTests.Comments.cs" />
+    <Compile Include="ZipTestHelper.cs" />
     <Compile Include="$(CommonTestPath)System\IO\PathFeatures.cs" Link="Common\System\IO\PathFeatures.cs" />
     <Compile Include="$(CommonTestPath)System\IO\Compression\CRC.cs" Link="Common\System\IO\Compression\CRC.cs" />
     <Compile Include="$(CommonTestPath)System\IO\Compression\CompressionStreamTestBase.cs" Link="Common\System\IO\Compression\CompressionStreamTestBase.cs" />
@@ -35,7 +36,6 @@
     <Compile Include="$(CommonTestPath)System\IO\Compression\LocalMemoryStream.cs" Link="Common\System\IO\Compression\LocalMemoryStream.cs" />
     <Compile Include="$(CommonTestPath)System\IO\Compression\StreamHelpers.cs" Link="Common\System\IO\Compression\StreamHelpers.cs" />
     <Compile Include="$(CommonTestPath)System\IO\TempFile.cs" Link="Common\System\IO\TempFile.cs" />
-    <Compile Include="$(CommonTestPath)System\IO\Compression\ZipTestHelper.cs" Link="Common\System\IO\Compression\ZipTestHelper.cs" />
     <Compile Include="$(CommonPath)System\Threading\Tasks\TaskToApm.cs" Link="Common\System\Threading\Tasks\TaskToApm.cs" />
     <Compile Include="$(CommonTestPath)System\IO\ConnectedStreams.cs" Link="Common\System\IO\ConnectedStreams.cs" />
     <Compile Include="$(CommonPath)System\Net\MultiArrayBuffer.cs" Link="ProductionCode\Common\System\Net\MultiArrayBuffer.cs" />

src/libraries/System.IO.Compression/tests/ZipArchive/zip_CreateTests.Comments.cs

@@ -11,33 +11,33 @@ public partial class zip_CreateTests : ZipFileTestBase
         [Theory]
         [MemberData(nameof(Utf8Comment_Data))]
         public static void Create_Comment_AsciiEntryName_NullEncoding(string originalComment, string expectedComment) =>
-            Create_Comment_EntryName_Encoding_Internal("file.txt", originalComment, expectedComment, null);
+            Create_Comment_EntryName_Encoding_Internal(AsciiFileName, originalComment, expectedComment, null);
 
         [Theory]
         [MemberData(nameof(Utf8Comment_Data))]
         public static void Create_Comment_AsciiEntryName_Utf8Encoding(string originalComment, string expectedComment) =>
-            Create_Comment_EntryName_Encoding_Internal("file.txt", originalComment, expectedComment, Encoding.UTF8);
+            Create_Comment_EntryName_Encoding_Internal(AsciiFileName, originalComment, expectedComment, Encoding.UTF8);
 
         [Theory]
         [MemberData(nameof(Latin1Comment_Data))]
         public static void Create_Comment_AsciiEntryName_Latin1Encoding(string originalComment, string expectedComment) =>
-            Create_Comment_EntryName_Encoding_Internal("file.txt", originalComment, expectedComment, Encoding.Latin1);
+            Create_Comment_EntryName_Encoding_Internal(AsciiFileName, originalComment, expectedComment, Encoding.Latin1);
 
         [Theory]
         [MemberData(nameof(Utf8Comment_Data))]
         public static void Create_Comment_Utf8EntryName_NullEncoding(string originalComment, string expectedComment) =>
-            Create_Comment_EntryName_Encoding_Internal($"{SmileyEmoji}.txt", originalComment, expectedComment, null);
+            Create_Comment_EntryName_Encoding_Internal(Utf8FileName, originalComment, expectedComment, null);
 
         [Theory]
         [MemberData(nameof(Utf8Comment_Data))]
         public static void Create_Comment_Utf8EntryName_Utf8Encoding(string originalComment, string expectedComment) =>
-            Create_Comment_EntryName_Encoding_Internal($"{SmileyEmoji}.txt", originalComment, expectedComment, Encoding.UTF8);
+            Create_Comment_EntryName_Encoding_Internal(Utf8FileName, originalComment, expectedComment, Encoding.UTF8);
 
         [Theory]
         [MemberData(nameof(Latin1Comment_Data))]
         public static void Create_Comment_Utf8EntryName_Latin1Encoding(string originalComment, string expectedComment) =>
             // Emoji not supported by latin1
-            Create_Comment_EntryName_Encoding_Internal($"{LowerCaseOUmlautChar}.txt", originalComment, expectedComment, Encoding.Latin1);
+            Create_Comment_EntryName_Encoding_Internal(Utf8AndLatin1FileName, originalComment, expectedComment, Encoding.Latin1);
 
         private static void Create_Comment_EntryName_Encoding_Internal(string entryName, string originalComment, string expectedComment, Encoding encoding)
         {

src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.Comments.cs

@@ -13,46 +13,45 @@ public partial class zip_UpdateTests : ZipFileTestBase
         [Theory]
         [MemberData(nameof(Utf8Comment_Data))]
         public static void Update_Comment_AsciiEntryName_NullEncoding(string originalComment, string expectedComment) =>
-            Update_Comment_EntryName_Encoding_Internal("file.txt",
+            Update_Comment_EntryName_Encoding_Internal(AsciiFileName,
                 originalComment, expectedComment, null,
-                new string('a', ushort.MaxValue - 1) + $"{CopyrightChar}", new string('a', ushort.MaxValue - 1));
+                ALettersUShortMaxValueMinusOneAndCopyRightChar, ALettersUShortMaxValueMinusOne);
 
         [Theory]
         [MemberData(nameof(Utf8Comment_Data))]
         public static void Update_Comment_AsciiEntryName_Utf8Encoding(string originalComment, string expectedComment) =>
-            Update_Comment_EntryName_Encoding_Internal("file.txt",
+            Update_Comment_EntryName_Encoding_Internal(AsciiFileName,
                 originalComment, expectedComment, Encoding.UTF8,
-                new string('a', ushort.MaxValue - 1) + $"{CopyrightChar}", new string('a', ushort.MaxValue - 1));
+                ALettersUShortMaxValueMinusOneAndCopyRightChar, ALettersUShortMaxValueMinusOne);
 
         [Theory]
         [MemberData(nameof(Latin1Comment_Data))]
         public static void Update_Comment_AsciiEntryName_Latin1Encoding(string originalComment, string expectedComment) =>
-            Update_Comment_EntryName_Encoding_Internal("file.txt",
+            Update_Comment_EntryName_Encoding_Internal(AsciiFileName,
                 originalComment, expectedComment, Encoding.Latin1,
-                new string('a', ushort.MaxValue - 1) + $"{CopyrightChar}{CopyrightChar}", new string('a', ushort.MaxValue - 1) + $"{CopyrightChar}");
+                ALettersUShortMaxValueMinusOneAndTwoCopyRightChars, ALettersUShortMaxValueMinusOneAndCopyRightChar);
 
         [Theory]
         [MemberData(nameof(Utf8Comment_Data))]
         public static void Update_Comment_Utf8EntryName_NullEncoding(string originalComment, string expectedComment) =>
-            Update_Comment_EntryName_Encoding_Internal($"{SmileyEmoji}.txt",
+            Update_Comment_EntryName_Encoding_Internal(Utf8FileName,
                 originalComment, expectedComment, null,
-                new string('a', ushort.MaxValue - 1) + $"{CopyrightChar}", new string('a', ushort.MaxValue - 1));
+                ALettersUShortMaxValueMinusOneAndCopyRightChar, ALettersUShortMaxValueMinusOne);
 
         [Theory]
         [MemberData(nameof(Utf8Comment_Data))]
         public static void Update_Comment_Utf8EntryName_Utf8Encoding(string originalComment, string expectedComment) =>
-            Update_Comment_EntryName_Encoding_Internal($"{SmileyEmoji}.txt",
+            Update_Comment_EntryName_Encoding_Internal(Utf8FileName,
                 originalComment, expectedComment, Encoding.UTF8,
-                new string('a', ushort.MaxValue - 1) + $"{CopyrightChar}", new string('a', ushort.MaxValue - 1));
+                ALettersUShortMaxValueMinusOneAndCopyRightChar, ALettersUShortMaxValueMinusOne);
 
         [Theory]
         [MemberData(nameof(Latin1Comment_Data))]
         public static void Update_Comment_Utf8EntryName_Latin1Encoding(string originalComment, string expectedComment) =>
-            // Emoji not supported by latin1
-            Update_Comment_EntryName_Encoding_Internal($"{LowerCaseOUmlautChar}.txt",
+            // Emoji is not supported/detected in latin1
+            Update_Comment_EntryName_Encoding_Internal(Utf8AndLatin1FileName,
                 originalComment, expectedComment, Encoding.Latin1,
-                new string('a', ushort.MaxValue - 1) + $"{CopyrightChar}{CopyrightChar}", new string('a', ushort.MaxValue - 1) + $"{CopyrightChar}");
-
+                ALettersUShortMaxValueMinusOneAndTwoCopyRightChars, ALettersUShortMaxValueMinusOneAndCopyRightChar);
 
         private static void Update_Comment_EntryName_Encoding_Internal(string entryName,
             string originalCreateComment, string expectedCreateComment, Encoding encoding,

src/libraries/Common/tests/System/IO/Compression/ZipTestHelper.cs → src/libraries/System.IO.Compression/tests/ZipTestHelper.cs

@@ -384,9 +384,18 @@ internal static void AddEntry(ZipArchive archive, string name, string contents,
             }
         }
 
-        protected const string SmileyEmoji = "\ud83d\ude04";
-        protected const string LowerCaseOUmlautChar = "\u00F6";
-        protected const string CopyrightChar = "\u00A9";
+        protected const string Utf8SmileyEmoji = "\ud83d\ude04";
+        protected const string Utf8LowerCaseOUmlautChar = "\u00F6";
+        protected const string Utf8CopyrightChar = "\u00A9";
+        protected const string AsciiFileName = "file.txt";
+        // The o with umlaut is a character that exists in both latin1 and utf8
+        protected const string Utf8AndLatin1FileName = $"{Utf8LowerCaseOUmlautChar}.txt";
+        // emojis only make sense in utf8
+        protected const string Utf8FileName = $"{Utf8SmileyEmoji}.txt";
+        protected static readonly string ALettersUShortMaxValueMinusOne = new string('a', ushort.MaxValue - 1);
+        protected static readonly string ALettersUShortMaxValue = ALettersUShortMaxValueMinusOne + 'a';
+        protected static readonly string ALettersUShortMaxValueMinusOneAndCopyRightChar = ALettersUShortMaxValueMinusOne + Utf8CopyrightChar;
+        protected static readonly string ALettersUShortMaxValueMinusOneAndTwoCopyRightChars = ALettersUShortMaxValueMinusOneAndCopyRightChar + Utf8CopyrightChar;
 
         // Returns pairs that are returned the same way by Utf8 and Latin1
         // Returns: originalComment, expectedComment
@@ -395,28 +404,31 @@ private static IEnumerable<object[]> SharedComment_Data()
             yield return new object[] { null, string.Empty };
             yield return new object[] { string.Empty, string.Empty };
             yield return new object[] { "a", "a" };
-            yield return new object[] { LowerCaseOUmlautChar, LowerCaseOUmlautChar };
+            yield return new object[] { Utf8LowerCaseOUmlautChar, Utf8LowerCaseOUmlautChar };
         }
 
         // Returns pairs as expected by Utf8
         // Returns: originalComment, expectedComment
         public static IEnumerable<object[]> Utf8Comment_Data()
         {
-            string asciiExpectedExactMaxLength = new('a', ushort.MaxValue);
-            string asciiOriginalOverMaxLength = asciiExpectedExactMaxLength + "aaa";
+            string asciiOriginalOverMaxLength = ALettersUShortMaxValue + "aaa";
 
             // A smiley emoji code point consists of two characters,
             // meaning the whole emoji should be fully truncated
-            string utf8ExpectedJustALetters = new('a', ushort.MaxValue - 1);
-            string utf8OriginalALettersAndOneEmoji = utf8ExpectedJustALetters + SmileyEmoji;
+            string utf8OriginalALettersAndOneEmojiDoesNotFit = ALettersUShortMaxValueMinusOne + Utf8SmileyEmoji;
+
+            // A smiley emoji code point consists of two characters,
+            // so it should not be truncated if it's the last character and the total length is not over the limit.
+            string utf8OriginalALettersAndOneEmojiFits = "aaaaa" + Utf8SmileyEmoji;
+
+            yield return new object[] { asciiOriginalOverMaxLength, ALettersUShortMaxValue };
+            yield return new object[] { utf8OriginalALettersAndOneEmojiDoesNotFit, ALettersUShortMaxValueMinusOne };
+            yield return new object[] { utf8OriginalALettersAndOneEmojiFits, utf8OriginalALettersAndOneEmojiFits };
 
             foreach (object[] e in SharedComment_Data())
             {
                 yield return e;
             }
-
-            yield return new object[] { asciiOriginalOverMaxLength, asciiExpectedExactMaxLength };
-            yield return new object[] { utf8OriginalALettersAndOneEmoji, utf8ExpectedJustALetters };
         }
 
         // Returns pairs as expected by Latin1
@@ -425,15 +437,15 @@ public static IEnumerable<object[]> Latin1Comment_Data()
         {
             // In Latin1, all characters are exactly 1 byte
 
-            string latin1ExpectedALettersAndOneOUmlaut = new string('a', ushort.MaxValue - 1) + LowerCaseOUmlautChar;
-            string latin1OriginalALettersAndTwoOUmlauts = latin1ExpectedALettersAndOneOUmlaut + LowerCaseOUmlautChar;
+            string latin1ExpectedALettersAndOneOUmlaut = ALettersUShortMaxValueMinusOne + Utf8LowerCaseOUmlautChar;
+            string latin1OriginalALettersAndTwoOUmlauts = latin1ExpectedALettersAndOneOUmlaut + Utf8LowerCaseOUmlautChar;
+
+            yield return new object[] { latin1OriginalALettersAndTwoOUmlauts, latin1ExpectedALettersAndOneOUmlaut };
 
             foreach (object[] e in SharedComment_Data())
             {
                 yield return e;
             }
-
-            yield return new object[] { latin1OriginalALettersAndTwoOUmlauts, latin1ExpectedALettersAndOneOUmlaut };
         }
     }
 }