search

threerings / getdown

Download, Install, Update
Other
507 stars 125 forks source link

Digest exception when using a protected zip file (with password) in the application package #276

Open serge-xav opened 8 months ago

serge-xav commented 8 months ago

If I add a password protected zip file in the application package, I get the following error with version 1.8.6 : 

Exception in thread "main" java.io.IOException: Error computing digest for: externalProperties.zip
    at com.threerings.getdown.data.f.run(Unknown Source)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
Caused by: java.util.zip.ZipException: invalid CEN header (encrypted entry)
    at java.util.zip.ZipFile.open(Native Method)
    at java.util.zip.ZipFile.<init>(ZipFile.java:225)
    at java.util.zip.ZipFile.<init>(ZipFile.java:155)
    at java.util.zip.ZipFile.<init>(ZipFile.java:169)
    at com.threerings.getdown.data.Resource.computeDigest(Unknown Source)

The error is not present in version 1.7.X I think the cause is the pull request https://github.com/threerings/getdown/pull/206#issuecomment-482947615

I'm wondering if usage of zip files as resource, should be configurable, using a switch in getdown.txt file ? This would also close #275

bekoenig commented 8 months ago

Hi @serge-xav,

in my opinion computing zip file digest in a robust way is an important improvement. So https://github.com/threerings/getdown/issues/275 is the implication of a change to an unsafe implementation.

For your mentioned bug here, the best solution would be a fallback to the default digest calculcation, when the zip file is encrypted.

Another solution would be to change the extension to ".zip.dat", so getdown don't tries to open the file.

serge-xav commented 8 months ago

Hi @bekoenig, thanks for your comment. I will try to implement your workaround for encrypted zip files. If a correction is needed on Getdown side, do you know when/how the new digest calculation has been implemented for zip files ? I tried to take a look at it, but I don't know how to implement the "fallback to default digest calculation when zip file is encrypted"

bekoenig commented 8 months ago

In the next weeks getdown 2.0.0 will be released with different group-id (see https://github.com/bekoenig/getdown/discussions/1). This version is feature complete and waits for final release. After this, I can implement this fallback in https://github.com/threerings/getdown/blob/917b7c346d14e167a8714b27561a28601532ba4a/core/src/main/java/com/threerings/getdown/data/Resource.java#L62 and check for openable.

serge-xav commented 8 months ago

Hi again Benjamin, I did not see this announcment before, that's good news to see Getdown up and living. Do you want me to also create the issue on your project ?

bekoenig commented 8 months ago

You are welcome to open a new issue in my repository. I will try to fix it in the next month.

After the first release I will open a new issue here to mark this repository as deprecated and link to my fork. I'm new to github and opensource contribution, so I need time to adapt my wellknown processes.