From fa5865cb710863b694e2049523495ea0a6630ef7 Mon Sep 17 00:00:00 2001 From: "Ricardo Bossan (BEYONDSOFT CONSULTING INC) (from Dev Box)" Date: Mon, 6 Jul 2026 16:37:55 -0300 Subject: [PATCH] Fixes #14064 - Fix `ComManagedStream.Read` (the COM `IStream` wrapper used when a managed `Stream` is handed to GDI+) to keep reading until the requested buffer is filled or the stream reaches EOF, instead of issuing a single `Stream.Read`. - This repairs `Image.FromStream` / `Bitmap.FromStream` / `new Bitmap(stream)` / `Metafile(stream)` for any stream that legally returns fewer bytes than requested (chunked / network / zip-wrapped), which GDI+ was rejecting with `Parameter is not valid.` - Add regression tests for the short-read case and the read-past-EOF case. - Loading images through `System.Drawing` from a stream that returns partial reads now succeeds instead of throwing `ArgumentException: Parameter is not valid.` Reported in #14064 (and earlier in #8824). - No change for streams that already returned full reads (`FileStream`, `MemoryStream`, ...); those keep working exactly as before. - No. The single-read behavior was ported verbatim from the legacy `GPStream` (git blame) and predates that; #8824 shows the same failure on .NET Core 3.1 / System.Drawing.Common 4.7.0. - Low. The change is confined to one method and only adds a fill loop: it returns identical results for full-read streams and the correct total for short-read streams. The COM `IStream::Read` contract explicitly allows satisfying a request across multiple source reads. - New unit tests in `ComManagedStreamTests` call `IStream::Read` directly against a stream that caps every read: one asserts the buffer is filled across chunks, the other asserts a request past EOF returns only the available bytes without throwing. - Manual end-to-end check using the attached `Program.cs` (paste it into `src/test/integration/ScratchProject/Program.cs` and run): it decodes an in-memory PNG through a 32-byte-per-read stream via `Image.FromStream` and shows the loaded image. Throws `Parameter is not valid.` before the fix, loads after. - Full `System.Private.Windows.Core.Tests` suite. - 11.0.100-preview.5.26302.115 --- .../Win32/System/Com/ComManagedStream.cs | 15 ++- .../Win32/System/Com/ComManagedStreamTests.cs | 106 ++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/src/System.Private.Windows.Core/src/Windows/Win32/System/Com/ComManagedStream.cs b/src/System.Private.Windows.Core/src/Windows/Win32/System/Com/ComManagedStream.cs index 395aca540c3..ec089192330 100644 --- a/src/System.Private.Windows.Core/src/Windows/Win32/System/Com/ComManagedStream.cs +++ b/src/System.Private.Windows.Core/src/Windows/Win32/System/Com/ComManagedStream.cs @@ -122,7 +122,20 @@ HRESULT ISequentialStream.Interface.Read(void* pv, uint cb, uint* pcbRead) ActualizeVirtualPosition(); Span buffer = new(pv, checked((int)cb)); - int read = _dataStream.Read(buffer); + + // Stream.Read can legally return fewer bytes than requested (e.g. chunked/network streams). + // Some callers (GDI+) treat a short read as failure, so fill the buffer until EOF. + int read = 0; + while (read < buffer.Length) + { + int bytesRead = _dataStream.Read(buffer[read..]); + if (bytesRead == 0) + { + break; + } + + read += bytesRead; + } if (pcbRead is not null) *pcbRead = (uint)read; diff --git a/src/System.Private.Windows.Core/tests/System.Private.Windows.Core.Tests/Windows/Win32/System/Com/ComManagedStreamTests.cs b/src/System.Private.Windows.Core/tests/System.Private.Windows.Core.Tests/Windows/Win32/System/Com/ComManagedStreamTests.cs index 47117e5e100..4d1457ff6bc 100644 --- a/src/System.Private.Windows.Core/tests/System.Private.Windows.Core.Tests/Windows/Win32/System/Com/ComManagedStreamTests.cs +++ b/src/System.Private.Windows.Core/tests/System.Private.Windows.Core.Tests/Windows/Win32/System/Com/ComManagedStreamTests.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Windows.Win32.Foundation; + namespace Windows.Win32.System.Com.Tests; public class ComManagedStreamTests @@ -22,6 +24,54 @@ public void Ctor_SeekableStream_UsesOriginalStream() comManagedStream.GetDataStream().Should().BeSameAs(seekableStream); } + [Fact] + public unsafe void Read_StreamReturnsShortReads_FillsBuffer() + { + // Streams are allowed to return fewer bytes than requested. Simulate a chunked/network stream + // and verify the wrapper keeps reading until the requested buffer is filled. See issue #14064. + byte[] sourceBytes = new byte[1024]; + for (int index = 0; index < sourceBytes.Length; index++) + { + sourceBytes[index] = (byte)index; + } + + using ChunkingStream shortReadStream = new(sourceBytes, chunkSize: 100); + ComManagedStream comManagedStream = new(shortReadStream); + + byte[] destinationBuffer = new byte[sourceBytes.Length]; + uint bytesReadCount; + fixed (byte* destinationPointer = destinationBuffer) + { + ((IStream.Interface)comManagedStream).Read(destinationPointer, (uint)destinationBuffer.Length, &bytesReadCount).Should().Be(HRESULT.S_OK); + } + + bytesReadCount.Should().Be((uint)sourceBytes.Length); + destinationBuffer.Should().Equal(sourceBytes); + } + + [Fact] + public unsafe void Read_RequestPastEndOfStream_ReturnsOnlyAvailableBytes() + { + // When more bytes are requested than remain, only the available bytes are returned (no throw on EOF). + byte[] sourceBytes = new byte[50]; + for (int index = 0; index < sourceBytes.Length; index++) + { + sourceBytes[index] = (byte)index; + } + + using ChunkingStream shortReadStream = new(sourceBytes, chunkSize: 10); + ComManagedStream comManagedStream = new(shortReadStream); + + byte[] destinationBuffer = new byte[sourceBytes.Length * 2]; + uint bytesReadCount; + fixed (byte* destinationPointer = destinationBuffer) + { + ((IStream.Interface)comManagedStream).Read(destinationPointer, (uint)destinationBuffer.Length, &bytesReadCount).Should().Be(HRESULT.S_OK); + } + + bytesReadCount.Should().Be((uint)sourceBytes.Length); + } + private class TestStream : MemoryStream { private readonly bool _canSeek; @@ -33,4 +83,60 @@ public TestStream(bool canSeek, int numBytes) : base(new byte[numBytes]) _canSeek = canSeek; } } + + // Seekable stream that never returns more than a fixed chunk per Read, simulating chunked/network streams. + // Derives from Stream (not MemoryStream) so the span-based Read routes through this Read override on all targets. + private sealed class ChunkingStream : Stream + { + private readonly byte[] _sourceData; + private readonly int _maxBytesPerRead; + private int _position; + + public ChunkingStream(byte[] sourceData, int chunkSize) + { + _sourceData = sourceData; + _maxBytesPerRead = chunkSize; + } + + public override bool CanRead => true; + public override bool CanSeek => true; + public override bool CanWrite => false; + public override long Length => _sourceData.Length; + + public override long Position + { + get => _position; + set => _position = (int)value; + } + + public override int Read(byte[] buffer, int offset, int count) + { + int bytesToRead = Math.Min(Math.Min(count, _maxBytesPerRead), _sourceData.Length - _position); + if (bytesToRead <= 0) + { + return 0; + } + + Array.Copy(_sourceData, _position, buffer, offset, bytesToRead); + _position += bytesToRead; + return bytesToRead; + } + + public override long Seek(long offset, SeekOrigin origin) + { + _position = origin switch + { + SeekOrigin.Begin => (int)offset, + SeekOrigin.Current => _position + (int)offset, + SeekOrigin.End => _sourceData.Length + (int)offset, + _ => _position, + }; + + return _position; + } + + public override void Flush() { } + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } }