Remove BinaryReader and BinaryWriter references from ZipArchive

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

@@ -18,6 +18,8 @@
     <Compile Include="$(SharedOpenSourcePath)System\IO\Compression\ZipArchiveEntry.cs" />
     <Compile Include="$(SharedOpenSourcePath)System\IO\Compression\ZipArchiveMode.cs" />
     <Compile Include="$(SharedOpenSourcePath)System\IO\Compression\ZipBlocks.cs" />
+    <Compile Include="$(SharedOpenSourcePath)System\IO\Compression\ZipBlocks.FieldLengths.cs" />
+    <Compile Include="$(SharedOpenSourcePath)System\IO\Compression\ZipBlocks.FieldLocations.cs" />
     <Compile Include="$(SharedOpenSourcePath)System\IO\Compression\ZipCustomStreams.cs" />
     <Compile Include="$(SharedOpenSourcePath)System\IO\Compression\ZipHelper.cs" />
     <Compile Include="$(SharedOpenSourcePath)System\IO\Compression\ZipVersion.cs" />

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

@@ -8,6 +8,7 @@
 using System.Collections.ObjectModel;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
+using System.Runtime.InteropServices;
 using System.Text;
 
 namespace System.IO.Compression
@@ -16,16 +17,15 @@ public class ZipArchive : IDisposable
     {
         private readonly Stream _archiveStream;
         private ZipArchiveEntry? _archiveStreamOwner;
-        private readonly BinaryReader? _archiveReader;
         private readonly ZipArchiveMode _mode;
         private readonly List<ZipArchiveEntry> _entries;
         private readonly ReadOnlyCollection<ZipArchiveEntry> _entriesCollection;
         private readonly Dictionary<string, ZipArchiveEntry> _entriesDictionary;
         private bool _readEntries;
         private readonly bool _leaveOpen;
-        private long _centralDirectoryStart; //only valid after ReadCentralDirectory
+        private long _centralDirectoryStart; // only valid after ReadCentralDirectory
         private bool _isDisposed;
-        private uint _numberOfThisDisk; //only valid after ReadCentralDirectory
+        private uint _numberOfThisDisk; // only valid after ReadCentralDirectory
         private long _expectedNumberOfEntries;
         private readonly Stream? _backingStream;
         private byte[] _archiveComment;
@@ -161,10 +161,6 @@ public ZipArchive(Stream stream, ZipArchiveMode mode, bool leaveOpen, Encoding?
                 else
                     _archiveStream = stream;
                 _archiveStreamOwner = null;
-                if (mode == ZipArchiveMode.Create)
-                    _archiveReader = null;
-                else
-                    _archiveReader = new BinaryReader(_archiveStream, Encoding.UTF8, leaveOpen: true);
                 _entries = new List<ZipArchiveEntry>();
                 _entriesCollection = new ReadOnlyCollection<ZipArchiveEntry>(_entries);
                 _entriesDictionary = new Dictionary<string, ZipArchiveEntry>();
@@ -351,8 +347,6 @@ public void Dispose()
             return result;
         }
 
-        internal BinaryReader? ArchiveReader => _archiveReader;
-
         internal Stream ArchiveStream => _archiveStream;
 
         internal uint NumberOfThisDisk => _numberOfThisDisk;
@@ -460,7 +454,6 @@ private void CloseStreams()
             {
                 _archiveStream.Dispose();
                 _backingStream?.Dispose();
-                _archiveReader?.Dispose();
             }
             else
             {
@@ -483,32 +476,90 @@ private void EnsureCentralDirectoryRead()
 
         private void ReadCentralDirectory()
         {
+            const int ReadBufferSize = 4096;
+
+            byte[] fileBuffer = System.Buffers.ArrayPool<byte>.Shared.Rent(ReadBufferSize);
+            Span<byte> fileBufferSpan = fileBuffer.AsSpan(0, ReadBufferSize);
+
             try
             {
                 // assume ReadEndOfCentralDirectory has been called and has populated _centralDirectoryStart
 
                 _archiveStream.Seek(_centralDirectoryStart, SeekOrigin.Begin);
 
                 long numberOfEntries = 0;
-
-                Debug.Assert(_archiveReader != null);
-                //read the central directory
-                ZipCentralDirectoryFileHeader currentHeader;
                 bool saveExtraFieldsAndComments = Mode == ZipArchiveMode.Update;
-                while (ZipCentralDirectoryFileHeader.TryReadBlock(_archiveReader,
-                                                        saveExtraFieldsAndComments, out currentHeader))
+
+                bool continueReadingCentralDirectory = true;
+                // total bytes read from central directory
+                int bytesRead = 0;
+                // current position in the current buffer
+                int currPosition = 0;
+                // total bytes read from all file headers starting in the current buffer
+                int bytesConsumed = 0;
+
+                _entries.Clear();
+                _entriesDictionary.Clear();
+
+                // read the central directory
+                while (continueReadingCentralDirectory)
                 {
-                    AddEntry(new ZipArchiveEntry(this, currentHeader));
-                    numberOfEntries++;
+                    int currBytesRead = _archiveStream.Read(fileBufferSpan);
+                    ReadOnlySpan<byte> sizedFileBuffer = fileBufferSpan.Slice(0, currBytesRead);
+
+                    // the buffer read must always be large enough to fit the constant section size of at least one header
+                    continueReadingCentralDirectory = continueReadingCentralDirectory
+                        && sizedFileBuffer.Length >= ZipCentralDirectoryFileHeader.BlockConstantSectionSize;
+
+                    while (continueReadingCentralDirectory
+                        && currPosition + ZipCentralDirectoryFileHeader.BlockConstantSectionSize < sizedFileBuffer.Length)
+                    {
+                        ZipCentralDirectoryFileHeader currentHeader = default;
+
+                        continueReadingCentralDirectory = continueReadingCentralDirectory &&
+                            ZipCentralDirectoryFileHeader.TryReadBlock(sizedFileBuffer.Slice(currPosition), _archiveStream,
+                            saveExtraFieldsAndComments, out bytesConsumed, out currentHeader);
+
+                        if (!continueReadingCentralDirectory)
+                        {
+                            break;
+                        }
+
+                        AddEntry(new ZipArchiveEntry(this, currentHeader));
+                        numberOfEntries++;
+                        if (numberOfEntries > _expectedNumberOfEntries)
+                        {
+                            throw new InvalidDataException(SR.NumEntriesWrong);
+                        }
+
+                        currPosition += bytesConsumed;
+                        bytesRead += bytesConsumed;
+                    }
+
+                    // We've run out of possible space in the entry - seek backwards by the number of bytes remaining in
+                    // this buffer (so that the next buffer overlaps with this one) and retry.
+                    if (currPosition < sizedFileBuffer.Length)
+                    {
+                        _archiveStream.Seek(-(sizedFileBuffer.Length - currPosition), SeekOrigin.Current);
+                    }
+                    currPosition = 0;
                 }
 
                 if (numberOfEntries != _expectedNumberOfEntries)
+                {
                     throw new InvalidDataException(SR.NumEntriesWrong);
+                }
+
+                _archiveStream.Seek(_centralDirectoryStart + bytesRead, SeekOrigin.Begin);
             }
             catch (EndOfStreamException ex)
             {
                 throw new InvalidDataException(SR.Format(SR.CentralDirectoryInvalid, ex));
             }
+            finally
+            {
+                System.Buffers.ArrayPool<byte>.Shared.Return(fileBuffer);
+            }
         }
 
         // This function reads all the EOCD stuff it needs to find the offset to the start of the central directory
@@ -526,16 +577,15 @@ private void ReadEndOfCentralDirectory()
                 // If the EOCD has the minimum possible size (no zip file comment), then exactly the previous 4 bytes will contain the signature
                 // But if the EOCD has max possible size, the signature should be found somewhere in the previous 64K + 4 bytes
                 if (!ZipHelper.SeekBackwardsToSignature(_archiveStream,
-                        ZipEndOfCentralDirectoryBlock.SignatureConstant,
-                        ZipEndOfCentralDirectoryBlock.ZipFileCommentMaxLength + ZipEndOfCentralDirectoryBlock.SignatureSize))
+                        ZipEndOfCentralDirectoryBlock.SignatureConstantBytes,
+                        ZipEndOfCentralDirectoryBlock.ZipFileCommentMaxLength + ZipEndOfCentralDirectoryBlock.FieldLengths.Signature))
                     throw new InvalidDataException(SR.EOCDNotFound);
 
                 long eocdStart = _archiveStream.Position;
 
-                Debug.Assert(_archiveReader != null);
                 // read the EOCD
                 ZipEndOfCentralDirectoryBlock eocd;
-                bool eocdProper = ZipEndOfCentralDirectoryBlock.TryReadBlock(_archiveReader, out eocd);
+                bool eocdProper = ZipEndOfCentralDirectoryBlock.TryReadBlock(_archiveStream, out eocd);
                 Debug.Assert(eocdProper); // we just found this using the signature finder, so it should be okay
 
                 if (eocd.NumberOfThisDisk != eocd.NumberOfTheDiskWithTheStartOfTheCentralDirectory)
@@ -587,14 +637,12 @@ private void TryReadZip64EndOfCentralDirectory(ZipEndOfCentralDirectoryBlock eoc
                 // Exactly the previous 4 bytes should contain the Zip64-EOCDL signature
                 // if we don't find it, assume it doesn't exist and use data from normal EOCD
                 if (ZipHelper.SeekBackwardsToSignature(_archiveStream,
-                        Zip64EndOfCentralDirectoryLocator.SignatureConstant,
-                        Zip64EndOfCentralDirectoryLocator.SignatureSize))
+                        Zip64EndOfCentralDirectoryLocator.SignatureConstantBytes,
+                        Zip64EndOfCentralDirectoryLocator.FieldLengths.Signature))
                 {
-                    Debug.Assert(_archiveReader != null);
-
                     // use locator to get to Zip64-EOCD
                     Zip64EndOfCentralDirectoryLocator locator;
-                    bool zip64eocdLocatorProper = Zip64EndOfCentralDirectoryLocator.TryReadBlock(_archiveReader, out locator);
+                    bool zip64eocdLocatorProper = Zip64EndOfCentralDirectoryLocator.TryReadBlock(_archiveStream, out locator);
                     Debug.Assert(zip64eocdLocatorProper); // we just found this using the signature finder, so it should be okay
 
                     if (locator.OffsetOfZip64EOCD > long.MaxValue)
@@ -607,7 +655,7 @@ private void TryReadZip64EndOfCentralDirectory(ZipEndOfCentralDirectoryBlock eoc
                     // Read Zip64 End of Central Directory Record
 
                     Zip64EndOfCentralDirectoryRecord record;
-                    if (!Zip64EndOfCentralDirectoryRecord.TryReadBlock(_archiveReader, out record))
+                    if (!Zip64EndOfCentralDirectoryRecord.TryReadBlock(_archiveStream, out record))
                         throw new InvalidDataException(SR.Zip64EOCDNotWhereExpected);
 
                     _numberOfThisDisk = record.NumberOfThisDisk;