From 292694b529ef31e4936b80a07aa5f5391fe9bf00 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 19 Sep 2022 21:03:37 -0600 Subject: [PATCH] Fix `Stream.Seek` implementations to reliably shift position as required The bug is that `Stream.Read` was assumed to always fill the provided buffer, but in fact only a non-zero movement is contractually guaranteed. --- .../ManagedGit/GitPackDeltafiedStream.cs | 6 +---- .../ManagedGit/GitPackMemoryCacheStream.cs | 8 ++---- .../ManagedGit/StreamExtensions.cs | 27 +++++++++++++++++++ .../ManagedGit/ZLibStream.cs | 6 +---- 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/NerdBank.GitVersioning/ManagedGit/GitPackDeltafiedStream.cs b/src/NerdBank.GitVersioning/ManagedGit/GitPackDeltafiedStream.cs index 9b73a7bb..3dae2a4d 100644 --- a/src/NerdBank.GitVersioning/ManagedGit/GitPackDeltafiedStream.cs +++ b/src/NerdBank.GitVersioning/ManagedGit/GitPackDeltafiedStream.cs @@ -135,11 +135,7 @@ public override long Seek(long offset, SeekOrigin origin) if (origin == SeekOrigin.Begin && offset > this.position) { // We can optimise this by skipping over instructions rather than executing them - int length = (int)(offset - this.position); - - byte[] buffer = ArrayPool.Shared.Rent(length); - this.Read(buffer, 0, length); - ArrayPool.Shared.Return(buffer); + this.ReadExactly(checked((int)(offset - this.position))); return this.position; } else diff --git a/src/NerdBank.GitVersioning/ManagedGit/GitPackMemoryCacheStream.cs b/src/NerdBank.GitVersioning/ManagedGit/GitPackMemoryCacheStream.cs index 66fa2f04..4b0a9191 100644 --- a/src/NerdBank.GitVersioning/ManagedGit/GitPackMemoryCacheStream.cs +++ b/src/NerdBank.GitVersioning/ManagedGit/GitPackMemoryCacheStream.cs @@ -73,13 +73,9 @@ public override long Seek(long offset, SeekOrigin origin) if (offset > this.cacheStream.Length) { - var toRead = (int)(offset - this.cacheStream.Length); - byte[] buffer = ArrayPool.Shared.Rent(toRead); - int read = this.stream.Read(buffer, 0, toRead); this.cacheStream.Seek(0, SeekOrigin.End); - this.cacheStream.Write(buffer, 0, read); - ArrayPool.Shared.Return(buffer); - + int toRead = (int)(offset - this.cacheStream.Length); + this.stream.ReadExactly(toRead, this.cacheStream); this.DisposeStreamIfRead(); return this.cacheStream.Position; } diff --git a/src/NerdBank.GitVersioning/ManagedGit/StreamExtensions.cs b/src/NerdBank.GitVersioning/ManagedGit/StreamExtensions.cs index 550431c8..df84ebdd 100644 --- a/src/NerdBank.GitVersioning/ManagedGit/StreamExtensions.cs +++ b/src/NerdBank.GitVersioning/ManagedGit/StreamExtensions.cs @@ -150,5 +150,32 @@ internal static bool TryAdd(this System.Collections.Generic.IDicti return true; } #endif + + /// + /// Reads the specified number of bytes from a stream, or until the end of the stream. + /// + /// The stream to read from. + /// The number of bytes to be read. + /// The stream to copy the read bytes to, if required. + /// The number of bytes actually read. This will be less than only if the end of is reached. + internal static int ReadExactly(this Stream readFrom, int length, Stream? copyTo = null) + { + int bytesRemaining = length; + byte[] buffer = ArrayPool.Shared.Rent(Math.Min(50 * 1024, bytesRemaining)); + while (bytesRemaining > 0) + { + int read = readFrom.Read(buffer, 0, Math.Min(buffer.Length, bytesRemaining)); + if (read == 0) + { + break; + } + + copyTo?.Write(buffer, 0, read); + bytesRemaining -= read; + } + + ArrayPool.Shared.Return(buffer); + return length - bytesRemaining; + } } } diff --git a/src/NerdBank.GitVersioning/ManagedGit/ZLibStream.cs b/src/NerdBank.GitVersioning/ManagedGit/ZLibStream.cs index 4a350e30..d09e0b34 100644 --- a/src/NerdBank.GitVersioning/ManagedGit/ZLibStream.cs +++ b/src/NerdBank.GitVersioning/ManagedGit/ZLibStream.cs @@ -151,11 +151,7 @@ public override long Seek(long offset, SeekOrigin origin) if (origin == SeekOrigin.Begin && offset > this.position) { // We may be able to optimize this by skipping over the compressed data - int length = (int)(offset - this.position); - - byte[] buffer = ArrayPool.Shared.Rent(length); - this.Read(buffer, 0, length); - ArrayPool.Shared.Return(buffer); + this.ReadExactly(checked((int)(offset - this.position))); return this.position; } else