From 36b3fab2d9b9e340d691ea2fa796b37aa4561ab3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 24 Jul 2020 01:13:55 +0100 Subject: [PATCH 1/6] Make stream buffer size configurable. Fixes #1276 --- src/ImageSharp/Configuration.cs | 24 ++++- src/ImageSharp/Formats/Bmp/BmpDecoder.cs | 8 +- src/ImageSharp/Formats/Gif/GifDecoder.cs | 8 +- src/ImageSharp/Formats/Jpeg/JpegDecoder.cs | 8 +- src/ImageSharp/Formats/Png/PngDecoder.cs | 8 +- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 +- src/ImageSharp/Formats/Tga/TgaDecoder.cs | 8 +- src/ImageSharp/IO/BufferedReadStream.cs | 47 +++++----- src/ImageSharp/Image.FromStream.cs | 4 +- .../Codecs/Jpeg/DecodeJpegParseStreamOnly.cs | 2 +- .../General/IO/BufferedStreams.cs | 6 +- tests/ImageSharp.Tests/ConfigurationTests.cs | 7 ++ .../Formats/Jpg/JpegDecoderTests.cs | 2 +- .../Formats/Jpg/SpectralJpegTests.cs | 4 +- .../Formats/Jpg/Utils/JpegFixture.cs | 2 +- .../Formats/Png/PngDecoderTests.Chunks.cs | 2 +- .../IO/BufferedReadStreamTests.cs | 87 ++++++++++--------- 17 files changed, 137 insertions(+), 93 deletions(-) diff --git a/src/ImageSharp/Configuration.cs b/src/ImageSharp/Configuration.cs index 067257132f..b96204ed9e 100644 --- a/src/ImageSharp/Configuration.cs +++ b/src/ImageSharp/Configuration.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; -using System.Net.Http; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Bmp; using SixLabors.ImageSharp.Formats.Gif; @@ -27,6 +26,9 @@ public sealed class Configuration /// private static readonly Lazy Lazy = new Lazy(CreateDefaultInstance); + private const int MinStreamProcessingBufferSize = 128; + private const int DefaultStreamProcessingBufferSize = 8096; + private int streamProcessingBufferSize = DefaultStreamProcessingBufferSize; private int maxDegreeOfParallelism = Environment.ProcessorCount; /// @@ -75,6 +77,25 @@ public int MaxDegreeOfParallelism } } + /// + /// Gets or sets the size of the buffer to use when working with streams. + /// Intitialized with by default + /// and can accept a minimum value of . + /// + public int StreamProcessingBufferSize + { + get => this.streamProcessingBufferSize; + set + { + if (value < MinStreamProcessingBufferSize) + { + value = MinStreamProcessingBufferSize; + } + + this.streamProcessingBufferSize = value; + } + } + /// /// Gets a set of properties for the Congiguration. /// @@ -145,6 +166,7 @@ public Configuration Clone() return new Configuration { MaxDegreeOfParallelism = this.MaxDegreeOfParallelism, + StreamProcessingBufferSize = this.StreamProcessingBufferSize, ImageFormatsManager = this.ImageFormatsManager, MemoryAllocator = this.MemoryAllocator, ImageOperationsProvider = this.ImageOperationsProvider, diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoder.cs b/src/ImageSharp/Formats/Bmp/BmpDecoder.cs index 7e8ac07215..cb26ff606a 100644 --- a/src/ImageSharp/Formats/Bmp/BmpDecoder.cs +++ b/src/ImageSharp/Formats/Bmp/BmpDecoder.cs @@ -39,7 +39,7 @@ public Image Decode(Configuration configuration, Stream stream) try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.Decode(bufferedStream); } catch (InvalidMemoryOperationException ex) @@ -64,7 +64,7 @@ public async Task> DecodeAsync(Configuration configuration try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return await decoder.DecodeAsync(bufferedStream).ConfigureAwait(false); } catch (InvalidMemoryOperationException ex) @@ -84,7 +84,7 @@ public IImageInfo Identify(Configuration configuration, Stream stream) { Guard.NotNull(stream, nameof(stream)); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return new BmpDecoderCore(configuration, this).Identify(bufferedStream); } @@ -93,7 +93,7 @@ public Task IdentifyAsync(Configuration configuration, Stream stream { Guard.NotNull(stream, nameof(stream)); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return new BmpDecoderCore(configuration, this).IdentifyAsync(bufferedStream); } } diff --git a/src/ImageSharp/Formats/Gif/GifDecoder.cs b/src/ImageSharp/Formats/Gif/GifDecoder.cs index 2a5fde6acd..2b7103072b 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoder.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoder.cs @@ -33,7 +33,7 @@ public Image Decode(Configuration configuration, Stream stream) try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.Decode(bufferedStream); } catch (InvalidMemoryOperationException ex) @@ -59,7 +59,7 @@ public async Task> DecodeAsync(Configuration configuration try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return await decoder.DecodeAsync(bufferedStream).ConfigureAwait(false); } catch (InvalidMemoryOperationException ex) @@ -84,7 +84,7 @@ public IImageInfo Identify(Configuration configuration, Stream stream) var decoder = new GifDecoderCore(configuration, this); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.Identify(bufferedStream); } @@ -95,7 +95,7 @@ public Task IdentifyAsync(Configuration configuration, Stream stream var decoder = new GifDecoderCore(configuration, this); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.IdentifyAsync(bufferedStream); } } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoder.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoder.cs index c5332acb5a..3eaf3a4c47 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoder.cs @@ -26,7 +26,7 @@ public Image Decode(Configuration configuration, Stream stream) using var decoder = new JpegDecoderCore(configuration, this); try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.Decode(bufferedStream); } catch (InvalidMemoryOperationException ex) @@ -53,7 +53,7 @@ public async Task> DecodeAsync(Configuration configuration using var decoder = new JpegDecoderCore(configuration, this); try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return await decoder.DecodeAsync(bufferedStream).ConfigureAwait(false); } catch (InvalidMemoryOperationException ex) @@ -77,7 +77,7 @@ public IImageInfo Identify(Configuration configuration, Stream stream) Guard.NotNull(stream, nameof(stream)); using var decoder = new JpegDecoderCore(configuration, this); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.Identify(bufferedStream); } @@ -88,7 +88,7 @@ public Task IdentifyAsync(Configuration configuration, Stream stream Guard.NotNull(stream, nameof(stream)); using var decoder = new JpegDecoderCore(configuration, this); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.IdentifyAsync(bufferedStream); } diff --git a/src/ImageSharp/Formats/Png/PngDecoder.cs b/src/ImageSharp/Formats/Png/PngDecoder.cs index 9eb9277840..87e0195c35 100644 --- a/src/ImageSharp/Formats/Png/PngDecoder.cs +++ b/src/ImageSharp/Formats/Png/PngDecoder.cs @@ -25,7 +25,7 @@ public Image Decode(Configuration configuration, Stream stream) try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.Decode(bufferedStream); } catch (InvalidMemoryOperationException ex) @@ -50,7 +50,7 @@ public async Task> DecodeAsync(Configuration configuration try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return await decoder.DecodeAsync(bufferedStream).ConfigureAwait(false); } catch (InvalidMemoryOperationException ex) @@ -71,7 +71,7 @@ public async Task> DecodeAsync(Configuration configuration public IImageInfo Identify(Configuration configuration, Stream stream) { var decoder = new PngDecoderCore(configuration, this); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.Identify(bufferedStream); } @@ -79,7 +79,7 @@ public IImageInfo Identify(Configuration configuration, Stream stream) public Task IdentifyAsync(Configuration configuration, Stream stream) { var decoder = new PngDecoderCore(configuration, this); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.IdentifyAsync(bufferedStream); } } diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 89fa4e63d0..89caac3f68 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -9,7 +9,6 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; -using System.Threading.Tasks; using SixLabors.ImageSharp.Formats.Png.Chunks; using SixLabors.ImageSharp.Formats.Png.Filters; using SixLabors.ImageSharp.Formats.Png.Zlib; @@ -1027,7 +1026,7 @@ private void ReadInternationalTextChunk(PngMetadata metadata, ReadOnlySpan private bool TryUncompressTextData(ReadOnlySpan compressedData, Encoding encoding, out string value) { using (var memoryStream = new MemoryStream(compressedData.ToArray())) - using (var bufferedStream = new BufferedReadStream(memoryStream)) + using (var bufferedStream = new BufferedReadStream(this.Configuration, memoryStream)) using (var inflateStream = new ZlibInflateStream(bufferedStream)) { if (!inflateStream.AllocateNewBytes(compressedData.Length, false)) diff --git a/src/ImageSharp/Formats/Tga/TgaDecoder.cs b/src/ImageSharp/Formats/Tga/TgaDecoder.cs index 3d9b9a3d2a..25aa233db8 100644 --- a/src/ImageSharp/Formats/Tga/TgaDecoder.cs +++ b/src/ImageSharp/Formats/Tga/TgaDecoder.cs @@ -24,7 +24,7 @@ public Image Decode(Configuration configuration, Stream stream) try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return decoder.Decode(bufferedStream); } catch (InvalidMemoryOperationException ex) @@ -52,7 +52,7 @@ public async Task> DecodeAsync(Configuration configuration try { - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return await decoder.DecodeAsync(bufferedStream).ConfigureAwait(false); } catch (InvalidMemoryOperationException ex) @@ -75,7 +75,7 @@ public IImageInfo Identify(Configuration configuration, Stream stream) { Guard.NotNull(stream, nameof(stream)); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return new TgaDecoderCore(configuration, this).Identify(bufferedStream); } @@ -84,7 +84,7 @@ public Task IdentifyAsync(Configuration configuration, Stream stream { Guard.NotNull(stream, nameof(stream)); - using var bufferedStream = new BufferedReadStream(stream); + using var bufferedStream = new BufferedReadStream(configuration, stream); return new TgaDecoderCore(configuration, this).IdentifyAsync(bufferedStream); } } diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 8166263744..54d919963e 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -14,12 +14,7 @@ namespace SixLabors.ImageSharp.IO /// internal sealed class BufferedReadStream : Stream { - /// - /// The length, in bytes, of the underlying buffer. - /// - public const int BufferLength = 8192; - - private const int MaxBufferIndex = BufferLength - 1; + private readonly int maxBufferIndex; private readonly byte[] readBuffer; @@ -38,9 +33,11 @@ internal sealed class BufferedReadStream : Stream /// /// Initializes a new instance of the class. /// + /// The configuration which allows altering default behaviour or extending the library. /// The input stream. - public BufferedReadStream(Stream stream) + public BufferedReadStream(Configuration configuration, Stream stream) { + Guard.NotNull(configuration, nameof(configuration)); Guard.IsTrue(stream.CanRead, nameof(stream), "Stream must be readable."); Guard.IsTrue(stream.CanSeek, nameof(stream), "Stream must be seekable."); @@ -55,8 +52,9 @@ public BufferedReadStream(Stream stream) this.BaseStream = stream; this.Position = (int)stream.Position; this.Length = stream.Length; - - this.readBuffer = ArrayPool.Shared.Rent(BufferLength); + this.BufferSize = configuration.StreamProcessingBufferSize; + this.maxBufferIndex = this.BufferSize - 1; + this.readBuffer = ArrayPool.Shared.Rent(this.BufferSize); this.readBufferHandle = new Memory(this.readBuffer).Pin(); unsafe { @@ -64,7 +62,16 @@ public BufferedReadStream(Stream stream) } // This triggers a full read on first attempt. - this.readBufferIndex = BufferLength; + this.readBufferIndex = this.BufferSize; + } + + /// + /// Gets the size, in bytes, of the underlying buffer. + /// + public int BufferSize + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get; } /// @@ -91,7 +98,7 @@ public override long Position // Base stream seek will throw for us if invalid. this.BaseStream.Seek(value, SeekOrigin.Begin); this.readerPosition = value; - this.readBufferIndex = BufferLength; + this.readBufferIndex = this.BufferSize; } } } @@ -125,7 +132,7 @@ public override int ReadByte() // Our buffer has been read. // We need to refill and start again. - if (this.readBufferIndex > MaxBufferIndex) + if (this.readBufferIndex > this.maxBufferIndex) { this.FillReadBuffer(); } @@ -142,14 +149,14 @@ public override int ReadByte() public override int Read(byte[] buffer, int offset, int count) { // Too big for our buffer. Read directly from the stream. - if (count > BufferLength) + if (count > this.BufferSize) { return this.ReadToBufferDirectSlow(buffer, offset, count); } // Too big for remaining buffer but less than entire buffer length // Copy to buffer then read from there. - if (count + this.readBufferIndex > BufferLength) + if (count + this.readBufferIndex > this.BufferSize) { return this.ReadToBufferViaCopySlow(buffer, offset, count); } @@ -164,14 +171,14 @@ public override int Read(Span buffer) { // Too big for our buffer. Read directly from the stream. int count = buffer.Length; - if (count > BufferLength) + if (count > this.BufferSize) { return this.ReadToBufferDirectSlow(buffer); } // Too big for remaining buffer but less than entire buffer length // Copy to buffer then read from there. - if (count + this.readBufferIndex > BufferLength) + if (count + this.readBufferIndex > this.BufferSize) { return this.ReadToBufferViaCopySlow(buffer); } @@ -192,7 +199,7 @@ public override void Flush() } // Reset to trigger full read on next attempt. - this.readBufferIndex = BufferLength; + this.readBufferIndex = this.BufferSize; } /// @@ -249,7 +256,7 @@ protected override void Dispose(bool disposing) private bool IsInReadBuffer(long newPosition, out long index) { index = newPosition - this.readerPosition + this.readBufferIndex; - return index > -1 && index < BufferLength; + return index > -1 && index < this.BufferSize; } [MethodImpl(MethodImplOptions.NoInlining)] @@ -267,10 +274,10 @@ private void FillReadBuffer() int i; do { - i = baseStream.Read(this.readBuffer, n, BufferLength - n); + i = baseStream.Read(this.readBuffer, n, this.BufferSize - n); n += i; } - while (n < BufferLength && i > 0); + while (n < this.BufferSize && i > 0); this.readBufferIndex = 0; } diff --git a/src/ImageSharp/Image.FromStream.cs b/src/ImageSharp/Image.FromStream.cs index beec0b1880..fae88f21e1 100644 --- a/src/ImageSharp/Image.FromStream.cs +++ b/src/ImageSharp/Image.FromStream.cs @@ -687,7 +687,7 @@ private static T WithSeekableStream( // We want to be able to load images from things like HttpContext.Request.Body using MemoryStream memoryStream = configuration.MemoryAllocator.AllocateFixedCapacityMemoryStream(stream.Length); - stream.CopyTo(memoryStream); + stream.CopyTo(memoryStream, configuration.StreamProcessingBufferSize); memoryStream.Position = 0; return action(memoryStream); @@ -729,7 +729,7 @@ private static async Task WithSeekableStreamAsync( } using MemoryStream memoryStream = configuration.MemoryAllocator.AllocateFixedCapacityMemoryStream(stream.Length); - await stream.CopyToAsync(memoryStream).ConfigureAwait(false); + await stream.CopyToAsync(memoryStream, configuration.StreamProcessingBufferSize).ConfigureAwait(false); memoryStream.Position = 0; return await action(memoryStream).ConfigureAwait(false); diff --git a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs index ef098e2632..8c597a8c5b 100644 --- a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs +++ b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs @@ -40,7 +40,7 @@ public SDSize JpegSystemDrawing() public void ParseStreamPdfJs() { using var memoryStream = new MemoryStream(this.jpegBytes); - using var bufferedStream = new BufferedReadStream(memoryStream); + using var bufferedStream = new BufferedReadStream(Configuration.Default, memoryStream); var decoder = new JpegDecoderCore(Configuration.Default, new JpegDecoder { IgnoreMetadata = true }); decoder.ParseStream(bufferedStream); diff --git a/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs b/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs index 72cceae90e..be232c78d6 100644 --- a/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs +++ b/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs @@ -35,8 +35,8 @@ public void CreateStreams() this.stream4 = new MemoryStream(this.buffer); this.stream5 = new MemoryStream(this.buffer); this.stream6 = new MemoryStream(this.buffer); - this.bufferedStream1 = new BufferedReadStream(this.stream3); - this.bufferedStream2 = new BufferedReadStream(this.stream4); + this.bufferedStream1 = new BufferedReadStream(Configuration.Default, this.stream3); + this.bufferedStream2 = new BufferedReadStream(Configuration.Default, this.stream4); this.bufferedStreamWrap1 = new BufferedReadStreamWrapper(this.stream5); this.bufferedStreamWrap2 = new BufferedReadStreamWrapper(this.stream6); } @@ -158,7 +158,7 @@ public int ArrayReadByte() private static byte[] CreateTestBytes() { - var buffer = new byte[BufferedReadStream.BufferLength * 3]; + var buffer = new byte[Configuration.Default.StreamProcessingBufferSize * 3]; var random = new Random(); random.NextBytes(buffer); diff --git a/tests/ImageSharp.Tests/ConfigurationTests.cs b/tests/ImageSharp.Tests/ConfigurationTests.cs index b18d048340..3507d5c4de 100644 --- a/tests/ImageSharp.Tests/ConfigurationTests.cs +++ b/tests/ImageSharp.Tests/ConfigurationTests.cs @@ -133,5 +133,12 @@ public void WorkingBufferSizeHint_DefaultIsCorrect() Configuration config = this.DefaultConfiguration; Assert.True(config.WorkingBufferSizeHintInBytes > 1024); } + + [Fact] + public void StreamBufferSize_DefaultIsCorrect() + { + Configuration config = this.DefaultConfiguration; + Assert.True(config.StreamProcessingBufferSize == 8096); + } } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 0694a08556..912f606b2c 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -73,7 +73,7 @@ public void ParseStream_BasicPropertiesAreCorrect() { byte[] bytes = TestFile.Create(TestImages.Jpeg.Progressive.Progress).Bytes; using var ms = new MemoryStream(bytes); - using var bufferedStream = new BufferedReadStream(ms); + using var bufferedStream = new BufferedReadStream(Configuration.Default, ms); var decoder = new JpegDecoderCore(Configuration.Default, new JpegDecoder()); decoder.ParseStream(bufferedStream); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs index 1d200592a8..662ea9e330 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs @@ -53,7 +53,7 @@ public void Decoder_ParseStream_SaveSpectralResult(TestImageProvider(TestImageProvider provider byte[] sourceBytes = TestFile.Create(provider.SourceFileOrDescription).Bytes; using var ms = new MemoryStream(sourceBytes); - using var bufferedStream = new BufferedReadStream(ms); + using var bufferedStream = new BufferedReadStream(Configuration.Default, ms); decoder.ParseStream(bufferedStream); var imageSharpData = LibJpegTools.SpectralData.LoadFromImageSharpDecoder(decoder); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/Utils/JpegFixture.cs b/tests/ImageSharp.Tests/Formats/Jpg/Utils/JpegFixture.cs index 96d85fd8e4..c6f4704f05 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/Utils/JpegFixture.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/Utils/JpegFixture.cs @@ -193,7 +193,7 @@ internal static JpegDecoderCore ParseJpegStream(string testFileName, bool metaDa { byte[] bytes = TestFile.Create(testFileName).Bytes; using var ms = new MemoryStream(bytes); - using var bufferedStream = new BufferedReadStream(ms); + using var bufferedStream = new BufferedReadStream(Configuration.Default, ms); var decoder = new JpegDecoderCore(Configuration.Default, new JpegDecoder()); decoder.ParseStream(bufferedStream, metaDataOnly); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs index 6284191f39..1ec7e24486 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs @@ -77,7 +77,7 @@ public void Decode_IncorrectCRCForCriticalChunk_ExceptionIsThrown(uint chunkType var decoder = new PngDecoder(); ImageFormatException exception = - Assert.Throws(() => decoder.Decode(null, memStream)); + Assert.Throws(() => decoder.Decode(Configuration.Default, memStream)); Assert.Equal($"CRC Error. PNG {chunkName} chunk is corrupt!", exception.Message); } diff --git a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs index d08d5adef4..b15093a3d1 100644 --- a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs @@ -10,18 +10,27 @@ namespace SixLabors.ImageSharp.Tests.IO { public class BufferedReadStreamTests { + private readonly Configuration configuration; + private readonly int bufferSize; + + public BufferedReadStreamTests() + { + this.configuration = Configuration.Default; + this.bufferSize = this.configuration.StreamProcessingBufferSize; + } + [Fact] public void BufferedStreamCanReadSingleByteFromOrigin() { - using (MemoryStream stream = this.CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) { byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(stream)) + using (var reader = new BufferedReadStream(this.configuration, stream)) { Assert.Equal(expected[0], reader.ReadByte()); // We've read a whole chunk but increment by 1 in our reader. - Assert.Equal(BufferedReadStream.BufferLength, stream.Position); + Assert.Equal(this.bufferSize, stream.Position); Assert.Equal(1, reader.Position); } @@ -33,18 +42,18 @@ public void BufferedStreamCanReadSingleByteFromOrigin() [Fact] public void BufferedStreamCanReadSingleByteFromOffset() { - using (MemoryStream stream = this.CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) { byte[] expected = stream.ToArray(); const int offset = 5; - using (var reader = new BufferedReadStream(stream)) + using (var reader = new BufferedReadStream(this.configuration, stream)) { reader.Position = offset; Assert.Equal(expected[offset], reader.ReadByte()); // We've read a whole chunk but increment by 1 in our reader. - Assert.Equal(BufferedReadStream.BufferLength + offset, stream.Position); + Assert.Equal(this.bufferSize + offset, stream.Position); Assert.Equal(offset + 1, reader.Position); } @@ -55,30 +64,30 @@ public void BufferedStreamCanReadSingleByteFromOffset() [Fact] public void BufferedStreamCanReadSubsequentSingleByteCorrectly() { - using (MemoryStream stream = this.CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) { byte[] expected = stream.ToArray(); int i; - using (var reader = new BufferedReadStream(stream)) + using (var reader = new BufferedReadStream(this.configuration, stream)) { for (i = 0; i < expected.Length; i++) { Assert.Equal(expected[i], reader.ReadByte()); Assert.Equal(i + 1, reader.Position); - if (i < BufferedReadStream.BufferLength) + if (i < this.bufferSize) { - Assert.Equal(stream.Position, BufferedReadStream.BufferLength); + Assert.Equal(stream.Position, this.bufferSize); } - else if (i >= BufferedReadStream.BufferLength && i < BufferedReadStream.BufferLength * 2) + else if (i >= this.bufferSize && i < this.bufferSize * 2) { // We should have advanced to the second chunk now. - Assert.Equal(stream.Position, BufferedReadStream.BufferLength * 2); + Assert.Equal(stream.Position, this.bufferSize * 2); } else { // We should have advanced to the third chunk now. - Assert.Equal(stream.Position, BufferedReadStream.BufferLength * 3); + Assert.Equal(stream.Position, this.bufferSize * 3); } } } @@ -90,18 +99,18 @@ public void BufferedStreamCanReadSubsequentSingleByteCorrectly() [Fact] public void BufferedStreamCanReadMultipleBytesFromOrigin() { - using (MemoryStream stream = this.CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) { var buffer = new byte[2]; byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(stream)) + using (var reader = new BufferedReadStream(this.configuration, stream)) { Assert.Equal(2, reader.Read(buffer, 0, 2)); Assert.Equal(expected[0], buffer[0]); Assert.Equal(expected[1], buffer[1]); // We've read a whole chunk but increment by the buffer length in our reader. - Assert.Equal(stream.Position, BufferedReadStream.BufferLength); + Assert.Equal(stream.Position, this.bufferSize); Assert.Equal(buffer.Length, reader.Position); } } @@ -110,11 +119,11 @@ public void BufferedStreamCanReadMultipleBytesFromOrigin() [Fact] public void BufferedStreamCanReadSubsequentMultipleByteCorrectly() { - using (MemoryStream stream = this.CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) { var buffer = new byte[2]; byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(stream)) + using (var reader = new BufferedReadStream(this.configuration, stream)) { for (int i = 0, o = 0; i < expected.Length / 2; i++, o += 2) { @@ -124,19 +133,19 @@ public void BufferedStreamCanReadSubsequentMultipleByteCorrectly() Assert.Equal(o + 2, reader.Position); int offset = i * 2; - if (offset < BufferedReadStream.BufferLength) + if (offset < this.bufferSize) { - Assert.Equal(stream.Position, BufferedReadStream.BufferLength); + Assert.Equal(stream.Position, this.bufferSize); } - else if (offset >= BufferedReadStream.BufferLength && offset < BufferedReadStream.BufferLength * 2) + else if (offset >= this.bufferSize && offset < this.bufferSize * 2) { // We should have advanced to the second chunk now. - Assert.Equal(stream.Position, BufferedReadStream.BufferLength * 2); + Assert.Equal(stream.Position, this.bufferSize * 2); } else { // We should have advanced to the third chunk now. - Assert.Equal(stream.Position, BufferedReadStream.BufferLength * 3); + Assert.Equal(stream.Position, this.bufferSize * 3); } } } @@ -146,11 +155,11 @@ public void BufferedStreamCanReadSubsequentMultipleByteCorrectly() [Fact] public void BufferedStreamCanReadSubsequentMultipleByteSpanCorrectly() { - using (MemoryStream stream = this.CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) { Span buffer = new byte[2]; byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(stream)) + using (var reader = new BufferedReadStream(this.configuration, stream)) { for (int i = 0, o = 0; i < expected.Length / 2; i++, o += 2) { @@ -160,19 +169,19 @@ public void BufferedStreamCanReadSubsequentMultipleByteSpanCorrectly() Assert.Equal(o + 2, reader.Position); int offset = i * 2; - if (offset < BufferedReadStream.BufferLength) + if (offset < this.bufferSize) { - Assert.Equal(stream.Position, BufferedReadStream.BufferLength); + Assert.Equal(stream.Position, this.bufferSize); } - else if (offset >= BufferedReadStream.BufferLength && offset < BufferedReadStream.BufferLength * 2) + else if (offset >= this.bufferSize && offset < this.bufferSize * 2) { // We should have advanced to the second chunk now. - Assert.Equal(stream.Position, BufferedReadStream.BufferLength * 2); + Assert.Equal(stream.Position, this.bufferSize * 2); } else { // We should have advanced to the third chunk now. - Assert.Equal(stream.Position, BufferedReadStream.BufferLength * 3); + Assert.Equal(stream.Position, this.bufferSize * 3); } } } @@ -182,14 +191,14 @@ public void BufferedStreamCanReadSubsequentMultipleByteSpanCorrectly() [Fact] public void BufferedStreamCanSkip() { - using (MemoryStream stream = this.CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) { byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(stream)) + using (var reader = new BufferedReadStream(this.configuration, stream)) { int skip = 50; int plusOne = 1; - int skip2 = BufferedReadStream.BufferLength; + int skip2 = this.bufferSize; // Skip reader.Skip(skip); @@ -216,18 +225,18 @@ public void BufferedStreamCanSkip() public void BufferedStreamReadsSmallStream() { // Create a stream smaller than the default buffer length - using (MemoryStream stream = this.CreateTestStream(BufferedReadStream.BufferLength / 4)) + using (MemoryStream stream = this.CreateTestStream(this.bufferSize / 4)) { byte[] expected = stream.ToArray(); const int offset = 5; - using (var reader = new BufferedReadStream(stream)) + using (var reader = new BufferedReadStream(this.configuration, stream)) { reader.Position = offset; Assert.Equal(expected[offset], reader.ReadByte()); // We've read a whole length of the stream but increment by 1 in our reader. - Assert.Equal(BufferedReadStream.BufferLength / 4, stream.Position); + Assert.Equal(this.bufferSize / 4, stream.Position); Assert.Equal(offset + 1, reader.Position); } @@ -238,10 +247,10 @@ public void BufferedStreamReadsSmallStream() [Fact] public void BufferedStreamReadsCanReadAllAsSingleByteFromOrigin() { - using (MemoryStream stream = this.CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) { byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(stream)) + using (var reader = new BufferedReadStream(this.configuration, stream)) { for (int i = 0; i < expected.Length; i++) { @@ -251,7 +260,7 @@ public void BufferedStreamReadsCanReadAllAsSingleByteFromOrigin() } } - private MemoryStream CreateTestStream(int length = BufferedReadStream.BufferLength * 3) + private MemoryStream CreateTestStream(int length) { var buffer = new byte[length]; var random = new Random(); From 8fe053fa8ae3aa86a17ab200f65fb112e789df7f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 24 Jul 2020 08:24:06 +0100 Subject: [PATCH 2/6] Update ConfigurationTests.cs --- tests/ImageSharp.Tests/ConfigurationTests.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/ImageSharp.Tests/ConfigurationTests.cs b/tests/ImageSharp.Tests/ConfigurationTests.cs index 3507d5c4de..32fd5202c6 100644 --- a/tests/ImageSharp.Tests/ConfigurationTests.cs +++ b/tests/ImageSharp.Tests/ConfigurationTests.cs @@ -76,10 +76,7 @@ public void MaxDegreeOfParallelism_CompatibleWith_ParallelOptions(int maxDegreeO if (throws) { Assert.Throws( - () => - { - cfg.MaxDegreeOfParallelism = maxDegreeOfParallelism; - }); + () => cfg.MaxDegreeOfParallelism = maxDegreeOfParallelism); } else { @@ -122,7 +119,7 @@ public void ConfigurationCannotAddDuplicates() [Fact] public void DefaultConfigurationHasCorrectFormatCount() { - Configuration config = Configuration.CreateDefaultInstance(); + var config = Configuration.CreateDefaultInstance(); Assert.Equal(this.expectedDefaultConfigurationCount, config.ImageFormats.Count()); } @@ -140,5 +137,16 @@ public void StreamBufferSize_DefaultIsCorrect() Configuration config = this.DefaultConfiguration; Assert.True(config.StreamProcessingBufferSize == 8096); } + + [Fact] + public void StreamBufferSize_CannotGoBelowMinimum() + { + var config = new Configuration + { + StreamProcessingBufferSize = 0 + }; + + Assert.Equal(128, config.StreamProcessingBufferSize); + } } } From 10b57fdb7ad9902f7a9a579b78bd5236e43d5d69 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 24 Jul 2020 18:25:07 +0100 Subject: [PATCH 3/6] Throw, don't adjust. --- src/ImageSharp/Configuration.cs | 9 +++------ tests/ImageSharp.Tests/ConfigurationTests.cs | 8 +++----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/ImageSharp/Configuration.cs b/src/ImageSharp/Configuration.cs index b96204ed9e..062bcb229c 100644 --- a/src/ImageSharp/Configuration.cs +++ b/src/ImageSharp/Configuration.cs @@ -25,8 +25,6 @@ public sealed class Configuration /// A lazily initialized configuration default instance. /// private static readonly Lazy Lazy = new Lazy(CreateDefaultInstance); - - private const int MinStreamProcessingBufferSize = 128; private const int DefaultStreamProcessingBufferSize = 8096; private int streamProcessingBufferSize = DefaultStreamProcessingBufferSize; private int maxDegreeOfParallelism = Environment.ProcessorCount; @@ -79,17 +77,16 @@ public int MaxDegreeOfParallelism /// /// Gets or sets the size of the buffer to use when working with streams. - /// Intitialized with by default - /// and can accept a minimum value of . + /// Intitialized with by default. /// public int StreamProcessingBufferSize { get => this.streamProcessingBufferSize; set { - if (value < MinStreamProcessingBufferSize) + if (value <= 0) { - value = MinStreamProcessingBufferSize; + throw new ArgumentOutOfRangeException(nameof(this.StreamProcessingBufferSize)); } this.streamProcessingBufferSize = value; diff --git a/tests/ImageSharp.Tests/ConfigurationTests.cs b/tests/ImageSharp.Tests/ConfigurationTests.cs index 32fd5202c6..655e98c7f6 100644 --- a/tests/ImageSharp.Tests/ConfigurationTests.cs +++ b/tests/ImageSharp.Tests/ConfigurationTests.cs @@ -141,12 +141,10 @@ public void StreamBufferSize_DefaultIsCorrect() [Fact] public void StreamBufferSize_CannotGoBelowMinimum() { - var config = new Configuration - { - StreamProcessingBufferSize = 0 - }; + var config = new Configuration(); - Assert.Equal(128, config.StreamProcessingBufferSize); + Assert.Throws( + () => config.StreamProcessingBufferSize = 0); } } } From 881b79b00f8e532b73df12a282f78dc7bd45a7d4 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 25 Jul 2020 19:24:27 +0100 Subject: [PATCH 4/6] Sneak in some docs fixes --- .github/CONTRIBUTING.md | 12 +++++------- .github/ISSUE_TEMPLATE/config.yml | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 346bfd5340..89d1a75f27 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -1,4 +1,4 @@ -# How to contribute to ImageSharp +# How to contribute to SixLabors.ImageSharp #### **Did you find a bug?** @@ -12,11 +12,11 @@ * Ensure the PR description clearly describes the problem and solution. Include the relevant issue number if applicable. -* Before submitting, please ensure that your code matches the existing coding patterns and practise as demonstrated in the repository. These follow strict Stylecop rules :cop:. +* Before submitting, please ensure that your code matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules :cop:. #### **Do you intend to add a new feature or change an existing one?** -* Suggest your change in the [ImageSharp Gitter Chat Room](https://gitter.im/ImageSharp/General) and start writing code. +* Suggest your change in the [Ideas Discussions Channel](https://github.com/SixLabors/ImageSharp/discussions?discussions_q=category%3AIdeas) and start writing code. * Do not open an issue on GitHub until you have collected positive feedback about the change. GitHub issues are primarily intended for bug reports and fixes. @@ -33,14 +33,12 @@ #### **Do you have questions about consuming the library or the source code?** -* Ask any question about how to use ImageSharp over in the [discussions section](https://github.com/SixLabors/ImageSharp/discussions). +* Ask any question about how to use SixLabors.ImageSharp in the [Help Discussions Channel](https://github.com/SixLabors/ImageSharp/discussions?discussions_q=category%3AHelp). #### Code of Conduct This project has adopted the code of conduct defined by the [Contributor Covenant](https://contributor-covenant.org/) to clarify expected behavior in our community. For more information, see the [.NET Foundation Code of Conduct](https://dotnetfoundation.org/code-of-conduct). -And please remember. ImageSharp is the work of a very, very, small number of developers who struggle balancing time to contribute to the project with family time and work commitments. We encourage you to pitch in and help make our vision of simple accessible imageprocessing available to all. Open Source can only exist with your help. +And please remember. SixLabors.ImageSharp is the work of a very, very, small number of developers who struggle balancing time to contribute to the project with family time and work commitments. We encourage you to pitch in and help make our vision of simple accessible image processing available to all. Open Source can only exist with your help. Thanks for reading! - -James Jackson-South :heart: diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 5a9d1dde09..cf9f787526 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,8 +1,8 @@ blank_issues_enabled: false contact_links: - name: Ask a Question - url: https://github.com/SixLabors/ImageSharp/discussions/new?category_id=6331980 + url: https://github.com/SixLabors/ImageSharp/discussions?discussions_q=category%3AHelp about: Ask a question about this project. - name: Feature Request - url: https://github.com/SixLabors/ImageSharp/discussions/new?category_id=6331981 + url: https://github.com/SixLabors/ImageSharp/discussions?discussions_q=category%3AIdeas about: Share ideas for new features for this project. From 91a978336b52e63169f87b53a98851a283b08dbd Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 26 Jul 2020 15:27:06 +0100 Subject: [PATCH 5/6] Add sanitation and make tests parametric. --- .../Formats/Jpeg/JpegDecoderCore.cs | 11 + src/ImageSharp/IO/BufferedReadStream.cs | 5 +- .../IO/BufferedReadStreamTests.cs | 209 ++++++++++++------ 3 files changed, 153 insertions(+), 72 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 2956d2c11b..cdf9060764 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -514,6 +514,11 @@ private void ProcessApplicationHeaderMarker(BufferedReadStream stream, int remai // TODO: thumbnail if (remaining > 0) { + if (stream.Position + remaining >= stream.Length) + { + JpegThrowHelper.ThrowInvalidImageContentException("Bad App0 Marker length."); + } + stream.Skip(remaining); } } @@ -533,6 +538,11 @@ private void ProcessApp1Marker(BufferedReadStream stream, int remaining) return; } + if (stream.Position + remaining >= stream.Length) + { + JpegThrowHelper.ThrowInvalidImageContentException("Bad App1 Marker length."); + } + var profile = new byte[remaining]; stream.Read(profile, 0, remaining); @@ -550,6 +560,7 @@ private void ProcessApp1Marker(BufferedReadStream stream, int remaining) this.ExtendProfile(ref this.exifData, profile.AsSpan(Exif00).ToArray()); } } + } /// diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 54d919963e..02015eb56a 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -50,8 +50,8 @@ public BufferedReadStream(Configuration configuration, Stream stream) } this.BaseStream = stream; - this.Position = (int)stream.Position; this.Length = stream.Length; + this.Position = (int)stream.Position; this.BufferSize = configuration.StreamProcessingBufferSize; this.maxBufferIndex = this.BufferSize - 1; this.readBuffer = ArrayPool.Shared.Rent(this.BufferSize); @@ -86,6 +86,9 @@ public override long Position [MethodImpl(MethodImplOptions.NoInlining)] set { + Guard.MustBeGreaterThanOrEqualTo(value, 0, nameof(this.Position)); + Guard.MustBeLessThan(value, this.Length, nameof(this.Position)); + // Only reset readBufferIndex if we are out of bounds of our working buffer // otherwise we should simply move the value by the diff. if (this.IsInReadBuffer(value, out long index)) diff --git a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs index b15093a3d1..6ceaca012e 100644 --- a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs @@ -11,18 +11,27 @@ namespace SixLabors.ImageSharp.Tests.IO public class BufferedReadStreamTests { private readonly Configuration configuration; - private readonly int bufferSize; public BufferedReadStreamTests() { - this.configuration = Configuration.Default; - this.bufferSize = this.configuration.StreamProcessingBufferSize; + this.configuration = Configuration.CreateDefaultInstance(); } - [Fact] - public void BufferedStreamCanReadSingleByteFromOrigin() + public static readonly TheoryData BufferSizes = + new TheoryData() + { + 1, 2, 4, 8, + 16, 97, 503, + 719, 1024, + 8096, 64768 + }; + + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamCanReadSingleByteFromOrigin(int bufferSize) { - using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) + this.configuration.StreamProcessingBufferSize = bufferSize; + using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) { byte[] expected = stream.ToArray(); using (var reader = new BufferedReadStream(this.configuration, stream)) @@ -30,7 +39,7 @@ public void BufferedStreamCanReadSingleByteFromOrigin() Assert.Equal(expected[0], reader.ReadByte()); // We've read a whole chunk but increment by 1 in our reader. - Assert.Equal(this.bufferSize, stream.Position); + Assert.True(stream.Position >= bufferSize); Assert.Equal(1, reader.Position); } @@ -39,13 +48,15 @@ public void BufferedStreamCanReadSingleByteFromOrigin() } } - [Fact] - public void BufferedStreamCanReadSingleByteFromOffset() + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamCanReadSingleByteFromOffset(int bufferSize) { - using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) + this.configuration.StreamProcessingBufferSize = bufferSize; + using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) { byte[] expected = stream.ToArray(); - const int offset = 5; + int offset = expected.Length / 2; using (var reader = new BufferedReadStream(this.configuration, stream)) { reader.Position = offset; @@ -53,7 +64,7 @@ public void BufferedStreamCanReadSingleByteFromOffset() Assert.Equal(expected[offset], reader.ReadByte()); // We've read a whole chunk but increment by 1 in our reader. - Assert.Equal(this.bufferSize + offset, stream.Position); + Assert.Equal(bufferSize + offset, stream.Position); Assert.Equal(offset + 1, reader.Position); } @@ -61,10 +72,12 @@ public void BufferedStreamCanReadSingleByteFromOffset() } } - [Fact] - public void BufferedStreamCanReadSubsequentSingleByteCorrectly() + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamCanReadSubsequentSingleByteCorrectly(int bufferSize) { - using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) + this.configuration.StreamProcessingBufferSize = bufferSize; + using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) { byte[] expected = stream.ToArray(); int i; @@ -75,19 +88,19 @@ public void BufferedStreamCanReadSubsequentSingleByteCorrectly() Assert.Equal(expected[i], reader.ReadByte()); Assert.Equal(i + 1, reader.Position); - if (i < this.bufferSize) + if (i < bufferSize) { - Assert.Equal(stream.Position, this.bufferSize); + Assert.Equal(stream.Position, bufferSize); } - else if (i >= this.bufferSize && i < this.bufferSize * 2) + else if (i >= bufferSize && i < bufferSize * 2) { // We should have advanced to the second chunk now. - Assert.Equal(stream.Position, this.bufferSize * 2); + Assert.Equal(stream.Position, bufferSize * 2); } else { // We should have advanced to the third chunk now. - Assert.Equal(stream.Position, this.bufferSize * 3); + Assert.Equal(stream.Position, bufferSize * 3); } } } @@ -96,10 +109,12 @@ public void BufferedStreamCanReadSubsequentSingleByteCorrectly() } } - [Fact] - public void BufferedStreamCanReadMultipleBytesFromOrigin() + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamCanReadMultipleBytesFromOrigin(int bufferSize) { - using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) + this.configuration.StreamProcessingBufferSize = bufferSize; + using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) { var buffer = new byte[2]; byte[] expected = stream.ToArray(); @@ -110,95 +125,127 @@ public void BufferedStreamCanReadMultipleBytesFromOrigin() Assert.Equal(expected[1], buffer[1]); // We've read a whole chunk but increment by the buffer length in our reader. - Assert.Equal(stream.Position, this.bufferSize); + Assert.True(stream.Position >= bufferSize); Assert.Equal(buffer.Length, reader.Position); } } } - [Fact] - public void BufferedStreamCanReadSubsequentMultipleByteCorrectly() + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamCanReadSubsequentMultipleByteCorrectly(int bufferSize) { - using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) + this.configuration.StreamProcessingBufferSize = bufferSize; + using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) { + const int increment = 2; var buffer = new byte[2]; byte[] expected = stream.ToArray(); using (var reader = new BufferedReadStream(this.configuration, stream)) { - for (int i = 0, o = 0; i < expected.Length / 2; i++, o += 2) + for (int i = 0, o = 0; i < expected.Length / increment; i++, o += increment) { - Assert.Equal(2, reader.Read(buffer, 0, 2)); + // Check values are correct. + Assert.Equal(increment, reader.Read(buffer, 0, increment)); Assert.Equal(expected[o], buffer[0]); Assert.Equal(expected[o + 1], buffer[1]); - Assert.Equal(o + 2, reader.Position); + Assert.Equal(o + increment, reader.Position); + + // These tests ensure that we are correctly reading + // our buffer in chunks of the given size. + int offset = i * increment; - int offset = i * 2; - if (offset < this.bufferSize) + // First chunk. + if (offset < bufferSize) { - Assert.Equal(stream.Position, this.bufferSize); + // We've read an entire chunk once and are + // now reading from that chunk. + Assert.True(stream.Position >= bufferSize); + continue; } - else if (offset >= this.bufferSize && offset < this.bufferSize * 2) - { - // We should have advanced to the second chunk now. - Assert.Equal(stream.Position, this.bufferSize * 2); - } - else + + // Second chunk + if (offset < bufferSize * 2) { - // We should have advanced to the third chunk now. - Assert.Equal(stream.Position, this.bufferSize * 3); + Assert.True(stream.Position > bufferSize); + + // Odd buffer size with even increments can + // jump to the third chunk on final read. + Assert.True(stream.Position <= bufferSize * 3); + continue; } + + // Third chunk + Assert.True(stream.Position > bufferSize * 2); } } } } - [Fact] - public void BufferedStreamCanReadSubsequentMultipleByteSpanCorrectly() + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamCanReadSubsequentMultipleByteSpanCorrectly(int bufferSize) { - using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) + this.configuration.StreamProcessingBufferSize = bufferSize; + using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) { + const int increment = 2; Span buffer = new byte[2]; byte[] expected = stream.ToArray(); using (var reader = new BufferedReadStream(this.configuration, stream)) { - for (int i = 0, o = 0; i < expected.Length / 2; i++, o += 2) + for (int i = 0, o = 0; i < expected.Length / increment; i++, o += increment) { - Assert.Equal(2, reader.Read(buffer, 0, 2)); + // Check values are correct. + Assert.Equal(increment, reader.Read(buffer, 0, increment)); Assert.Equal(expected[o], buffer[0]); Assert.Equal(expected[o + 1], buffer[1]); - Assert.Equal(o + 2, reader.Position); + Assert.Equal(o + increment, reader.Position); - int offset = i * 2; - if (offset < this.bufferSize) - { - Assert.Equal(stream.Position, this.bufferSize); - } - else if (offset >= this.bufferSize && offset < this.bufferSize * 2) + // These tests ensure that we are correctly reading + // our buffer in chunks of the given size. + int offset = i * increment; + + // First chunk. + if (offset < bufferSize) { - // We should have advanced to the second chunk now. - Assert.Equal(stream.Position, this.bufferSize * 2); + // We've read an entire chunk once and are + // now reading from that chunk. + Assert.True(stream.Position >= bufferSize); + continue; } - else + + // Second chunk + if (offset < bufferSize * 2) { - // We should have advanced to the third chunk now. - Assert.Equal(stream.Position, this.bufferSize * 3); + Assert.True(stream.Position > bufferSize); + + // Odd buffer size with even increments can + // jump to the third chunk on final read. + Assert.True(stream.Position <= bufferSize * 3); + continue; } + + // Third chunk + Assert.True(stream.Position > bufferSize * 2); } } } } - [Fact] - public void BufferedStreamCanSkip() + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamCanSkip(int bufferSize) { - using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) + this.configuration.StreamProcessingBufferSize = bufferSize; + using (MemoryStream stream = this.CreateTestStream(bufferSize * 4)) { byte[] expected = stream.ToArray(); using (var reader = new BufferedReadStream(this.configuration, stream)) { - int skip = 50; + int skip = 1; int plusOne = 1; - int skip2 = this.bufferSize; + int skip2 = bufferSize; // Skip reader.Skip(skip); @@ -221,14 +268,17 @@ public void BufferedStreamCanSkip() } } - [Fact] - public void BufferedStreamReadsSmallStream() + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamReadsSmallStream(int bufferSize) { + this.configuration.StreamProcessingBufferSize = bufferSize; + // Create a stream smaller than the default buffer length - using (MemoryStream stream = this.CreateTestStream(this.bufferSize / 4)) + using (MemoryStream stream = this.CreateTestStream(Math.Max(1, bufferSize / 4))) { byte[] expected = stream.ToArray(); - const int offset = 5; + int offset = expected.Length / 2; using (var reader = new BufferedReadStream(this.configuration, stream)) { reader.Position = offset; @@ -236,7 +286,7 @@ public void BufferedStreamReadsSmallStream() Assert.Equal(expected[offset], reader.ReadByte()); // We've read a whole length of the stream but increment by 1 in our reader. - Assert.Equal(this.bufferSize / 4, stream.Position); + Assert.Equal(Math.Max(1, bufferSize / 4), stream.Position); Assert.Equal(offset + 1, reader.Position); } @@ -244,10 +294,12 @@ public void BufferedStreamReadsSmallStream() } } - [Fact] - public void BufferedStreamReadsCanReadAllAsSingleByteFromOrigin() + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamReadsCanReadAllAsSingleByteFromOrigin(int bufferSize) { - using (MemoryStream stream = this.CreateTestStream(this.bufferSize * 3)) + this.configuration.StreamProcessingBufferSize = bufferSize; + using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) { byte[] expected = stream.ToArray(); using (var reader = new BufferedReadStream(this.configuration, stream)) @@ -260,6 +312,21 @@ public void BufferedStreamReadsCanReadAllAsSingleByteFromOrigin() } } + [Theory] + [MemberData(nameof(BufferSizes))] + public void BufferedStreamThrowsOnBadPosition(int bufferSize) + { + this.configuration.StreamProcessingBufferSize = bufferSize; + using (MemoryStream stream = this.CreateTestStream(bufferSize)) + { + using (var reader = new BufferedReadStream(this.configuration, stream)) + { + Assert.Throws(() => reader.Position = -stream.Length); + Assert.Throws(() => reader.Position = stream.Length); + } + } + } + private MemoryStream CreateTestStream(int length) { var buffer = new byte[length]; From 6aa647231e014ff54eb77e373e763f8a6729b481 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 26 Jul 2020 15:38:00 +0100 Subject: [PATCH 6/6] Fix formatting. --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index cdf9060764..c0b09c4c2b 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -560,7 +560,6 @@ private void ProcessApp1Marker(BufferedReadStream stream, int remaining) this.ExtendProfile(ref this.exifData, profile.AsSpan(Exif00).ToArray()); } } - } ///