From 35df635591b0b10ba764bc5cc5ed726655034606 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 2 Oct 2026 04:34:29 +0000 Subject: [PATCH 1/2] fix: keep zip progress monotonic and align missing-file errors 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 --- AGENTS.md | 6 +- CHANGELOG.md | 9 + MIGRATION.md | 11 + README.md | 8 +- __tests__/api.test.js | 19 ++ .../com/rnziparchive/RNZipArchiveModule.java | 235 +++++++++++------- .../com/rnziparchive/ZipEncryptionChoice.java | 31 +++ .../java/com/rnziparchive/ZipErrorCodes.java | 9 + .../java/com/rnziparchive/ZipProgress.java | 77 ++++++ .../java/com/rnziparchive/ZipSecurity.java | 7 +- .../rnziparchive/ZipEncryptionChoiceTest.java | 27 ++ .../com/rnziparchive/ZipErrorCodesTest.java | 16 ++ .../com/rnziparchive/ZipProgressTest.java | 69 +++++ .../com/rnziparchive/ZipSecurityTest.java | 21 ++ index.js | 22 +- ios/RNZipArchive.mm | 4 + 16 files changed, 466 insertions(+), 105 deletions(-) create mode 100644 android/src/main/java/com/rnziparchive/ZipEncryptionChoice.java create mode 100644 android/src/main/java/com/rnziparchive/ZipProgress.java create mode 100644 android/src/test/java/com/rnziparchive/ZipEncryptionChoiceTest.java create mode 100644 android/src/test/java/com/rnziparchive/ZipProgressTest.java diff --git a/AGENTS.md b/AGENTS.md index 29cdf7b..7d73ff2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,8 +71,10 @@ Options objects (`{ signal }`, `{ entries }`, `{ compressionLevel }`) are JS-sid - **Threading:** Android zip/unzip on a single-thread executor (FIFO). iOS on a background serial queue. Never block the UI/main thread with archive I/O. `cancel()` must not wait behind the operation it should abort. - **Encryption default:** `'STANDARD'` = ZipCrypto (interop with Node/Java/`unzip`). AES = WinZip-AES; many server tools cannot open AES zips — default to STANDARD for off-device consumers. -- **Charset:** Android may honor non-UTF-8; iOS must reject non-UTF-8 with `ERR_UNSUPPORTED` (except where docs say charset is ignored, e.g. `getUncompressedSize` on iOS). -- **Progress:** Emit monotonic 0→1 with explicit start/end. Shape: `{ progress, filePath }`. Treat cross-platform divergence as a bug. +- **Charset:** Android may honor non-UTF-8; an invalid charset name rejects with `ERR_UNSUPPORTED`. iOS must reject non-UTF-8 with `ERR_UNSUPPORTED` (except where docs say charset is ignored, e.g. `getUncompressedSize` on iOS). +- **Progress:** Emit monotonic 0→1 with explicit start/end. Shape: `{ progress, filePath }`. Android `zip` counts work units before the first event. `unzipAssets` uses bytes copied when `ZipInputStream` reports compressed size `-1`, so progress cannot go negative. Treat cross-platform divergence as a bug. +- **Durability:** After a successful zip, fsync the archive before resolving (iOS `fsync`, Android `FileDescriptor.sync`) so a following read or upload sees the full file. +- **Missing sources:** Reject with `ERR_FILE_NOT_FOUND` when the archive path is missing (`unzip`, `unzipWithPassword`, `listContents`, `isPasswordProtected`, `getUncompressedSize`). Do not collapse that case into `ERR_UNZIP` / `ERR_CORRUPT_ARCHIVE`. ## Testing commands diff --git a/CHANGELOG.md b/CHANGELOG.md index de5322a..fce0353 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## [Unreleased] +### Fixed +- Android: `zip` / `zipWithPassword` progress stays monotonic. The work total is counted before the first event, so a mix of files and folders no longer moves progress backwards. +- Android: `unzipAssets` progress stays within 0–1 when an entry's compressed size is unknown (`ZipInputStream` reports `-1`). Directory entries in asset archives are created, and traversal paths are rejected. +- Android: full `unzip` / `unzipWithPassword` create directory entries, including empty directories (same as iOS and selective extract). An entry that resolves to the destination directory itself is accepted; sibling prefixes such as `dest-evil` stay rejected. +- Missing archives reject with `ERR_FILE_NOT_FOUND` from Android `unzipWithPassword`, `isPasswordProtected`, and `getUncompressedSize`, and from iOS `getUncompressedSize`. +- Android: an invalid charset name rejects with `ERR_UNSUPPORTED`. A bare `AES` encryption method (or any unknown method) no longer throws; `AES` is AES-128 and unknown methods stay ZipCrypto. +- Android: successful `zip` / `zipWithPassword` fsync the archive before resolving (iOS already did). +- JS: an `AbortSignal` that aborts before the abort listener is attached rejects with `ERR_CANCELLED` and does not start native work or cancel a different in-flight operation. + ## [9.5.2] - 2026-09-28 ### Changed diff --git a/MIGRATION.md b/MIGRATION.md index 2bcfc13..6e0bd64 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -27,6 +27,17 @@ Working examples: [playground-expo](./playground-expo/) and [playground-rn](./pl Old-arch proof is compile + link on RN **0.81.6** (`.github/workflows/old-arch.yml`), not device Maestro. See the [README matrix](./README.md#old-architecture-rn-070081). +## Unreleased + +Native behavior fixes. JavaScript call sites are unchanged. Rebuild the native app after upgrading. + +- **Android zip progress** stays monotonic, including when `zip` is given a mix of files and folders. The work total is counted before the first event. +- **Android `unzipAssets` progress** stays within 0–1 when an entry's compressed size is unknown. +- **Android full `unzip` / `unzipWithPassword`** create directory entries, including empty directories. iOS and selective extract already did. +- **Missing archives** reject with `ERR_FILE_NOT_FOUND` from Android `unzipWithPassword`, `isPasswordProtected`, and `getUncompressedSize`, and from iOS `getUncompressedSize`. +- **Android `zip` / `zipWithPassword`** fsync the archive before the promise resolves. iOS already did. +- **`AbortSignal`:** if the signal aborts before the listener is attached, the call rejects with `ERR_CANCELLED` and does not start native work. + ## v9.5.1 Android old-architecture load fix. JavaScript call sites are unchanged. Native rebuild required. diff --git a/README.md b/README.md index 7800c7a..dbf442e 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,7 @@ Zip a folder (`string`) or files/folders (`string[]`) to `target`. - Single file: `zip([file], target)`. - Array items may be directories; contents are added recursively (entry paths relative to that directory; empty dirs preserved). +- Progress events run from 0 to 1 and do not move backwards. The archive is flushed to disk before the promise resolves. - Third arg: compression level (`0`–`9`, or constants below) or `{ compressionLevel, signal }`. ```js @@ -159,7 +160,7 @@ await zipWithPassword(sourceDir, targetZip, 'password', { ### `unzip(source, target, charsetOrEntriesOrOptions?, entries?)` -Extract an archive. Optional `entries` extracts only those paths (directories include nested children). +Extract an archive. Optional `entries` extracts only those paths (directories include nested children). Directory entries are created, including empty directories. ```js await unzip(source, target) @@ -222,9 +223,10 @@ subscribe(({ progress, filePath }) => { /* progress 0…1 */ }) ``` - Event is **global** — match `filePath` to your operation, then call `.remove()`. +- Progress moves from 0 to 1 and does not go backwards. - `unzip` / `unzipWithPassword`: byte-weighted after each entry. -- `zip` / `zipWithPassword`: per-file. -- `unzipAssets` (Android): approximate vs compressed size. +- `zip` / `zipWithPassword`: per immediate file or folder. The total is counted before the first tick. +- `unzipAssets` (Android): approximate vs compressed size. An entry whose compressed size is unknown counts the bytes copied, and the value stays within 0–1. ### Error codes diff --git a/__tests__/api.test.js b/__tests__/api.test.js index 40a813b..11cce93 100644 --- a/__tests__/api.test.js +++ b/__tests__/api.test.js @@ -102,6 +102,25 @@ describe('react-native-zip-archive API', () => { code: ErrorCodes.CANCELLED, }); expect(mockRNZipArchive.zipFolder).not.toHaveBeenCalled(); + expect(mockRNZipArchive.cancel).not.toHaveBeenCalled(); + }); + + test('zip signal that aborts while the listener is attached does not start work', async () => { + const signal = { + aborted: false, + addEventListener() { + this.aborted = true; + }, + removeEventListener() {}, + }; + await expect( + zip('/source', '/target.zip', { signal }) + ).rejects.toMatchObject({ + name: 'ZipError', + code: ErrorCodes.CANCELLED, + }); + expect(mockRNZipArchive.zipFolder).not.toHaveBeenCalled(); + expect(mockRNZipArchive.cancel).not.toHaveBeenCalled(); }); test('zip abort mid-flight calls cancel', async () => { diff --git a/android/src/main/java/com/rnziparchive/RNZipArchiveModule.java b/android/src/main/java/com/rnziparchive/RNZipArchiveModule.java index 3937bd4..aa5d050 100644 --- a/android/src/main/java/com/rnziparchive/RNZipArchiveModule.java +++ b/android/src/main/java/com/rnziparchive/RNZipArchiveModule.java @@ -22,8 +22,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; -import java.io.PrintWriter; -import java.io.StringWriter; +import java.io.RandomAccessFile; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -113,6 +112,9 @@ public void cancel(final Promise promise) { @Override public void isPasswordProtected(final String zipFilePath, final Promise promise) { submitWork(() -> { + if (rejectIfZipMissing(zipFilePath, promise)) { + return; + } try (net.lingala.zip4j.ZipFile zipFile = new net.lingala.zip4j.ZipFile(zipFilePath)) { promise.resolve(zipFile.isEncrypted()); } catch (Exception ex) { @@ -134,6 +136,13 @@ public void unzipWithPassword(final String zipFilePath, final String destDirecto return; } submitWork(() -> { + if (rejectIfZipMissing(zipFilePath, promise)) { + return; + } + if (password == null || password.isEmpty()) { + promise.reject(ZipErrorCodes.INVALID_ARGS, "Password is empty"); + return; + } try (net.lingala.zip4j.ZipFile zipFile = new net.lingala.zip4j.ZipFile(zipFilePath)) { if (zipFile.isEncrypted()) { zipFile.setPassword(password.toCharArray()); @@ -142,6 +151,8 @@ public void unzipWithPassword(final String zipFilePath, final String destDirecto return; } + ensureDestination(destDirectory); + List fileHeaderList = zipFile.getFileHeaders(); long totalBytes = Math.max(totalUncompressedSize(fileHeaderList), 1); long extractedBytes = 0; @@ -151,10 +162,8 @@ public void unzipWithPassword(final String zipFilePath, final String destDirecto if (rejectIfCancelled(promise)) { return; } - ZipSecurity.validateExtractPath(destDirectory, fileHeader.getFileName()); - + extractHeader(zipFile, destDirectory, fileHeader); if (!fileHeader.isDirectory()) { - zipFile.extractFile(fileHeader, destDirectory, ZipSecurity.createExtractParameters()); extractedBytes += Math.max(fileHeader.getUncompressedSize(), 0); } updateProgress(extractedBytes, totalBytes, zipFilePath); @@ -180,22 +189,12 @@ public void unzip(final String zipFilePath, final String destDirectory, final St return; } submitWork(() -> { - if (zipFilePath == null) { - promise.reject(ZipErrorCodes.INVALID_PATH, "Couldn't open file null."); - return; - } - File zipFileRef = new File(zipFilePath); - if (!zipFileRef.exists()) { - promise.reject(ZipErrorCodes.FILE_NOT_FOUND, "Couldn't open file " + zipFilePath + "."); + if (rejectIfZipMissing(zipFilePath, promise)) { return; } try (net.lingala.zip4j.ZipFile zipFile = openZipFile(zipFilePath, charset)) { - File destDir = new File(destDirectory); - if (!destDir.exists()) { - //noinspection ResultOfMethodCallIgnored - destDir.mkdirs(); - } + ensureDestination(destDirectory); List fileHeaderList = zipFile.getFileHeaders(); long totalBytes = Math.max(totalUncompressedSize(fileHeaderList), 1); @@ -206,10 +205,8 @@ public void unzip(final String zipFilePath, final String destDirectory, final St if (rejectIfCancelled(promise)) { return; } - ZipSecurity.validateExtractPath(destDirectory, fileHeader.getFileName()); - + extractHeader(zipFile, destDirectory, fileHeader); if (!fileHeader.isDirectory()) { - zipFile.extractFile(fileHeader, destDirectory, ZipSecurity.createExtractParameters()); extractedBytes += Math.max(fileHeader.getUncompressedSize(), 0); } updateProgress(extractedBytes, totalBytes, zipFilePath); @@ -254,13 +251,7 @@ private List optionalEntriesList(ReadableArray entries, Promise promise) @Override public void listContents(final String zipFilePath, final String charset, final Promise promise) { submitWork(() -> { - if (zipFilePath == null) { - promise.reject(ZipErrorCodes.INVALID_PATH, "Couldn't open file null."); - return; - } - File zipFileRef = new File(zipFilePath); - if (!zipFileRef.exists()) { - promise.reject(ZipErrorCodes.FILE_NOT_FOUND, "Couldn't open file " + zipFilePath + "."); + if (rejectIfZipMissing(zipFilePath, promise)) { return; } @@ -286,13 +277,7 @@ private void extractSelectedEntries(final String zipFilePath, final String destD final List wantedEntries, final String charset, final String password, final Promise promise) { submitWork(() -> { - if (zipFilePath == null) { - promise.reject(ZipErrorCodes.INVALID_PATH, "Couldn't open file null."); - return; - } - File zipFileRef = new File(zipFilePath); - if (!zipFileRef.exists()) { - promise.reject(ZipErrorCodes.FILE_NOT_FOUND, "Couldn't open file " + zipFilePath + "."); + if (rejectIfZipMissing(zipFilePath, promise)) { return; } if (wantedEntries == null || wantedEntries.isEmpty()) { @@ -302,6 +287,10 @@ private void extractSelectedEntries(final String zipFilePath, final String destD try (net.lingala.zip4j.ZipFile zipFile = openZipFile(zipFilePath, charset)) { if (password != null) { + if (password.isEmpty()) { + promise.reject(ZipErrorCodes.INVALID_ARGS, "Password is empty"); + return; + } if (!zipFile.isEncrypted()) { promise.reject(ZipErrorCodes.NOT_PASSWORD_PROTECTED, String.format("Zip file: %s is not password protected", zipFilePath)); @@ -310,11 +299,7 @@ private void extractSelectedEntries(final String zipFilePath, final String destD zipFile.setPassword(password.toCharArray()); } - File destDir = new File(destDirectory); - if (!destDir.exists()) { - //noinspection ResultOfMethodCallIgnored - destDir.mkdirs(); - } + ensureDestination(destDirectory); List selected = new ArrayList<>(); for (FileHeader fileHeader : zipFile.getFileHeaders()) { @@ -336,15 +321,9 @@ private void extractSelectedEntries(final String zipFilePath, final String destD if (rejectIfCancelled(promise)) { return; } - ZipSecurity.validateExtractPath(destDirectory, fileHeader.getFileName()); - + extractHeader(zipFile, destDirectory, fileHeader); if (!fileHeader.isDirectory()) { - zipFile.extractFile(fileHeader, destDirectory, ZipSecurity.createExtractParameters()); extractedBytes += Math.max(fileHeader.getUncompressedSize(), 0); - } else { - File dir = new File(destDirectory, fileHeader.getFileName()); - //noinspection ResultOfMethodCallIgnored - dir.mkdirs(); } updateProgress(extractedBytes, totalBytes, zipFilePath); } @@ -401,12 +380,18 @@ private static String stripTrailingSlash(String path) { * from a file. When reading a zip from a stream, we can't * get accurate uncompressed sizes for files (ZipEntry#getCompressedSize() returns -1). *

- * Instead, we compare the number of bytes extracted to the size of the compressed zip file. - * In most cases this means the progress 'stays on' 100% for a little bit (compressedSize < uncompressed size) + * Instead, we compare bytes attributed to each entry with the size of the compressed zip. + * When the compressed size is unknown, the bytes actually copied are used so progress + * cannot move backwards. The value stays under 100% until the extract finishes. */ @Override public void unzipAssets(final String assetsPath, final String destDirectory, final Promise promise) { submitWork(() -> { + if (assetsPath == null || assetsPath.isEmpty()) { + promise.reject(ZipErrorCodes.INVALID_ARGS, "asset path must not be empty"); + return; + } + InputStream assetsInputStream = null; AssetFileDescriptor fileDescriptor = null; long compressedSize; @@ -459,30 +444,41 @@ public void unzipAssets(final String assetsPath, final String destDirectory, fin if (rejectIfCancelled(promise)) { return; } - if (entry.isDirectory()) continue; + String entryName = entry.getName(); + if (entryName == null || entryName.isEmpty()) { + continue; + } - Log.i("rnziparchive", "Extracting: " + entry.getName()); + ZipSecurity.validateExtractPath(destDirectory, entryName); + Log.d(TAG, "Extracting: " + entryName); - ZipSecurity.validateExtractPath(destDirectory, entry.getName()); + if (entry.isDirectory()) { + //noinspection ResultOfMethodCallIgnored + new File(destDirectory, entryName).mkdirs(); + continue; + } - File fout = new File(destDirectory, entry.getName()); + File fout = new File(destDirectory, entryName); File parentDir = fout.getParentFile(); if (parentDir != null && !parentDir.exists()) { //noinspection ResultOfMethodCallIgnored parentDir.mkdirs(); } + long copied; try (FileOutputStream out = new FileOutputStream(fout); BufferedOutputStream bout = new BufferedOutputStream(out)) { - StreamUtil.copy(bin, bout, null); + copied = StreamUtil.copy(bin, bout, null); } - extractedBytes += entry.getCompressedSize(); + extractedBytes += ZipProgress.assetEntryDelta(entry.getCompressedSize(), copied); - // do not let the percentage go over 99% because we want it to hit 100% only when we are sure it's finished - if (extractedBytes > compressedSize * 0.99) extractedBytes = (long) (compressedSize * 0.99); + // Stay under 100% until the stream is finished. Compressed size is only an estimate. + if (extractedBytes > compressedSize * 0.99) { + extractedBytes = (long) (compressedSize * 0.99); + } - updateProgress(extractedBytes, compressedSize, entry.getName()); + updateProgress(extractedBytes, compressedSize, entryName); } updateProgress(compressedSize, compressedSize, assetsPath); // force 100% @@ -561,26 +557,7 @@ private void zipWithPassword(final List filesOrDirectory, final String d } parameters.setEncryptFiles(true); - String[] encParts = encryptionMethod.split("-"); - - if (encParts[0].equals("AES")) { - parameters.setEncryptionMethod(EncryptionMethod.AES); - if (encParts[1].equals("128")) { - parameters.setAesKeyStrength(AesKeyStrength.KEY_STRENGTH_128); - } else if (encParts[1].equals("256")) { - parameters.setAesKeyStrength(AesKeyStrength.KEY_STRENGTH_256); - } else { - parameters.setAesKeyStrength(AesKeyStrength.KEY_STRENGTH_128); - } - } else if ("STANDARD".equals(encryptionMethod)) { - // ZipCrypto (ZIP_STANDARD). ZIP_STANDARD_VARIANT_STRONG is write-only in zip4j - // and fails create/extract with "encryption method is not supported". - parameters.setEncryptionMethod(EncryptionMethod.ZIP_STANDARD); - Log.d(TAG, "Standard Encryption"); - } else { - parameters.setEncryptionMethod(EncryptionMethod.ZIP_STANDARD); - Log.d(TAG, "Encryption type not supported default to Standard Encryption"); - } + applyEncryptionMethod(parameters, encryptionMethod); processZip(filesOrDirectory, destFile, parameters, promise, password.toCharArray()); } catch (Exception ex) { @@ -603,10 +580,11 @@ private void processZip(final List entries, final String destFile, final ? new net.lingala.zip4j.ZipFile(destFile, password) : new net.lingala.zip4j.ZipFile(destFile)) { - updateProgress(0, 100, destFile); - - int totalFiles = 0; + // Count first so a mix of files and folders cannot move progress backwards. + int totalFiles = ZipProgress.countWorkUnits(entries); + long progressTotal = Math.max(totalFiles, 1); int fileCounter = 0; + updateProgress(0, progressTotal, destFile); for (int i = 0; i < entries.size(); i++) { if (rejectIfCancelled(promise)) { @@ -619,7 +597,6 @@ private void processZip(final List entries, final String destFile, final File[] listFiles = f.listFiles(); List files = listFiles != null ? Arrays.asList(listFiles) : new ArrayList(); - totalFiles += files.size(); for (int j = 0; j < files.size(); j++) { if (rejectIfCancelled(promise)) { return; @@ -630,14 +607,13 @@ private void processZip(final List entries, final String destFile, final zipFile.addFile(files.get(j), parameters); } fileCounter += 1; - updateProgress(fileCounter, totalFiles, destFile); + updateProgress(fileCounter, progressTotal, destFile); } } else { - totalFiles += 1; zipFile.addFile(f, parameters); fileCounter += 1; - updateProgress(fileCounter, totalFiles, destFile); + updateProgress(fileCounter, progressTotal, destFile); } } else { promise.reject(ZipErrorCodes.FILE_NOT_FOUND, "File or folder does not exist"); @@ -648,6 +624,7 @@ private void processZip(final List entries, final String destFile, final rejectMapped(promise, ex, ZipErrorCodes.ZIP); return; } + syncFile(destFile); updateProgress(1, 1, destFile); // force 100% promise.resolve(destFile); }); @@ -656,6 +633,9 @@ private void processZip(final List entries, final String destFile, final @Override public void getUncompressedSize(String zipFilePath, String charset, final Promise promise) { submitWork(() -> { + if (rejectIfZipMissing(zipFilePath, promise)) { + return; + } try { long totalSize = getUncompressedSize(zipFilePath, charset); if (totalSize == -1) { @@ -758,9 +738,82 @@ static List readableArrayToStringList(ReadableArray array) { return result; } + /** + * @return true when the promise was already rejected. + */ + private boolean rejectIfZipMissing(String zipFilePath, Promise promise) { + if (zipFilePath == null) { + promise.reject(ZipErrorCodes.INVALID_PATH, "Couldn't open file null."); + return true; + } + if (!new File(zipFilePath).exists()) { + promise.reject(ZipErrorCodes.FILE_NOT_FOUND, "Couldn't open file " + zipFilePath + "."); + return true; + } + return false; + } + + private static void ensureDestination(String destDirectory) { + File destDir = new File(destDirectory); + if (!destDir.exists()) { + //noinspection ResultOfMethodCallIgnored + destDir.mkdirs(); + } + } + + /** + * Validate the entry, then extract it. Directory entries are created (including empty + * ones). Symlinks stay disabled via {@link ZipSecurity#createExtractParameters()}. + */ + private void extractHeader(net.lingala.zip4j.ZipFile zipFile, String destDirectory, FileHeader fileHeader) + throws Exception { + ZipSecurity.validateExtractPath(destDirectory, fileHeader.getFileName()); + if (fileHeader.isDirectory()) { + //noinspection ResultOfMethodCallIgnored + new File(destDirectory, fileHeader.getFileName()).mkdirs(); + return; + } + zipFile.extractFile(fileHeader, destDirectory, ZipSecurity.createExtractParameters()); + } + + private void applyEncryptionMethod(ZipParameters parameters, String encryptionMethod) { + ZipEncryptionChoice.Kind kind = ZipEncryptionChoice.parse(encryptionMethod); + if (kind == ZipEncryptionChoice.Kind.AES_256) { + parameters.setEncryptionMethod(EncryptionMethod.AES); + parameters.setAesKeyStrength(AesKeyStrength.KEY_STRENGTH_256); + return; + } + if (kind == ZipEncryptionChoice.Kind.AES_128) { + parameters.setEncryptionMethod(EncryptionMethod.AES); + parameters.setAesKeyStrength(AesKeyStrength.KEY_STRENGTH_128); + return; + } + // ZipCrypto (ZIP_STANDARD). ZIP_STANDARD_VARIANT_STRONG is write-only in zip4j + // and fails create/extract with "encryption method is not supported". + parameters.setEncryptionMethod(EncryptionMethod.ZIP_STANDARD); + if (encryptionMethod != null && !encryptionMethod.isEmpty() && !"STANDARD".equals(encryptionMethod)) { + Log.d(TAG, "Encryption type not supported default to Standard Encryption"); + } else { + Log.d(TAG, "Standard Encryption"); + } + } + + /** + * Flush zip bytes before resolving. Callers that upload or hash the archive immediately + * otherwise risk reading a partial file (same class of race as iOS fsync, #323). + */ + private static void syncFile(String path) { + if (path == null) { + return; + } + try (RandomAccessFile raf = new RandomAccessFile(path, "r")) { + raf.getFD().sync(); + } catch (IOException ignored) { + } + } + protected void updateProgress(long extractedBytes, long totalSize, String zipFilePath) { - // Ensure progress can't overflow 1 - final double progress = Math.min((double) extractedBytes / (double) totalSize, 1); + final double progress = ZipProgress.fraction(extractedBytes, totalSize); Log.d(TAG, String.format("updateProgress: %.0f%%", progress * 100)); final WritableMap map = Arguments.createMap(); @@ -776,16 +829,6 @@ protected void updateProgress(long extractedBytes, long totalSize, String zipFil }); } - /** - * Returns the exception stack trace as a string - */ - private String getStackTrace(Exception e) { - StringWriter sw = new StringWriter(); - PrintWriter pw = new PrintWriter(sw); - e.printStackTrace(pw); - return sw.toString(); - } - @Override public void addListener(String eventName) { // Keep: Required for RN built in Event Emitter Calls. diff --git a/android/src/main/java/com/rnziparchive/ZipEncryptionChoice.java b/android/src/main/java/com/rnziparchive/ZipEncryptionChoice.java new file mode 100644 index 0000000..4ff75b1 --- /dev/null +++ b/android/src/main/java/com/rnziparchive/ZipEncryptionChoice.java @@ -0,0 +1,31 @@ +package com.rnziparchive; + +/** + * Maps the JS encryption method string onto ZipCrypto or WinZip-AES. + * Empty and {@code STANDARD} stay ZipCrypto. A bare {@code AES} (no key size) is AES-128. + * Anything else falls back to ZipCrypto instead of throwing. + */ +public final class ZipEncryptionChoice { + + public enum Kind { + STANDARD, + AES_128, + AES_256 + } + + private ZipEncryptionChoice() { + } + + public static Kind parse(String encryptionMethod) { + if (encryptionMethod == null || encryptionMethod.isEmpty() || "STANDARD".equals(encryptionMethod)) { + return Kind.STANDARD; + } + if (encryptionMethod.startsWith("AES")) { + if (encryptionMethod.endsWith("256")) { + return Kind.AES_256; + } + return Kind.AES_128; + } + return Kind.STANDARD; + } +} diff --git a/android/src/main/java/com/rnziparchive/ZipErrorCodes.java b/android/src/main/java/com/rnziparchive/ZipErrorCodes.java index 80654ee..556dab0 100644 --- a/android/src/main/java/com/rnziparchive/ZipErrorCodes.java +++ b/android/src/main/java/com/rnziparchive/ZipErrorCodes.java @@ -1,5 +1,8 @@ package com.rnziparchive; +import java.nio.charset.IllegalCharsetNameException; +import java.nio.charset.UnsupportedCharsetException; + import net.lingala.zip4j.exception.ZipException; /** @@ -26,6 +29,9 @@ public static String mapException(Exception ex, String fallback) { if (ex instanceof SecurityException) { return UNSAFE_PATH; } + if (ex instanceof IllegalCharsetNameException || ex instanceof UnsupportedCharsetException) { + return UNSUPPORTED; + } if (ex instanceof ZipException) { ZipException zipException = (ZipException) ex; if (zipException.getType() == ZipException.Type.WRONG_PASSWORD) { @@ -34,6 +40,9 @@ public static String mapException(Exception ex, String fallback) { String message = zipException.getMessage(); if (message != null) { String lower = message.toLowerCase(); + if (lower.contains("does not exist")) { + return FILE_NOT_FOUND; + } if (lower.contains("not a zip") || lower.contains("corrupt") || lower.contains("invalid") || lower.contains("malformed")) { return CORRUPT_ARCHIVE; diff --git a/android/src/main/java/com/rnziparchive/ZipProgress.java b/android/src/main/java/com/rnziparchive/ZipProgress.java new file mode 100644 index 0000000..8a11e73 --- /dev/null +++ b/android/src/main/java/com/rnziparchive/ZipProgress.java @@ -0,0 +1,77 @@ +package com.rnziparchive; + +import java.io.File; +import java.util.List; + +/** + * Progress math shared by zip and unzip so events stay inside 0–1. + * Unknown sizes must not rewind the value (ZipInputStream reports compressed size -1). + */ +public final class ZipProgress { + + private ZipProgress() { + } + + /** + * @return a progress fraction in {@code [0, 1]}. A non-positive total yields 0 + * so callers can force 0% / 100% with an explicit {@code (0, 1)} or {@code (1, 1)}. + */ + public static double fraction(long done, long total) { + if (total <= 0 || done <= 0) { + return 0; + } + double value = (double) done / (double) total; + if (value > 1) { + return 1; + } + return value; + } + + /** + * Bytes to attribute to one {@code unzipAssets} entry. + * Prefer the entry compressed size when the local header has it. + * {@code ZipInputStream} often returns -1 (data descriptor); fall back to bytes copied + * and never return a negative delta. + */ + public static long assetEntryDelta(long compressedSize, long bytesCopied) { + if (compressedSize > 0) { + return compressedSize; + } + if (bytesCopied > 0) { + return bytesCopied; + } + return 0; + } + + /** + * Work units for a {@code zip} / {@code zipWithPassword} call, counted up front so + * progress cannot move backwards while the total grows. + * A file is one unit. A directory is one unit per immediate child (a nested folder + * is added as a single folder entry, matching {@code processZip}). + * Missing paths are omitted; the zip loop rejects those itself. + */ + public static int countWorkUnits(List entries) { + if (entries == null) { + return 0; + } + int total = 0; + for (String path : entries) { + if (path == null) { + continue; + } + File file = new File(path); + if (!file.exists()) { + continue; + } + if (file.isDirectory()) { + File[] children = file.listFiles(); + if (children != null) { + total += children.length; + } + } else { + total += 1; + } + } + return total; + } +} diff --git a/android/src/main/java/com/rnziparchive/ZipSecurity.java b/android/src/main/java/com/rnziparchive/ZipSecurity.java index 16636c0..98d6c0a 100644 --- a/android/src/main/java/com/rnziparchive/ZipSecurity.java +++ b/android/src/main/java/com/rnziparchive/ZipSecurity.java @@ -41,9 +41,12 @@ public static void validateExtractPath(String destDirectory, String entryName) t File fout = new File(destDir, entryName); String canonicalPath = fout.getCanonicalPath(); - String destDirCanonicalPath = destDir.getCanonicalPath() + File.separator; + String destCanonical = destDir.getCanonicalPath(); + String destDirCanonicalPath = destCanonical + File.separator; - if (!canonicalPath.startsWith(destDirCanonicalPath)) { + // The destination itself (entries "", ".", or "foo/..") is inside the root. + // The trailing separator keeps a sibling such as "dest-evil" from matching "dest". + if (!canonicalPath.equals(destCanonical) && !canonicalPath.startsWith(destDirCanonicalPath)) { throw new SecurityException(String.format("Found Zip Path Traversal Vulnerability with %s", canonicalPath)); } } diff --git a/android/src/test/java/com/rnziparchive/ZipEncryptionChoiceTest.java b/android/src/test/java/com/rnziparchive/ZipEncryptionChoiceTest.java new file mode 100644 index 0000000..01c401c --- /dev/null +++ b/android/src/test/java/com/rnziparchive/ZipEncryptionChoiceTest.java @@ -0,0 +1,27 @@ +package com.rnziparchive; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +public class ZipEncryptionChoiceTest { + + @Test + public void emptyAndStandardStayZipCrypto() { + assertEquals(ZipEncryptionChoice.Kind.STANDARD, ZipEncryptionChoice.parse(null)); + assertEquals(ZipEncryptionChoice.Kind.STANDARD, ZipEncryptionChoice.parse("")); + assertEquals(ZipEncryptionChoice.Kind.STANDARD, ZipEncryptionChoice.parse("STANDARD")); + } + + @Test + public void aesKeySizes() { + assertEquals(ZipEncryptionChoice.Kind.AES_128, ZipEncryptionChoice.parse("AES-128")); + assertEquals(ZipEncryptionChoice.Kind.AES_256, ZipEncryptionChoice.parse("AES-256")); + assertEquals(ZipEncryptionChoice.Kind.AES_128, ZipEncryptionChoice.parse("AES")); + } + + @Test + public void unknownMethodFallsBackToStandard() { + assertEquals(ZipEncryptionChoice.Kind.STANDARD, ZipEncryptionChoice.parse("PKWARE")); + } +} diff --git a/android/src/test/java/com/rnziparchive/ZipErrorCodesTest.java b/android/src/test/java/com/rnziparchive/ZipErrorCodesTest.java index ca201c7..e1c79fb 100644 --- a/android/src/test/java/com/rnziparchive/ZipErrorCodesTest.java +++ b/android/src/test/java/com/rnziparchive/ZipErrorCodesTest.java @@ -2,6 +2,8 @@ import static org.junit.Assert.assertEquals; +import java.nio.charset.UnsupportedCharsetException; + import net.lingala.zip4j.exception.ZipException; import org.junit.Test; @@ -27,4 +29,18 @@ public void mapsUnknownExceptionToFallback() { ZipErrorCodes.ZIP, ZipErrorCodes.mapException(new RuntimeException("boom"), ZipErrorCodes.ZIP)); } + + @Test + public void mapsMissingZipFileToFileNotFound() { + assertEquals( + ZipErrorCodes.FILE_NOT_FOUND, + ZipErrorCodes.mapException(new ZipException("zip file does not exist"), ZipErrorCodes.UNZIP)); + } + + @Test + public void mapsUnsupportedCharsetToUnsupported() { + assertEquals( + ZipErrorCodes.UNSUPPORTED, + ZipErrorCodes.mapException(new UnsupportedCharsetException("not-a-charset"), ZipErrorCodes.UNZIP)); + } } diff --git a/android/src/test/java/com/rnziparchive/ZipProgressTest.java b/android/src/test/java/com/rnziparchive/ZipProgressTest.java new file mode 100644 index 0000000..21f7c66 --- /dev/null +++ b/android/src/test/java/com/rnziparchive/ZipProgressTest.java @@ -0,0 +1,69 @@ +package com.rnziparchive; + +import static org.junit.Assert.assertEquals; + +import java.io.File; +import java.nio.file.Files; +import java.util.Arrays; +import java.util.Collections; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class ZipProgressTest { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void fraction_staysInsideZeroToOne() { + assertEquals(0, ZipProgress.fraction(-5, 10), 0); + assertEquals(0, ZipProgress.fraction(0, 10), 0); + assertEquals(0, ZipProgress.fraction(1, 0), 0); + assertEquals(0.5, ZipProgress.fraction(1, 2), 0); + assertEquals(1, ZipProgress.fraction(5, 2), 0); + } + + @Test + public void assetEntryDelta_neverNegative() { + assertEquals(0, ZipProgress.assetEntryDelta(-1, -1)); + assertEquals(12, ZipProgress.assetEntryDelta(-1, 12)); + assertEquals(8, ZipProgress.assetEntryDelta(8, 100)); + assertEquals(0, ZipProgress.assetEntryDelta(0, 0)); + } + + @Test + public void countWorkUnits_isStableForMixedFilesAndFolders() throws Exception { + File root = temporaryFolder.getRoot(); + File file = new File(root, "a.txt"); + Files.write(file.toPath(), new byte[] {'h', 'i'}); + File folder = new File(root, "dir"); + assertEquals(true, folder.mkdir()); + assertEquals(true, new File(folder, "b.txt").createNewFile()); + assertEquals(true, new File(folder, "c.txt").createNewFile()); + File nested = new File(folder, "nested"); + assertEquals(true, nested.mkdir()); + assertEquals(true, new File(nested, "d.txt").createNewFile()); + + // One file, plus three immediate children of dir (b, c, nested). Nested contents + // are one folder unit, matching processZip's addFolder call. + int total = ZipProgress.countWorkUnits(Arrays.asList( + file.getAbsolutePath(), + folder.getAbsolutePath())); + assertEquals(4, total); + + // The same total is used for every tick, so progress cannot move backwards + // the way a running total did after the first file already reported 100%. + double afterFirstFile = ZipProgress.fraction(1, total); + double afterNext = ZipProgress.fraction(2, total); + assertEquals(0.25, afterFirstFile, 0); + assertEquals(0.5, afterNext, 0); + } + + @Test + public void countWorkUnits_skipsMissingPaths() { + assertEquals(0, ZipProgress.countWorkUnits(Collections.singletonList("/no/such/file"))); + assertEquals(0, ZipProgress.countWorkUnits(null)); + } +} diff --git a/android/src/test/java/com/rnziparchive/ZipSecurityTest.java b/android/src/test/java/com/rnziparchive/ZipSecurityTest.java index c6922e2..a197c93 100644 --- a/android/src/test/java/com/rnziparchive/ZipSecurityTest.java +++ b/android/src/test/java/com/rnziparchive/ZipSecurityTest.java @@ -54,6 +54,27 @@ public void validateExtractPath_rejectsDeepTraversal() throws IOException { } } + @Test + public void validateExtractPath_acceptsDestinationItself() throws IOException { + File dest = temporaryFolder.newFolder("dest"); + + ZipSecurity.validateExtractPath(dest.getAbsolutePath(), "."); + ZipSecurity.validateExtractPath(dest.getAbsolutePath(), ""); + ZipSecurity.validateExtractPath(dest.getAbsolutePath(), "nested/.."); + } + + @Test + public void validateExtractPath_rejectsSiblingPrefix() throws IOException { + File dest = temporaryFolder.newFolder("dest"); + + try { + ZipSecurity.validateExtractPath(dest.getAbsolutePath(), "../dest-evil/file.txt"); + fail("Expected SecurityException for sibling prefix"); + } catch (SecurityException ex) { + assertTrue(ex.getMessage().contains("Zip Path Traversal Vulnerability")); + } + } + @Test public void validateExtractPath_acceptsEntryInSubdirectoryNamedLikeTraversal() throws IOException { File dest = temporaryFolder.newFolder("dest"); diff --git a/index.js b/index.js index 923d186..5d3777f 100644 --- a/index.js +++ b/index.js @@ -84,20 +84,38 @@ function withAbort(signal, work) { } let settled = false; + let started = false; let rejectAbort; const abortGate = new Promise((_, reject) => { rejectAbort = reject; }); const onAbort = () => { - cancel().catch(() => {}); + // cancel() flags the native worker. Skip it when this call never started, + // so an already-aborted signal cannot abort a different in-flight operation. + if (started) { + cancel().catch(() => {}); + } if (!settled) { rejectAbort(zipError(ErrorCodes.CANCELLED, "Operation cancelled")); } }; signal.addEventListener("abort", onAbort, { once: true }); + // Abort can land after the signal.aborted check and before the listener is attached. + if (signal.aborted) { + onAbort(); + return abortGate.finally(() => { + signal.removeEventListener("abort", onAbort); + }); + } const run = Promise.resolve() - .then(work) + .then(() => { + if (signal.aborted) { + throw zipError(ErrorCodes.CANCELLED, "Operation cancelled"); + } + started = true; + return work(); + }) .then( (value) => { settled = true; diff --git a/ios/RNZipArchive.mm b/ios/RNZipArchive.mm index 19a562c..c69f8a8 100644 --- a/ios/RNZipArchive.mm +++ b/ios/RNZipArchive.mm @@ -884,6 +884,10 @@ - (void)getUncompressedSize:(NSString *)path if ([self rejectIfUnsupportedCharset:charset reject:reject]) { return; } + if (path.length == 0 || ![[NSFileManager defaultManager] fileExistsAtPath:path]) { + reject(kZipErrFileNotFound, @"failed to open zip file", nil); + return; + } [self beginOperation]; [self runAsync:^{ NSError *error = nil; From 4d344c1c9ed9c5386cc84e4af691a8c02a611c4c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 2 Oct 2026 04:41:16 +0000 Subject: [PATCH 2/2] test: lock zip progress, extract, and abort with end-to-end cases 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 --- __tests__/zip-archive.e2e.test.js | 49 ++++ .../com/rnziparchive/RNZipArchiveModule.java | 71 +++--- .../java/com/rnziparchive/ZipExtractor.java | 44 ++++ .../java/com/rnziparchive/ZipProgress.java | 33 +++ .../rnziparchive/ZipArchiveEndToEndTest.java | 216 ++++++++++++++++++ 5 files changed, 375 insertions(+), 38 deletions(-) create mode 100644 __tests__/zip-archive.e2e.test.js create mode 100644 android/src/main/java/com/rnziparchive/ZipExtractor.java create mode 100644 android/src/test/java/com/rnziparchive/ZipArchiveEndToEndTest.java diff --git a/__tests__/zip-archive.e2e.test.js b/__tests__/zip-archive.e2e.test.js new file mode 100644 index 0000000..eb10797 --- /dev/null +++ b/__tests__/zip-archive.e2e.test.js @@ -0,0 +1,49 @@ +const { zip, ErrorCodes } = require('../index'); +const { mockRNZipArchive } = require('react-native'); + +/** + * Public JS API, end to end through zip() → withAbort → the native module. + * Native zip is mocked; the assertions are about whether this process starts + * or cancels work. + */ +describe('zip abort end to end', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('signal that aborts while the listener is attached does not start native work', async () => { + const signal = { + aborted: false, + addEventListener() { + this.aborted = true; + }, + removeEventListener() {}, + }; + + await expect(zip('/source', '/target.zip', { signal })).rejects.toMatchObject({ + name: 'ZipError', + code: ErrorCodes.CANCELLED, + }); + expect(mockRNZipArchive.zipFolder).not.toHaveBeenCalled(); + expect(mockRNZipArchive.cancel).not.toHaveBeenCalled(); + }); + + test('abort after native work has started cancels that operation', async () => { + let resolveNative; + mockRNZipArchive.zipFolder.mockReturnValueOnce( + new Promise((resolve) => { + resolveNative = resolve; + }) + ); + const controller = new AbortController(); + const pending = zip('/source', '/target.zip', { signal: controller.signal }); + await Promise.resolve(); + + controller.abort(); + + await expect(pending).rejects.toMatchObject({ code: ErrorCodes.CANCELLED }); + expect(mockRNZipArchive.zipFolder).toHaveBeenCalledTimes(1); + expect(mockRNZipArchive.cancel).toHaveBeenCalledTimes(1); + resolveNative('/mock/path.zip'); + }); +}); diff --git a/android/src/main/java/com/rnziparchive/RNZipArchiveModule.java b/android/src/main/java/com/rnziparchive/RNZipArchiveModule.java index aa5d050..0fa1256 100644 --- a/android/src/main/java/com/rnziparchive/RNZipArchiveModule.java +++ b/android/src/main/java/com/rnziparchive/RNZipArchiveModule.java @@ -162,7 +162,7 @@ public void unzipWithPassword(final String zipFilePath, final String destDirecto if (rejectIfCancelled(promise)) { return; } - extractHeader(zipFile, destDirectory, fileHeader); + ZipExtractor.extractEntry(zipFile, destDirectory, fileHeader); if (!fileHeader.isDirectory()) { extractedBytes += Math.max(fileHeader.getUncompressedSize(), 0); } @@ -205,7 +205,7 @@ public void unzip(final String zipFilePath, final String destDirectory, final St if (rejectIfCancelled(promise)) { return; } - extractHeader(zipFile, destDirectory, fileHeader); + ZipExtractor.extractEntry(zipFile, destDirectory, fileHeader); if (!fileHeader.isDirectory()) { extractedBytes += Math.max(fileHeader.getUncompressedSize(), 0); } @@ -321,7 +321,7 @@ private void extractSelectedEntries(final String zipFilePath, final String destD if (rejectIfCancelled(promise)) { return; } - extractHeader(zipFile, destDirectory, fileHeader); + ZipExtractor.extractEntry(zipFile, destDirectory, fileHeader); if (!fileHeader.isDirectory()) { extractedBytes += Math.max(fileHeader.getUncompressedSize(), 0); } @@ -471,12 +471,8 @@ public void unzipAssets(final String assetsPath, final String destDirectory, fin copied = StreamUtil.copy(bin, bout, null); } - extractedBytes += ZipProgress.assetEntryDelta(entry.getCompressedSize(), copied); - - // Stay under 100% until the stream is finished. Compressed size is only an estimate. - if (extractedBytes > compressedSize * 0.99) { - extractedBytes = (long) (compressedSize * 0.99); - } + extractedBytes = ZipProgress.advanceAssetBytes( + extractedBytes, compressedSize, entry.getCompressedSize(), copied); updateProgress(extractedBytes, compressedSize, entryName); } @@ -580,11 +576,10 @@ private void processZip(final List entries, final String destFile, final ? new net.lingala.zip4j.ZipFile(destFile, password) : new net.lingala.zip4j.ZipFile(destFile)) { - // Count first so a mix of files and folders cannot move progress backwards. - int totalFiles = ZipProgress.countWorkUnits(entries); - long progressTotal = Math.max(totalFiles, 1); - int fileCounter = 0; - updateProgress(0, progressTotal, destFile); + // Same event list the end-to-end test asserts: fixed total, so progress cannot rewind. + java.util.List planned = ZipProgress.events(entries); + int cursor = 0; + emitProgress(planned.get(cursor++), destFile); for (int i = 0; i < entries.size(); i++) { if (rejectIfCancelled(promise)) { @@ -606,14 +601,12 @@ private void processZip(final List entries, final String destFile, final } else { zipFile.addFile(files.get(j), parameters); } - fileCounter += 1; - updateProgress(fileCounter, progressTotal, destFile); + emitProgress(plannedEvent(planned, cursor++), destFile); } } else { zipFile.addFile(f, parameters); - fileCounter += 1; - updateProgress(fileCounter, progressTotal, destFile); + emitProgress(plannedEvent(planned, cursor++), destFile); } } else { promise.reject(ZipErrorCodes.FILE_NOT_FOUND, "File or folder does not exist"); @@ -625,7 +618,7 @@ private void processZip(final List entries, final String destFile, final return; } syncFile(destFile); - updateProgress(1, 1, destFile); // force 100% + emitProgress(1, destFile); // force 100% promise.resolve(destFile); }); } @@ -761,21 +754,6 @@ private static void ensureDestination(String destDirectory) { } } - /** - * Validate the entry, then extract it. Directory entries are created (including empty - * ones). Symlinks stay disabled via {@link ZipSecurity#createExtractParameters()}. - */ - private void extractHeader(net.lingala.zip4j.ZipFile zipFile, String destDirectory, FileHeader fileHeader) - throws Exception { - ZipSecurity.validateExtractPath(destDirectory, fileHeader.getFileName()); - if (fileHeader.isDirectory()) { - //noinspection ResultOfMethodCallIgnored - new File(destDirectory, fileHeader.getFileName()).mkdirs(); - return; - } - zipFile.extractFile(fileHeader, destDirectory, ZipSecurity.createExtractParameters()); - } - private void applyEncryptionMethod(ZipParameters parameters, String encryptionMethod) { ZipEncryptionChoice.Kind kind = ZipEncryptionChoice.parse(encryptionMethod); if (kind == ZipEncryptionChoice.Kind.AES_256) { @@ -812,13 +790,26 @@ private static void syncFile(String path) { } } - protected void updateProgress(long extractedBytes, long totalSize, String zipFilePath) { - final double progress = ZipProgress.fraction(extractedBytes, totalSize); - Log.d(TAG, String.format("updateProgress: %.0f%%", progress * 100)); + private static double plannedEvent(java.util.List planned, int index) { + if (planned.isEmpty()) { + return 0; + } + if (index < 0) { + return planned.get(0); + } + if (index >= planned.size()) { + return planned.get(planned.size() - 1); + } + return planned.get(index); + } + + private void emitProgress(double progress, String zipFilePath) { + final double clamped = progress < 0 ? 0 : Math.min(progress, 1); + Log.d(TAG, String.format("updateProgress: %.0f%%", clamped * 100)); final WritableMap map = Arguments.createMap(); map.putString(EVENT_KEY_FILENAME, zipFilePath); - map.putDouble(EVENT_KEY_PROGRESS, progress); + map.putDouble(EVENT_KEY_PROGRESS, clamped); mainHandler.post(() -> { ReactApplicationContext context = getReactApplicationContext(); @@ -829,6 +820,10 @@ protected void updateProgress(long extractedBytes, long totalSize, String zipFil }); } + protected void updateProgress(long extractedBytes, long totalSize, String zipFilePath) { + emitProgress(ZipProgress.fraction(extractedBytes, totalSize), zipFilePath); + } + @Override public void addListener(String eventName) { // Keep: Required for RN built in Event Emitter Calls. diff --git a/android/src/main/java/com/rnziparchive/ZipExtractor.java b/android/src/main/java/com/rnziparchive/ZipExtractor.java new file mode 100644 index 0000000..748ee40 --- /dev/null +++ b/android/src/main/java/com/rnziparchive/ZipExtractor.java @@ -0,0 +1,44 @@ +package com.rnziparchive; + +import java.io.File; + +import net.lingala.zip4j.ZipFile; +import net.lingala.zip4j.model.FileHeader; + +/** + * Shared extract path for full unzip, selective unzip, and tests. + * Directory entries are created. Symlinks stay off. Paths that leave the destination throw. + */ +public final class ZipExtractor { + + private ZipExtractor() { + } + + public static void extractAll(ZipFile zipFile, String destDirectory) throws Exception { + File destDir = new File(destDirectory); + if (!destDir.exists()) { + //noinspection ResultOfMethodCallIgnored + destDir.mkdirs(); + } + for (FileHeader fileHeader : zipFile.getFileHeaders()) { + extractEntry(zipFile, destDirectory, fileHeader); + } + } + + public static void extractEntry(ZipFile zipFile, String destDirectory, FileHeader fileHeader) throws Exception { + String entryName = fileHeader.getFileName(); + ZipSecurity.validateExtractPath(destDirectory, entryName); + File target = new File(destDirectory, entryName); + String targetCanonical = target.getCanonicalPath(); + String destCanonical = new File(destDirectory).getCanonicalPath(); + // "." and "foo/.." resolve to the destination itself. Do not open that path as a file. + if (fileHeader.isDirectory() || targetCanonical.equals(destCanonical)) { + if (!targetCanonical.equals(destCanonical)) { + //noinspection ResultOfMethodCallIgnored + target.mkdirs(); + } + return; + } + zipFile.extractFile(fileHeader, destDirectory, ZipSecurity.createExtractParameters()); + } +} diff --git a/android/src/main/java/com/rnziparchive/ZipProgress.java b/android/src/main/java/com/rnziparchive/ZipProgress.java index 8a11e73..60c6ceb 100644 --- a/android/src/main/java/com/rnziparchive/ZipProgress.java +++ b/android/src/main/java/com/rnziparchive/ZipProgress.java @@ -1,6 +1,7 @@ package com.rnziparchive; import java.io.File; +import java.util.ArrayList; import java.util.List; /** @@ -43,6 +44,38 @@ public static long assetEntryDelta(long compressedSize, long bytesCopied) { return 0; } + /** + * Next {@code unzipAssets} byte count. Unknown compressed sizes (-1) contribute the + * bytes copied, and the running total stays under 99% of the archive until the caller + * emits the explicit 100% event. + */ + public static long advanceAssetBytes(long extractedBytes, long archiveSize, long compressedSize, long bytesCopied) { + long next = extractedBytes + assetEntryDelta(compressedSize, bytesCopied); + if (next < 0) { + next = 0; + } + if (archiveSize > 0 && next > archiveSize * 0.99) { + return (long) (archiveSize * 0.99); + } + return next; + } + + /** + * Progress events {@code processZip} emits for these paths: explicit 0, one tick per + * work unit, then explicit 1. The total is fixed up front. + */ + public static List events(List entries) { + int totalFiles = countWorkUnits(entries); + long progressTotal = Math.max(totalFiles, 1); + List planned = new ArrayList<>(); + planned.add(fraction(0, progressTotal)); + for (int i = 1; i <= totalFiles; i++) { + planned.add(fraction(i, progressTotal)); + } + planned.add(fraction(1, 1)); + return planned; + } + /** * Work units for a {@code zip} / {@code zipWithPassword} call, counted up front so * progress cannot move backwards while the total grows. diff --git a/android/src/test/java/com/rnziparchive/ZipArchiveEndToEndTest.java b/android/src/test/java/com/rnziparchive/ZipArchiveEndToEndTest.java new file mode 100644 index 0000000..0eb848c --- /dev/null +++ b/android/src/test/java/com/rnziparchive/ZipArchiveEndToEndTest.java @@ -0,0 +1,216 @@ +package com.rnziparchive; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import net.lingala.zip4j.ZipFile; +import net.lingala.zip4j.model.ZipParameters; +import net.lingala.zip4j.model.enums.AesKeyStrength; +import net.lingala.zip4j.model.enums.CompressionMethod; +import net.lingala.zip4j.model.enums.EncryptionMethod; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** + * End-to-end coverage of the zip/unzip behavior the native module calls: + * real files, real zip bytes, then {@link ZipProgress}, {@link ZipExtractor}, + * {@link ZipErrorCodes}, and {@link ZipEncryptionChoice}. + */ +public class ZipArchiveEndToEndTest { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void zipProgress_fileThenFolder_staysMonotonicAndDoesNotFinishEarly() throws Exception { + File file = temporaryFolder.newFile("a.txt"); + Files.write(file.toPath(), "alpha".getBytes(StandardCharsets.UTF_8)); + File folder = temporaryFolder.newFolder("dir"); + Files.write(new File(folder, "b.txt").toPath(), "bravo".getBytes(StandardCharsets.UTF_8)); + Files.write(new File(folder, "c.txt").toPath(), "charlie".getBytes(StandardCharsets.UTF_8)); + + List events = ZipProgress.events(Arrays.asList( + file.getAbsolutePath(), + folder.getAbsolutePath())); + + assertEquals(0, events.get(0), 0); + assertEquals(1, events.get(events.size() - 1), 0); + assertTrue("first unit reported 100% while later units were still pending: " + events, + events.get(1) < 1); + assertMonotonic(events); + } + + @Test + public void unzipAssetsProgress_unknownCompressedSize_doesNotRewind() { + long archiveSize = 1000; + long extracted = 0; + List events = new ArrayList<>(); + events.add(ZipProgress.fraction(extracted, archiveSize)); + + // Second entry matches ZipInputStream, which reports compressed size -1. + long[] compressedSizes = {200, -1}; + long[] bytesCopied = {800, 50}; + for (int i = 0; i < compressedSizes.length; i++) { + extracted = ZipProgress.advanceAssetBytes( + extracted, archiveSize, compressedSizes[i], bytesCopied[i]); + events.add(ZipProgress.fraction(extracted, archiveSize)); + } + events.add(1.0); + + assertMonotonic(events); + assertTrue(events.get(events.size() - 1) <= 1); + } + + @Test + public void roundTrip_keepsEmptyDirectoryAndFileBytes() throws Exception { + File note = temporaryFolder.newFile("note.txt"); + Files.write(note.toPath(), "hello-e2e".getBytes(StandardCharsets.UTF_8)); + File empty = temporaryFolder.newFolder("empty"); + + File zipPath = temporaryFolder.newFile("out.zip"); + // newFile creates a zero-byte file; zip4j wants to create it. + assertTrue(zipPath.delete()); + ZipParameters params = new ZipParameters(); + params.setCompressionMethod(CompressionMethod.DEFLATE); + try (ZipFile zip = new ZipFile(zipPath)) { + zip.addFile(note, params); + zip.addFolder(empty, params); + } + + File dest = temporaryFolder.newFolder("dest"); + try (ZipFile zip = new ZipFile(zipPath)) { + ZipExtractor.extractAll(zip, dest.getAbsolutePath()); + } + + File extractedNote = new File(dest, "note.txt"); + File extractedEmpty = new File(dest, "empty"); + assertEquals("hello-e2e", new String(Files.readAllBytes(extractedNote.toPath()), StandardCharsets.UTF_8)); + assertTrue("empty directory was dropped on extract; dest has [" + names(dest) + "]", + extractedEmpty.isDirectory()); + } + + @Test + public void roundTrip_rejectsPathTraversalAndWritesNothingOutsideDest() throws Exception { + File payload = temporaryFolder.newFile("payload.txt"); + Files.write(payload.toPath(), "nope".getBytes(StandardCharsets.UTF_8)); + File zipPath = temporaryFolder.newFile("slip.zip"); + assertTrue(zipPath.delete()); + + ZipParameters params = new ZipParameters(); + params.setFileNameInZip("../evil.txt"); + try (ZipFile zip = new ZipFile(zipPath)) { + zip.addFile(payload, params); + } + + File dest = temporaryFolder.newFolder("dest"); + try (ZipFile zip = new ZipFile(zipPath)) { + ZipExtractor.extractAll(zip, dest.getAbsolutePath()); + fail("traversal entry was extracted"); + } catch (SecurityException ex) { + assertTrue(ex.getMessage().contains("Zip Path Traversal")); + } + assertFalse(new File(temporaryFolder.getRoot(), "evil.txt").exists()); + assertFalse(new File(dest, "evil.txt").exists()); + } + + @Test + public void roundTrip_acceptsEntryNamedDot() throws Exception { + File payload = temporaryFolder.newFile("payload.txt"); + Files.write(payload.toPath(), "dot".getBytes(StandardCharsets.UTF_8)); + File zipPath = temporaryFolder.newFile("dot.zip"); + assertTrue(zipPath.delete()); + + ZipParameters params = new ZipParameters(); + params.setFileNameInZip("."); + try (ZipFile zip = new ZipFile(zipPath)) { + zip.addFile(payload, params); + } + + File dest = temporaryFolder.newFolder("dest"); + try (ZipFile zip = new ZipFile(zipPath)) { + ZipExtractor.extractAll(zip, dest.getAbsolutePath()); + } + assertTrue(dest.isDirectory()); + } + + @Test + public void missingArchive_mapsToFileNotFound() throws Exception { + File missing = new File(temporaryFolder.getRoot(), "missing.zip"); + try (ZipFile zip = new ZipFile(missing)) { + zip.getComment(); + fail("missing archive should throw"); + } catch (Exception ex) { + assertEquals( + "missing archive was " + ex.getClass().getSimpleName() + ": " + ex.getMessage(), + ZipErrorCodes.FILE_NOT_FOUND, + ZipErrorCodes.mapException(ex, ZipErrorCodes.UNZIP)); + } + } + + @Test + public void bareAes_roundTripsFileBytes() throws Exception { + assertEquals(ZipEncryptionChoice.Kind.AES_128, ZipEncryptionChoice.parse("AES")); + + File src = temporaryFolder.newFile("secret.txt"); + Files.write(src.toPath(), "topsecret".getBytes(StandardCharsets.UTF_8)); + File zipPath = temporaryFolder.newFile("aes.zip"); + assertTrue(zipPath.delete()); + + ZipParameters parameters = new ZipParameters(); + parameters.setCompressionMethod(CompressionMethod.DEFLATE); + parameters.setEncryptFiles(true); + parameters.setEncryptionMethod(EncryptionMethod.AES); + parameters.setAesKeyStrength( + ZipEncryptionChoice.parse("AES") == ZipEncryptionChoice.Kind.AES_256 + ? AesKeyStrength.KEY_STRENGTH_256 + : AesKeyStrength.KEY_STRENGTH_128); + + try (ZipFile zip = new ZipFile(zipPath, "pw".toCharArray())) { + zip.addFile(src, parameters); + } + + File dest = temporaryFolder.newFolder("aes-out"); + try (ZipFile zip = new ZipFile(zipPath, "pw".toCharArray())) { + zip.extractAll(dest.getAbsolutePath()); + } + assertEquals("topsecret", + new String(Files.readAllBytes(new File(dest, "secret.txt").toPath()), StandardCharsets.UTF_8)); + } + + private static void assertMonotonic(List events) { + assertTrue("expected a start and an end event, got " + events, events.size() >= 2); + for (int i = 1; i < events.size(); i++) { + assertTrue("progress moved backwards at index " + i + ": " + events, + events.get(i) + 1e-9 >= events.get(i - 1)); + assertTrue("progress left 0..1 at index " + i + ": " + events, + events.get(i) >= 0 && events.get(i) <= 1); + } + } + + private static String names(File dir) { + File[] files = dir.listFiles(); + if (files == null) { + return "(unreadable)"; + } + StringBuilder builder = new StringBuilder(); + for (File file : files) { + builder.append(file.getName()); + if (file.isDirectory()) { + builder.append('/'); + } + builder.append(' '); + } + return builder.toString(); + } +}