Conversation
Android zip progress grew its total while emitting, so a file followed by a folder could move progress backwards. unzipAssets added compressed sizes that ZipInputStream often reports as -1. Full unzip skipped directory entries, and several missing-archive paths rejected with a generic code. Count zip work up front, attribute unknown asset sizes to bytes copied, create directory entries, fsync successful Android zips, and reject missing archives with ERR_FILE_NOT_FOUND. An AbortSignal that flips aborted before the listener is attached now rejects without starting native work. Co-authored-by: plrthink <plrthink@gmail.com>
The same tests fail on the previous behavior: zip progress hits 100% and then rewinds, unzipAssets progress drops when a compressed size is -1, empty directories are dropped, a bare AES method throws, a missing archive maps to ERR_UNZIP, and an AbortSignal that flips during addEventListener still resolves the zip. Route native zip progress and extract through ZipProgress and ZipExtractor so the tests execute the code the module calls. Co-authored-by: plrthink <plrthink@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Android zip progress counted work while it emitted, so a file followed by a folder could move progress backwards.
unzipAssetsaddedZipEntry.getCompressedSize(), whichZipInputStreamoften reports as-1. A few other native paths disagreed with the documented error and extract behavior.End-to-end tests now drive the same code the module calls (
ZipProgress.events,ZipProgress.advanceAssetBytes,ZipExtractor.extractAll,ZipErrorCodes.mapException,ZipEncryptionChoice.parse, and JSzip()). They were run against the previous behavior first.Red — same tests, previous behavior
Java, 6 failures:
[0.0, 1.0, 0.6666666666666666, 1.0, 1.0]— first unit already 100%, then the value rewindsunzipAssetsprogress:[0.0, 0.2, 0.199, 1.0]after a compressed size of-1note.txt.rejected as Zip SlipAESthrewArrayIndexOutOfBoundsExceptionERR_UNZIP(zip file does not exist, cannot read comment)JS
zip()against the previousindex.js: the signal that flips to aborted insideaddEventListenerresolved"/mock/path.zip"instead of rejectingERR_CANCELLED.Green — same tests, this branch
__tests__/zip-archive.e2e.test.js2 tests passed; full suite 70 tests passedBehavior
zip/zipWithPasswordcount work units before the first progress event.unzipAssetsuses bytes copied when the compressed size is unknown. Progress stays within 0–1.unzip/unzipWithPasswordcreate directory entries, including empty directories. An entry that resolves to the destination itself is accepted; a sibling prefix such asdest-evilis still rejected.ERR_FILE_NOT_FOUND.ERR_UNSUPPORTED. A bareAESmethod is AES-128. An unknown method stays ZipCrypto.AbortSignalthat aborts before the listener is attached rejects withERR_CANCELLEDand does not start native work.JavaScript call sites are unchanged. A native rebuild is required.
Test plan
npm test(70 tests)npm run lintnpm run test:docs-sync