Ensure ComManagedStream seekable wrapper stream has position of 0

[zivarah]

Fixes #12951

Proposed changes

• ComManagedStream will ensure that the stream wrapping a non-seekable source stream has a position of 0 at the end of its constructor.

Customer Impact

• Image.FromStream with a non-seekable stream should no longer throw an exception when loading .emf and .wmf images.

Regression?

• Yes

Risk

• Risk seems relatively low, in particular because this constructor created a stream with a position of 0 in .NET Framework and .NET Core 3.1. It seems unlikely that consumers are relying on a non-zero position.

Test methodology

• Added unit tests ensuring that the stream returned by ComManagedStream.GetDataStream() has the expected state.

Microsoft Reviewers: Open in CodeFlow

[zivarah]

@dotnet-policy-service agree

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.98329%. Comparing base (12d120b) to head (6fc354b).
Report is 10 commits behind head on main.

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main      #12953         +/-   ##
===================================================
+ Coverage   75.97906%   75.98329%   +0.00422%     
===================================================
  Files           3265        3265                 
  Lines         643272      643273          +1     
  Branches       47426       47426                 
===================================================
+ Hits          488752      488780         +28     
+ Misses        150962      150935         -27     
  Partials        3558        3558                 

Flag	Coverage Δ
Debug	75.98329% <100.00000%> (+0.00422%)	⬆️
integration	18.02996% <0.00000%> (+0.01357%)	⬆️
production	49.88590% <100.00000%> (+0.01028%)	⬆️
test	96.95378% <ø> (-0.00057%)	⬇️
unit	47.33428% <100.00000%> (+0.00088%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[zivarah]

I will note that it seems likely there are some other related issues at play here:

• It seems odd that only certain formats seem to fail (.jpg for example does not have any issues even without this fix)
• When the stream passed to the ComManagedStream constructor is seekable, we are not doing anything to ensure its position either (and weren't in .NET Framework/.NET Core 3.1)

So this fix should likely be regarded as a partial fix to restore pre-.NET 5 behavior and address that regression, but it may make sense for someone to follow up with more investigation.

[lucienmaloney]

Yeah, the image formats aside from .emf and .wmf seem to be seeking to the beginning unconditionally, which causes a problem if the stream is intentionally offset. So for example this code works:

MemoryStream memoryStream = new MemoryStream();
pngImageStream.CopyTo(memoryStream);
memoryStream.Position = 0;
Image.FromStream(memoryStream);

But this code fails when it shouldn't:

MemoryStream memoryStream = new MemoryStream();
memoryStream.Write(new byte[200], 0, 200);
pngImageStream.CopyTo(memoryStream);
memoryStream.Position = 200;
Image.FromStream(memoryStream);

Though fixing that won't be as easy as just not resetting the stream position for seekable streams, since it'd be a breaking change for callers. I guess same for making .emf and .wmf seek to the beginning, since that would break them from passing the second case above, even if it would make them behave consistently. I do wonder then, out of all the formats supported, which unconditionally seek to the start, and which don't.

[JeremyKuhne]

Thanks for digging into this!

[JeremyKuhne]

Marking as no merge temporarily as I try and write a test that repros the problem with WMF. Will address this today.