ZipPackagePart.GetStreamCore crashes with NotSupportedException

[rladuca]

This spawned off investigations done in dotnet/wpf#1363.

A simple reproduction in a console application can be obtained by doing:

var zp = ZipPackage.Open("test.zip", System.IO.FileMode.Create, System.IO.FileAccess.Write);
var part = zp.CreatePart(new System.Uri("/test.txt", UriKind.Relative), "");
var stream = part.GetStream(System.IO.FileMode.Create);

A NotSupportedException will arise due to this code:

https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.IO.Packaging/src/System/IO/Packaging/ZipPackagePart.cs#L30-L36

The call to SetLength fails since the underlying stream does not support this.

The flow here is:

• https://github.com/dotnet/corefx/blob/78589e4d2c98bf71cefa4dbc94cde3783b60a934/src/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs#L290
• ZipArchiveEntry.GetDataCompressor creates a new DeflateStream which is then wrapped, along with the original stream, by CheckSumAndSizeWriteStream.
  https://github.com/dotnet/corefx/blob/78589e4d2c98bf71cefa4dbc94cde3783b60a934/src/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs#L600
• CheckSumAndSizeWriteStream is never seekable and doesn't support SetPosition:
  https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs#L364

It seems that the assumption about the underlying stream is incorrect or the underlying stream should be allowing for this functionality.

[rladuca]

Note that this is breaking many different interactions with XPS documents in WPF.

[stephentoub]

cc: @ericstj

[danmoseley]

"System.NotSupportedException: This stream from ZipArchiveEntry does not support seeking.
   at System.IO.Compression.WrappedStream.ThrowIfCantSeek()
   at System.IO.Compression.WrappedStream.SetLength(Int64 value)
   at System.IO.Packaging.ZipWrappingStream.SetLength(Int64 value)
   at System.IO.Packaging.ZipPackagePart.GetStreamCore(FileMode streamFileMode, FileAccess streamFileAccess)
   at System.IO.Packaging.PackagePart.GetStream(FileMode mode, FileAccess access)
   at System.IO.Packaging.PackagePart.GetStream(FileMode mode)
   at ConsoleApp45.Program.Main(String[] args) in C:\Users\danmose\source\repos\ConsoleApp45\ConsoleApp45\Program.cs:line 12

[ericstj]

/cc @stevenbrix
This is a case of the old Windows-base ported code not matching the behavior of IO.Compression. It's expected that ZipArchiveEntry returns non-seekable streams. Let me take a look and see if I can suggest a fix.

[ericstj]

ZipArchiveEntry only ever supports opening once when the backing archive is in Create mode, so this call to SetLength is unnecessary and should be removed in that case. You could still open an archive in Update mode then call part.GetStream(FileMode.Create). If the check was removed entirely that would change the behavior in that case, so I believe we just need to further condition that check to not attempt to SetLength when the backing Archive is in Create mode.

[stevenbrix]

@rladuca said he found a workaround: dotnet/wpf#1363 (comment)

[rladuca]

@stevenbrix This isn't really a broad workaround. While it may be fine in instances where the developer is directly controlling the opening of a Package, they could be using libraries that are doing Package manipulation underneath that they may not be able to influence.

The seekability in these modes is going to have to be a documented breaking change from 4.8 for sure, but valid requests to open a Package shouldn't just crash.

[stevenbrix]

Sorry if I sent any wrong vibes, I want to be clear that I'm not advocating we don't fix this bug. As @ericstj mentioned earlier, the call to SetLength isn't necessary here. I just wanted to make sure it was documented on this issue that there is a workaround.

but valid requests to open a Package shouldn't just crash.

I totally agree, and it would be really nice if there was some sort of matrix that helped us understand what combination of requests are valid, and in which scenario the stream returned supports streaming and when it doesn't. It isn't exactly intuitive that passing FileAccess.ReadWrite returns a seekable stream whereas just passing FileAccess.Write doesn't. If the expected behavior was documented (hopefully with some explanation) then we could better understand whether or not the implementation matches those.

[rladuca]

I agree we should have this documented and not just at the Packaging level. WPF should provide this matrix for its own entry points that plumb down here as well. We don't want people hunting for implementation details just to find out how to use the API.

[rladuca]

Is a fix here going to make the 3.0 milestone?

[ericstj]

We still have time. @stevenbrix was this something you wanted to do or should we assign to someone else?

[stevenbrix]

@ericstj I can make the fix

[stevenbrix]

Reopening for release/3.0 port

[JeremyKuhne]

Looks like this is resolved now.

[ericstj]

Thanks @stevenbrix for the fix!