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

Capture error details from SharpZipLib for invalid ZIPs #2287

Merged
merged 1 commit into from
Feb 18, 2018

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Feb 17, 2018

Problem

We continue to have occasional difficulties with ZIP file validation. #2285 highlights an issue of discrepant redundant data in the local file header versus the central directory.

These issues are difficult to diagnose. In 1.22.6, the exceptions look like:

CKAN.FileNotFoundKraken: Trying to install CivilianPopulation 2.0.12, but it's not downloaded or download is corrupted

And in HEAD, we have:

Unhandled Exception: CKAN.InvalidModuleFileKraken: C:\Users\User\AppData\Local\Temp\tmpF3A6.tmp is not a valid ZIP file: ZipFile.TestArchive(true) returned false

These are pretty generic errors, which don't tell us much other than something is wrong with the ZIP. Lots of leg work is left to do to figure out what's wrong.

Changes

Now we capture more details from SharpZipLib, allowing us to specify which entry in the ZIP had a problem, as well as which validation step was being done and the detailed message from the validation function:

CKAN.InvalidModuleFileKraken: /tmp/tmp14ac6bce.tmp is not a valid ZIP file: Error in step EntryHeader for CivilianPopulation/Experience/Traits.cfg: Exception during test - 'Extract version mismatch'

At a code level this is fairly awkward as error reporting goes. ZipFile.TestArchive doesn't throw exceptions and it returns only a bool. To capture its error messages, we need to pass a delegate as its third parameter, which receives incremental status updates as it goes through each file. When it finds an error, the delegate is called with the error message, and we save it off and use it for our return parameter.

Not a fix for #2285, but it's probably the closest we'll get.

@HebaruSan HebaruSan added Core (ckan.dll) Issues affecting the core part of CKAN Pull request labels Feb 17, 2018
@politas politas merged commit 361527e into KSP-CKAN:master Feb 18, 2018
@HebaruSan HebaruSan deleted the fix/zip-validation-details branch February 18, 2018 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants