Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when overwriting manually installed files #3349

Merged
merged 2 commits into from
Apr 15, 2021

Conversation

HebaruSan
Copy link
Member

Problem

If you try to overwrite a manually installed file with the current dev build, an unhandled exception is thrown:

Unhandled Exception: System.NotSupportedException: InflaterInputStream Length is not supported
   at ICSharpCode.SharpZipLib.Zip.Compression.Streams.InflaterInputStream.get_Length()
   at CKAN.ModuleInstaller.StreamsEqual(Stream s1, Stream s2)

Cause

icsharpcode/SharpZipLib#84 changed the zip lib to throw an exception if you ask it for the length of a file in a ZIP. Before that I think it would have returned the correct length, because ModuleInstaller.StreamsEqual correctly reported whether files were the same.

We upgraded to that version of the lib in #3329, so users are not yet affected by this.

Changes

Now ModuleInstaller.StreamsEqual compares its input streams without checking their lengths.

Fixes #3348.

@HebaruSan HebaruSan added Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Pull request labels Apr 15, 2021
@HebaruSan HebaruSan requested a review from DasSkelett April 15, 2021 18:42
@DasSkelett
Copy link
Member

Smart compiler 😁
image

@HebaruSan
Copy link
Member Author

Smart compiler

Heh, I thought about that, but a dumber compiler would complain about the function missing a return statement at the end.

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

And works like a charm!
(Files modified at beginning, middle, and end, should've all edge cases covered :P)

image

@DasSkelett DasSkelett merged commit 621db7e into KSP-CKAN:master Apr 15, 2021
@HebaruSan HebaruSan deleted the fix/zip-crash branch April 15, 2021 19:19
DasSkelett added a commit to DasSkelett/CKAN that referenced this pull request Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Possibly a new zip lib problem
2 participants