Skip to content

Commit e855969

Browse files
Copilotrzikm
andcommitted
Address PR feedback: fix bounds checking, use localized strings, refactor test
Co-authored-by: rzikm <[email protected]>
1 parent a6c91cf commit e855969

File tree

3 files changed

+29
-38
lines changed

3 files changed

+29
-38
lines changed

src/libraries/System.IO.Compression/src/Resources/Strings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,4 +296,7 @@
296296
<data name="InvalidOffsetToZip64EOCD" xml:space="preserve">
297297
<value>Invalid offset to the Zip64 End of Central Directory record.</value>
298298
</data>
299+
<data name="IO_SeekBeforeBegin" xml:space="preserve">
300+
<value>An attempt was made to move the position before the beginning of the stream.</value>
301+
</data>
299302
</root>

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,6 @@ public override long Position
263263
throw new NotSupportedException(SR.SeekingNotSupported);
264264

265265
ArgumentOutOfRangeException.ThrowIfNegative(value);
266-
ArgumentOutOfRangeException.ThrowIfGreaterThan(value, _endInSuperStream - _startInSuperStream);
267266

268267
_positionInSuperStream = _startInSuperStream + value;
269268
}
@@ -385,10 +384,7 @@ public override long Seek(long offset, SeekOrigin origin)
385384
};
386385

387386
if (newPositionInSuperStream < _startInSuperStream)
388-
throw new IOException("An attempt was made to move the position before the beginning of the stream.");
389-
390-
if (newPositionInSuperStream > _endInSuperStream)
391-
throw new IOException("An attempt was made to move the position beyond the end of the stream.");
387+
throw new IOException(SR.IO_SeekBeforeBegin);
392388

393389
_positionInSuperStream = newPositionInSuperStream;
394390

src/libraries/System.IO.Compression/tests/ZipArchive/zip_ReadTests.cs

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,17 @@ public static IEnumerable<object[]> Get_TestPartialReads_Data()
9292
}
9393
}
9494

95+
public static IEnumerable<object[]> Get_BooleanCombinations_Data()
96+
{
97+
foreach (bool async in _bools)
98+
{
99+
foreach (bool useSeekMethod in _bools)
100+
{
101+
yield return new object[] { async, useSeekMethod };
102+
}
103+
}
104+
}
105+
95106
[Theory]
96107
[MemberData(nameof(Get_TestPartialReads_Data))]
97108
public static async Task TestPartialReads(string zipFile, string zipFolder, bool async)
@@ -667,8 +678,8 @@ public static async Task ReadStreamSeekOps(bool async)
667678
}
668679

669680
[Theory]
670-
[MemberData(nameof(Get_Booleans_Data))]
671-
public static async Task ReadEntryContentTwice(bool async)
681+
[MemberData(nameof(Get_BooleanCombinations_Data))]
682+
public static async Task ReadEntryContentTwice(bool async, bool useSeekMethod)
672683
{
673684
// Create a ZIP archive with stored (uncompressed) entries to test reading content twice
674685
using (var ms = new MemoryStream())
@@ -697,17 +708,24 @@ public static async Task ReadEntryContentTwice(bool async)
697708
// For stored entries, SubReadStream should be seekable when underlying stream is seekable
698709
Assert.True(s.CanSeek, $"SubReadStream should be seekable for stored entry '{e.FullName}' when underlying stream is seekable");
699710

700-
// Test 1: Read content using Seek method
711+
// Read content first time
701712
byte[] firstRead = new byte[e.Length];
702713
int bytesRead1 = s.Read(firstRead, 0, (int)e.Length);
703714
Assert.Equal(e.Length, bytesRead1);
704715

705-
// Seek back to beginning using Seek method
706-
long pos = s.Seek(0, SeekOrigin.Begin);
707-
Assert.Equal(0, pos);
716+
// Rewind to beginning using specified method
717+
if (useSeekMethod)
718+
{
719+
long pos = s.Seek(0, SeekOrigin.Begin);
720+
Assert.Equal(0, pos);
721+
}
722+
else
723+
{
724+
s.Position = 0;
725+
}
708726
Assert.Equal(0, s.Position);
709727

710-
// Read again using Seek method reset
728+
// Read content second time
711729
byte[] secondRead = new byte[e.Length];
712730
int bytesRead2 = s.Read(secondRead, 0, (int)e.Length);
713731
Assert.Equal(e.Length, bytesRead2);
@@ -717,38 +735,12 @@ public static async Task ReadEntryContentTwice(bool async)
717735
Assert.Equal(testData, firstRead);
718736
Assert.Equal(testData, secondRead);
719737

720-
// Test 2: Read content using Position setter
721-
s.Position = 0;
722-
byte[] thirdRead = new byte[e.Length];
723-
int bytesRead3 = s.Read(thirdRead, 0, (int)e.Length);
724-
Assert.Equal(e.Length, bytesRead3);
725-
726-
// Reset using Position setter
727-
s.Position = 0;
728-
Assert.Equal(0, s.Position);
729-
730-
// Read again using Position setter reset
731-
byte[] fourthRead = new byte[e.Length];
732-
int bytesRead4 = s.Read(fourthRead, 0, (int)e.Length);
733-
Assert.Equal(e.Length, bytesRead4);
734-
735-
// Compare the content - should be identical
736-
Assert.Equal(thirdRead, fourthRead);
737-
Assert.Equal(testData, thirdRead);
738-
Assert.Equal(testData, fourthRead);
739-
740-
// All reads should be identical
741-
Assert.Equal(firstRead, thirdRead);
742-
Assert.Equal(secondRead, fourthRead);
743-
744738
await DisposeStream(async, s);
745739
}
746740
}
747741
}
748742
}
749743

750-
751-
752744
private static byte[] ReverseCentralDirectoryEntries(byte[] zipFile)
753745
{
754746
byte[] destinationBuffer = new byte[zipFile.Length];

0 commit comments

Comments
 (0)