Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

fixing ZipPackagePart.GetStreamCore crashes with NotSupportedException#40319

Merged
stevenbrix merged 5 commits intomasterfrom
dev/stevenbrix/fix39816
Aug 15, 2019
Merged

fixing ZipPackagePart.GetStreamCore crashes with NotSupportedException#40319
stevenbrix merged 5 commits intomasterfrom
dev/stevenbrix/fix39816

Conversation

@stevenbrix
Copy link
Contributor

@stevenbrix stevenbrix commented Aug 14, 2019

The exception thrown here by ZipPackagePart is dubious and unnecessary, and an unintended mistake from the initial port of System.IO.Packaging from WindowsBase. ZipArchiveEntry only ever supports opening once when the backing archive is in Create mode, so the call to SetLength is unnecessary. You could still open an archive in Update mode then call part.GetStream(FileMode.Create), in which case we'll want this call to SetLength, so we only avoid this call when the backing Archive is in Create mode.

Fixes #39816

ZipArchiveEntry only ever supports opening once when the backing archive is in Create mode,  and the backing stream is non-seekable, so we shouldn't call SetLength in that case. You could still open an archive in Update mode then call part.GetStream(FileMode.Create), in which case we'll want this call to SetLength, so we only avoid this call when the backing Archive is in Create mode.
Copy link
Member

@rladuca rladuca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. The only suggestion may be to directly write a test case for the open in update, then use a part in Create scenario.

I saw it had coverage in other test functions, but it didn't seem called out as a specific scenario unless I missed that in my scan of the other tests.

@stevenbrix
Copy link
Contributor Author

stevenbrix commented Aug 14, 2019

@stephentoub some tests are failing in UWP due to the file system restrictions, i tried following the instructions here to try and repro the failure locally but run into these errors:

C:\Program Files\dotnet\sdk\3.0.100-preview9-013756\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.ConflictResolution.targets(39,5): error NETSDK1042: Could not load PlatformManifest from 'D:\dev\src\dotnet\corefx\artifacts\bin\testhost\uap-Windows_NT-Debug-x64\UAPLayout\PlatformManifest.txt' because it did not exist. [D:\dev\src\dotnet\corefx\src\Common\tests\CoreFx.Private.TestUtilities\CoreFx.Private.TestUtilities.csproj]

Is there something else i need to do?

@stephentoub
Copy link
Member

@ViktorHofer, can you help @stevenbrix?

@ViktorHofer
Copy link
Member

Sure. You need to build from root for uap first : build -f uap.

@stevenbrix stevenbrix merged commit 8ac53e0 into master Aug 15, 2019
@jkotas jkotas deleted the dev/stevenbrix/fix39816 branch August 19, 2019 12:25
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
dotnet/corefx#40319)

* fixing ZipPackagePart.GetStreamCore crashes with NotSupportedException

ZipArchiveEntry only ever supports opening once when the backing archive is in Create mode,  and the backing stream is non-seekable, so we shouldn't call SetLength in that case. You could still open an archive in Update mode then call part.GetStream(FileMode.Create), in which case we'll want this call to SetLength, so we only avoid this call when the backing Archive is in Create mode.

* updating test to explicitly test the Update path for ZipPackage

* skip UAP since we don't have access to the file system to create the .zip

* undo accidental change to existing test

* removing unnecessary variable


Commit migrated from dotnet/corefx@8ac53e0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ZipPackagePart.GetStreamCore crashes with NotSupportedException

4 participants