Binary patching: replace copy + delete with a filtered copy

android/app/src/main/java/com/microsoft/codepush/react/CodePushUpdateManager.java

@@ -273,22 +273,26 @@ void installDownloadedUpdate(JSONObject updatePackage, String expectedBundleFile
                 diffManifestFile.delete();
             }
 
-            FileUtils.copyDirectoryContents(unzippedFolderPath, newUpdateFolderPath);
+            // The patches folder of a binary diff must not end up in the installed package: it is
+            // not part of the released contents, so it changes the folder hash and surfaces later
+            // as a misleading integrity-check failure. The patcher reads it from unzippedFolderPath.
+            boolean isBinaryDiffUpdate = isDiffUpdate && diffManifest.isBinaryDiff();
+            FileUtils.copyDirectoryContents(unzippedFolderPath, newUpdateFolderPath,
+                    isBinaryDiffUpdate ? CodePushConstants.DIFF_PATCHES_FOLDER_NAME : null);
 
             if (isDiffUpdate) {
                 // Run patching after copyNecessaryFilesFromCurrentPackage() so patched output overwrites
                 // bytes copied in from the old package at the same paths.
                 if (diffManifest.getVersion() > 2 || diffManifest.getVersion() < 1) {
                     throw new IOException("Diff manifest version " + diffManifest.getVersion() + " is not supported by this SDK version.");
-                } else if (diffManifest.getVersion() == 2 && !mEnableDeltaUpdates) {
+                } else if (diffManifest.isBinaryDiff() && !mEnableDeltaUpdates) {
                     throw new IOException("Received a binary diff update, but delta updates are not enabled on this client. Set CodePushEnableDeltaUpdates to true in strings.xml to enable them.");
-                } else if (diffManifest.getVersion() == 2) {
+                } else if (diffManifest.isBinaryDiff()) {
                     String currentPackageFolderPath = getCurrentPackageFolderPath();
                     if (currentPackageFolderPath == null) {
                         throw new CodePushInvalidUpdateException("Received a binary diff update, but no currently installed package exists to diff against (this is likely the first CodePush update for this app install). Diffing against the embedded app binary is not yet supported.");
                     }
                     BinaryDiffPatcher.applyBinaryDiffPatches(diffManifest, new File(currentPackageFolderPath), new File(unzippedFolderPath), new File(newUpdateFolderPath));
-                    FileUtils.deleteDirectoryAtPath(new File(newUpdateFolderPath, CodePushConstants.DIFF_PATCHES_FOLDER_NAME).getPath());
                 }
             }
 

android/app/src/main/java/com/microsoft/codepush/react/FileUtils.java

@@ -16,13 +16,21 @@ public class FileUtils {
     private static final int WRITE_BUFFER_SIZE = 1024 * 8;
 
     public static void copyDirectoryContents(String sourceDirectoryPath, String destinationDirectoryPath) throws IOException {
+        copyDirectoryContents(sourceDirectoryPath, destinationDirectoryPath, null);
+    }
+
+    // excludedEntryName applies only to the top level of sourceDirectoryPath: nested entries with the same name are copied.
+    public static void copyDirectoryContents(String sourceDirectoryPath, String destinationDirectoryPath, String excludedEntryName) throws IOException {
         File sourceDir = new File(sourceDirectoryPath);
         File destDir = new File(destinationDirectoryPath);
         if (!destDir.exists()) {
             destDir.mkdir();
         }
 
         for (File sourceFile : sourceDir.listFiles()) {
+            if (sourceFile.getName().equals(excludedEntryName)) {
+                continue;
+            }
             if (sourceFile.isDirectory()) {
                 copyDirectoryContents(
                         CodePushUtils.appendPathComponent(sourceDirectoryPath, sourceFile.getName()),

android/app/src/main/java/com/microsoft/codepush/react/diffpatch/DiffManifest.kt

@@ -24,7 +24,14 @@ data class DiffManifest(
     val deletedFiles: List<String>,
     // Map key: file's relative path in the package being installed.
     val patchedFiles: Map<String, PatchedFileEntry>,
-)
+) {
+    // True if this manifest describes a binary diff update, which ships its patches under CodePushConstants.DIFF_PATCHES_FOLDER_NAME.
+    // This is a subset of "diff updates" in general, as a diff update payload can also consist of:
+    // - The modified files included in the ZIP, which are applied on top of the existing files (without any binary patching)
+    // - The list of files to delete from the old package
+    val isBinaryDiff: Boolean
+        get() = version == 2
+}
 
 @Throws(JSONException::class)
 fun parseDiffManifest(json: JSONObject): DiffManifest {
@@ -41,6 +48,12 @@ fun parseDiffManifest(json: JSONObject): DiffManifest {
     val patchedFilesJson = json.optJSONObject("patchedFiles")
     val patchedFiles = if (patchedFilesJson != null) {
         patchedFilesJson.keys().asSequence().associateWith { relativePath ->
+            if (relativePath.split('/').contains("..")) {
+                throw JSONException("Diff manifest patchedFiles[\"$relativePath\"] must not contain \"..\" components.")
+            }
+            if (topLevelComponentOf(relativePath) == CodePushConstants.DIFF_PATCHES_FOLDER_NAME) {
+                throw JSONException("Diff manifest patchedFiles[\"$relativePath\"] targets the reserved \"$reservedPatchesFolderPrefix\" folder, which is not part of the installed package.")
+            }
             val entry = patchedFilesJson.getJSONObject(relativePath)
             val patch = entry.getString("patch")
             if (!patch.startsWith(reservedPatchesFolderPrefix)) {
@@ -57,9 +70,15 @@ fun parseDiffManifest(json: JSONObject): DiffManifest {
         emptyMap()
     }
 
-    if (version != 2 && patchedFiles.isNotEmpty()) {
+    val manifest = DiffManifest(version = version, deletedFiles = deletedFiles, patchedFiles = patchedFiles)
+    if (!manifest.isBinaryDiff && patchedFiles.isNotEmpty()) {
         throw JSONException("Diff manifest declares version $version but contains patchedFiles, which requires version 2.")
     }
 
-    return DiffManifest(version = version, deletedFiles = deletedFiles, patchedFiles = patchedFiles)
+    return manifest
 }
+
+// The top-level entry that `relativePath` names under a base folder. Empty and "." components are skipped.
+// The caller rejects ".." components first, so they need no handling here.
+private fun topLevelComponentOf(relativePath: String): String? =
+    relativePath.split('/').firstOrNull { it.isNotEmpty() && it != "." }

android/app/src/test/java/com/microsoft/codepush/react/CodePushUpdateManagerTest.kt

@@ -14,6 +14,7 @@ import org.junit.rules.TemporaryFolder
 import org.mockito.MockedStatic
 import org.mockito.Mockito
 import java.io.File
+import java.security.MessageDigest
 import java.util.zip.ZipEntry
 import java.util.zip.ZipOutputStream
 
@@ -61,6 +62,15 @@ class CodePushUpdateManagerTest {
         return file
     }
 
+    // The package hash the CLI computes for a release with the given file contents: the SHA-256 of the
+    // JSON array of sorted "<relativePath>:<sha256 of contents>" entries.
+    private fun releaseHashOf(files: Map<String, String>): String {
+        fun sha256Hex(bytes: ByteArray) =
+            MessageDigest.getInstance("SHA-256").digest(bytes).joinToString("") { "%02x".format(it) }
+        val entries = files.map { (relativePath, content) -> "$relativePath:${sha256Hex(content.toByteArray())}" }.sorted()
+        return sha256Hex(entries.joinToString(",", "[", "]") { "\"$it\"" }.toByteArray())
+    }
+
     // Registers `hash` as the currently installed package, with the given file contents, so that
     // getCurrentPackageFolderPath() resolves to it. Needed to set up diff-update scenarios.
     private fun installCurrentPackage(update: CodePushUpdateManager, hash: String, files: Map<String, String>): String {
@@ -303,4 +313,31 @@ class CodePushUpdateManagerTest {
         assertEquals("new bundle contents", File(newUpdateFolderPath, "index.android.bundle").readText())
         assertFalse("the manifest itself should not be carried into the installed package", File(newUpdateFolderPath, CodePushConstants.DIFF_MANIFEST_FILE_NAME).exists())
     }
+
+    @Test
+    fun installDownloadedUpdate_binaryDiffUpdate_passesFolderHashCheckWithoutPatchesFolder() {
+        // Given
+        val update = manager(enableDeltaUpdates = true)
+        installCurrentPackage(update, "current-hash", mapOf("kept.txt" to "kept contents"))
+        val downloadFile = zipOf(
+            CodePushConstants.DIFF_MANIFEST_FILE_NAME to """{"version":2,"deletedFiles":[],"patchedFiles":{}}""",
+            "index.android.bundle" to "new bundle contents",
+            "${CodePushConstants.DIFF_PATCHES_FOLDER_NAME}/unused.bsdiff" to "patch bytes",
+        )
+        // The release contents, as the CLI hashes them: the patches folder and the manifest are not part of them.
+        val pkg = updatePackage(releaseHashOf(mapOf(
+            "kept.txt" to "kept contents",
+            "index.android.bundle" to "new bundle contents",
+        )))
+        val newUpdateFolderPath = update.getPackageFolderPath("new-hash")
+        val newUpdateMetadataPath = CodePushUtils.appendPathComponent(newUpdateFolderPath, CodePushConstants.PACKAGE_FILE_NAME)
+
+        // When
+        update.installDownloadedUpdate(pkg, "index.android.bundle", null, downloadFile, true, newUpdateFolderPath, newUpdateMetadataPath)
+
+        // Then
+        assertEquals("kept contents", File(newUpdateFolderPath, "kept.txt").readText())
+        assertEquals("new bundle contents", File(newUpdateFolderPath, "index.android.bundle").readText())
+        assertFalse(File(newUpdateFolderPath, CodePushConstants.DIFF_PATCHES_FOLDER_NAME).exists())
+    }
 }

android/app/src/test/java/com/microsoft/codepush/react/FileUtilsTest.kt

@@ -1,6 +1,7 @@
 package com.microsoft.codepush.react
 
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Rule
 import org.junit.Test
@@ -31,4 +32,25 @@ class FileUtilsTest {
         assertTrue(copiedNestedFile.exists())
         assertEquals("child contents", copiedNestedFile.readText())
     }
+
+    @Test
+    fun copyDirectoryContents_withExcludedEntry_skipsItOnlyAtTopLevel() {
+        // Given
+        val sourceDir = tempFolder.newFolder("source")
+        File(sourceDir, "root.txt").writeText("root contents")
+        File(sourceDir, "excluded").mkdir()
+        File(sourceDir, "excluded/skipped.txt").writeText("skipped contents")
+        File(sourceDir, "nested/excluded").mkdirs()
+        File(sourceDir, "nested/excluded/kept.txt").writeText("kept contents")
+
+        val destinationDir = File(tempFolder.root, "destination")
+
+        // When
+        FileUtils.copyDirectoryContents(sourceDir.absolutePath, destinationDir.absolutePath, "excluded")
+
+        // Then
+        assertEquals("root contents", File(destinationDir, "root.txt").readText())
+        assertFalse(File(destinationDir, "excluded").exists())
+        assertEquals("kept contents", File(destinationDir, "nested/excluded/kept.txt").readText())
+    }
 }

android/app/src/test/java/com/microsoft/codepush/react/diffpatch/DiffManifestTest.kt

@@ -152,6 +152,77 @@ class DiffManifestTest {
         }
     }
 
+    private fun manifestWithPatchedFileKey(relativePath: String) = JSONObject().apply {
+        put("version", 2)
+        put("patchedFiles", JSONObject().apply {
+            put(relativePath, JSONObject().apply {
+                put("algo", "bsdiff")
+                put("baseHash", "base-hash-1")
+                put("targetHash", "target-hash-1")
+                put("patch", "__hcp_patches/relative/path.js.bsdiff")
+            })
+        })
+    }
+
+    @Test
+    fun parseDiffManifest_patchedFileKeyResolvingIntoReservedFolder_throws() {
+        val keys = listOf(
+            "__hcp_patches",
+            "__hcp_patches/",
+            "__hcp_patches/relative/path.js",
+            "./__hcp_patches/relative/path.js",
+            "/__hcp_patches/relative/path.js",
+        )
+        for (key in keys) {
+            try {
+                parseDiffManifest(manifestWithPatchedFileKey(key))
+                fail("expected JSONException for key \"$key\"")
+            } catch (e: org.json.JSONException) {
+                assertTrue(e.message, e.message!!.contains("reserved \"__hcp_patches/\" folder"))
+            }
+        }
+    }
+
+    @Test
+    fun parseDiffManifest_patchedFileKeyOutsideReservedFolder_isAccepted() {
+        val keys = listOf(
+            "assets/__hcp_patches/path.js",
+            "__hcp_patches_extra/path.js",
+        )
+        for (key in keys) {
+            val manifest = parseDiffManifest(manifestWithPatchedFileKey(key))
+            assertEquals(setOf(key), manifest.patchedFiles.keys)
+        }
+    }
+
+    @Test
+    fun parseDiffManifest_patchedFileKeyWithParentComponent_throws() {
+        val keys = listOf(
+            "relative/../__hcp_patches/path.js",
+            "__hcp_patches/../relative/path.js",
+            "relative/../path.js",
+            "../path.js",
+            "relative/..",
+        )
+        for (key in keys) {
+            try {
+                parseDiffManifest(manifestWithPatchedFileKey(key))
+                fail("expected JSONException for key \"$key\"")
+            } catch (e: org.json.JSONException) {
+                assertTrue(e.message, e.message!!.contains("must not contain \"..\" components"))
+            }
+        }
+    }
+
+    @Test
+    fun parseDiffManifest_patchedFileKeyWithDotsInComponentName_isAccepted() {
+        val keys = listOf("relative/..path.js", "relative/path..js", "...")
+        for (key in keys) {
+            val manifest = parseDiffManifest(manifestWithPatchedFileKey(key))
+            assertEquals(setOf(key), manifest.patchedFiles.keys)
+        }
+    }
+
     @Test
     fun parseDiffManifest_patchedFileEntryPatchOutsideReservedPrefix_throws() {
         assertPatchFieldRejectedByParser("relative/path.js.bsdiff")

ios/CodePush/CodePush.h

@@ -180,6 +180,12 @@ failCallback:(void (^)(NSError *err))failCallback;
                  destFolder:(NSString *)destFolder
                       error:(NSError **)error;
 
+// excludedEntryName applies only to the top level of sourceFolder: nested entries with the same name are copied.
++ (BOOL)copyEntriesInFolder:(NSString *)sourceFolder
+                 destFolder:(NSString *)destFolder
+             excludingEntry:(NSString *)excludedEntryName
+                      error:(NSError **)error;
+
 + (NSString *)findMainBundleInFolder:(NSString *)folderPath
                     expectedFileName:(NSString *)expectedFileName
                                error:(NSError **)error;

ios/CodePush/CodePushDiffManifest.h

@@ -28,6 +28,11 @@ extern NSString *const CodePushDiffPatchesFolderName;
 // No version field, or version 1: original format, file-by-file patching only.
 // Version 2: adds support for binary diff patching.
 @property (nonatomic, readonly, assign) NSInteger version;
+// YES if this manifest describes a binary diff update, which ships its patches under CodePushDiffPatchesFolderName.
+// This is a subset of "diff updates" in general, as a diff update payload can also consist of:
+// - The modified files included in the ZIP, which are applied on top of the existing files (without any binary patching)
+// - The list of files to delete from the old package
+@property (nonatomic, readonly, assign) BOOL isBinaryDiff;
 // Relative paths, from the old package, to delete rather than carry over into the new one.
 @property (nonatomic, readonly, copy) NSArray<NSString *> *deletedFiles;
 // Key: file's relative path in the package being installed.

ios/CodePush/CodePushDiffManifest.m

@@ -16,6 +16,19 @@
 
 NSString *const CodePushDiffPatchesFolderName = @"__hcp_patches";
 
+// The top-level entry that `relativePath` names under a base folder. Empty and
+// "." components are skipped. The caller rejects ".." components first, so
+// they need no handling here.
+static NSString *topLevelComponentOf(NSString *relativePath)
+{
+    for (NSString *component in [relativePath componentsSeparatedByString:@"/"]) {
+        if (component.length > 0 && ![component isEqualToString:@"."]) {
+            return component;
+        }
+    }
+    return nil;
+}
+
 static BOOL isAbsent(id value)
 {
     return value == nil || [value isKindOfClass:[NSNull class]];
@@ -111,6 +124,11 @@ - (instancetype)initWithVersion:(NSInteger)version
     return self;
 }
 
+- (BOOL)isBinaryDiff
+{
+    return self.version == 2;
+}
+
 + (nullable instancetype)manifestFromJSON:(NSDictionary *)json error:(NSError **)error
 {
     if (![json isKindOfClass:[NSDictionary class]]) {
@@ -157,6 +175,17 @@ + (nullable instancetype)manifestFromJSON:(NSDictionary *)json error:(NSError **
         }
         for (NSString *relativePath in (NSDictionary *)patchedFilesJSON) {
             NSString *context = [NSString stringWithFormat:@"patchedFiles[\"%@\"]", relativePath];
+            NSString *reservedPatchesFolderPrefix = [CodePushDiffPatchesFolderName stringByAppendingString:@"/"];
+
+            if ([[relativePath componentsSeparatedByString:@"/"] containsObject:@".."]) {
+                if (error) *error = [CodePushErrorUtils errorWithMessage:[NSString stringWithFormat:@"Diff manifest %@ must not contain \"..\" components", context]];
+                return nil;
+            }
+
+            if ([topLevelComponentOf(relativePath) isEqualToString:CodePushDiffPatchesFolderName]) {
+                if (error) *error = [CodePushErrorUtils errorWithMessage:[NSString stringWithFormat:@"Diff manifest %@ targets the reserved \"%@\" folder, which is not part of the installed package", context, reservedPatchesFolderPrefix]];
+                return nil;
+            }
 
             id entryJSON = patchedFilesJSON[relativePath];
             if (![entryJSON isKindOfClass:[NSDictionary class]]) {
@@ -173,7 +202,6 @@ + (nullable instancetype)manifestFromJSON:(NSDictionary *)json error:(NSError **
             }
 
             NSString *patch = entryJSON[@"patch"];
-            NSString *reservedPatchesFolderPrefix = [CodePushDiffPatchesFolderName stringByAppendingString:@"/"];
             if (![patch hasPrefix:reservedPatchesFolderPrefix]) {
                 if (error) *error = [CodePushErrorUtils errorWithMessage:[NSString stringWithFormat:@"Diff manifest %@ field \"patch\" must be under the reserved \"%@\" prefix, but is \"%@\"", context, reservedPatchesFolderPrefix, patch]];
                 return nil;
@@ -189,14 +217,15 @@ + (nullable instancetype)manifestFromJSON:(NSDictionary *)json error:(NSError **
     // Only version 2 defines file patching.
     // A manifest of any other version that lists patched files is malformed,
     // and applying none of them would leave the old bytes behind.
-    if (version != 2 && patchedFiles.count > 0) {
+    CodePushDiffManifest *manifest = [[CodePushDiffManifest alloc] initWithVersion:version
+                                                                      deletedFiles:deletedFiles
+                                                                      patchedFiles:patchedFiles];
+    if (!manifest.isBinaryDiff && patchedFiles.count > 0) {
         if (error) *error = [CodePushErrorUtils errorWithMessage:[NSString stringWithFormat:@"Diff manifest declares version %ld but lists %lu patchedFiles, which require version 2", (long)version, (unsigned long)patchedFiles.count]];
         return nil;
     }
 
-    return [[CodePushDiffManifest alloc] initWithVersion:version
-                                            deletedFiles:deletedFiles
-                                            patchedFiles:patchedFiles];
+    return manifest;
 }
 
 + (nullable NSString *)resolvePath:(NSString *)relativePath