Support zip files with unsupported compresssion method

app/core/src/main/java/com/topjohnwu/magisk/core/download/DownloadEngine.kt

@@ -29,9 +29,8 @@ import com.topjohnwu.magisk.core.isRunningAsStub
 import com.topjohnwu.magisk.core.ktx.cachedFile
 import com.topjohnwu.magisk.core.ktx.copyAll
 import com.topjohnwu.magisk.core.ktx.copyAndClose
-import com.topjohnwu.magisk.core.ktx.forEach
 import com.topjohnwu.magisk.core.ktx.set
-import com.topjohnwu.magisk.core.ktx.withStreams
+import com.topjohnwu.magisk.core.ktx.withInOut
 import com.topjohnwu.magisk.core.ktx.writeTo
 import com.topjohnwu.magisk.core.tasks.AppMigration
 import com.topjohnwu.magisk.core.utils.MediaStoreUtils.outputStream
@@ -43,14 +42,13 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.launch
 import okhttp3.ResponseBody
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry
+import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream
+import org.apache.commons.compress.archivers.zip.ZipFile
 import timber.log.Timber
 import java.io.IOException
 import java.io.InputStream
 import java.io.OutputStream
-import java.util.zip.ZipEntry
-import java.util.zip.ZipFile
-import java.util.zip.ZipInputStream
-import java.util.zip.ZipOutputStream
 
 /**
  * This class drives the execution of file downloads and notification management.
@@ -99,33 +97,35 @@ class DownloadEngine(
             }
         }
 
-        private fun createIntent(context: Context, subject: Subject) =
+        private fun createBroadcastIntent(context: Context, subject: Subject) =
-            if (Build.VERSION.SDK_INT >= 34) {
+            context.intent<com.topjohnwu.magisk.core.Receiver>()
-                context.intent<com.topjohnwu.magisk.core.Receiver>()
+                .setAction(ACTION)
-                    .setAction(ACTION)
+                .putExtra(SUBJECT_KEY, subject)
-                    .putExtra(SUBJECT_KEY, subject)
+
-            } else {
+        private fun createServiceIntent(context: Context, subject: Subject) =
-                context.intent<com.topjohnwu.magisk.core.Service>()
+            context.intent<com.topjohnwu.magisk.core.Service>()
-                    .setAction(ACTION)
+                .setAction(ACTION)
-                    .putExtra(SUBJECT_KEY, subject)
+                .putExtra(SUBJECT_KEY, subject)
-            }
 
         @SuppressLint("InlinedApi")
         fun getPendingIntent(context: Context, subject: Subject): PendingIntent {
             val flag = PendingIntent.FLAG_IMMUTABLE or
                 PendingIntent.FLAG_UPDATE_CURRENT or
                 PendingIntent.FLAG_ONE_SHOT
-            val intent = createIntent(context, subject)
             return if (Build.VERSION.SDK_INT >= 34) {
                 // On API 34+, download tasks are handled with a user-initiated job.
                 // However, there is no way to schedule a new job directly with a pending intent.
                 // As a workaround, we send the subject to a broadcast receiver and have it
                 // schedule the job for us.
+                val intent = createBroadcastIntent(context, subject)
                 PendingIntent.getBroadcast(context, REQUEST_CODE, intent, flag)
-            } else if (Build.VERSION.SDK_INT >= 26) {
-                PendingIntent.getForegroundService(context, REQUEST_CODE, intent, flag)
             } else {
-                PendingIntent.getService(context, REQUEST_CODE, intent, flag)
+                val intent = createServiceIntent(context, subject)
+                if (Build.VERSION.SDK_INT >= 26) {
+                    PendingIntent.getForegroundService(context, REQUEST_CODE, intent, flag)
+                } else {
+                    PendingIntent.getService(context, REQUEST_CODE, intent, flag)
+                }
             }
         }
 
@@ -152,10 +152,13 @@ class DownloadEngine(
                     .setTransientExtras(extras)
                     .build()
                 scheduler.schedule(info)
-            } else if (Build.VERSION.SDK_INT >= 26) {
-                context.startForegroundService(createIntent(context, subject))
             } else {
-                context.startService(createIntent(context, subject))
+                val intent = createServiceIntent(context, subject)
+                if (Build.VERSION.SDK_INT >= 26) {
+                    context.startForegroundService(intent)
+                } else {
+                    context.startService(intent)
+                }
             }
         }
     }
@@ -285,11 +288,11 @@ class DownloadEngine(
                 }
 
                 // Extract stub
-                val zf = ZipFile(updateApk)
                 val apk = context.cachedFile("stub.apk")
-                apk.delete()
+                ZipFile.Builder().setFile(updateApk).get().use { zf ->
-                zf.getInputStream(zf.getEntry("assets/stub.apk")).writeTo(apk)
+                    apk.delete()
-                zf.close()
+                    zf.getInputStream(zf.getEntry("assets/stub.apk")).writeTo(apk)
+                }
 
                 // Patch and install
                 subject.intent = AppMigration.upgradeStub(context, apk)
@@ -308,29 +311,36 @@ class DownloadEngine(
     }
 
     private suspend fun handleModule(src: InputStream, file: Uri) {
-        val input = ZipInputStream(src)
+        val tmp = context.cachedFile("module.zip")
-        val output = ZipOutputStream(file.outputStream())
+        try {
+            // First download the entire zip into cache so we can process it
+            src.writeTo(tmp)
 
-        withStreams(input, output) { zin, zout ->
+            val input = ZipFile.Builder().setFile(tmp).get()
-            zout.putNextEntry(ZipEntry("META-INF/"))
+            val output = ZipArchiveOutputStream(file.outputStream())
-            zout.putNextEntry(ZipEntry("META-INF/com/"))
+            withInOut(input, output) { zin, zout ->
-            zout.putNextEntry(ZipEntry("META-INF/com/google/"))
+                zout.putArchiveEntry(ZipArchiveEntry("META-INF/"))
-            zout.putNextEntry(ZipEntry("META-INF/com/google/android/"))
+                zout.closeArchiveEntry()
-            zout.putNextEntry(ZipEntry("META-INF/com/google/android/update-binary"))
+                zout.putArchiveEntry(ZipArchiveEntry("META-INF/com/"))
-            context.assets.open("module_installer.sh").use { it.copyAll(zout) }
+                zout.closeArchiveEntry()
+                zout.putArchiveEntry(ZipArchiveEntry("META-INF/com/google/"))
+                zout.closeArchiveEntry()
+                zout.putArchiveEntry(ZipArchiveEntry("META-INF/com/google/android/"))
+                zout.closeArchiveEntry()
 
-            zout.putNextEntry(ZipEntry("META-INF/com/google/android/updater-script"))
+                zout.putArchiveEntry(ZipArchiveEntry("META-INF/com/google/android/update-binary"))
-            zout.write("#MAGISK\n".toByteArray())
+                context.assets.open("module_installer.sh").use { it.copyAll(zout) }
+                zout.closeArchiveEntry()
 
-            zin.forEach { entry ->
+                zout.putArchiveEntry(ZipArchiveEntry("META-INF/com/google/android/updater-script"))
-                val path = entry.name
+                zout.write("#MAGISK\n".toByteArray())
-                if (path.isNotEmpty() && !path.startsWith("META-INF")) {
+                zout.closeArchiveEntry()
-                    zout.putNextEntry(ZipEntry(path))
+
-                    if (!entry.isDirectory) {
+                // Then simply copy all entries to output
-                        zin.copyAll(zout)
+                zin.copyRawEntries(zout) { entry -> !entry.name.startsWith("META-INF") }
-                    }
-                }
             }
+        } finally {
+            tmp.delete()
         }
     }
 

app/core/src/main/java/com/topjohnwu/magisk/core/ktx/XJVM.kt

@@ -8,6 +8,7 @@ import kotlinx.coroutines.flow.flatMapMerge
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.isActive
 import kotlinx.coroutines.withContext
+import java.io.Closeable
 import java.io.File
 import java.io.IOException
 import java.io.InputStream
@@ -17,24 +18,14 @@ import java.text.DateFormat
 import java.text.SimpleDateFormat
 import java.util.Collections
 import java.util.Locale
-import java.util.zip.ZipEntry
-import java.util.zip.ZipInputStream
 
-inline fun ZipInputStream.forEach(callback: (ZipEntry) -> Unit) {
+inline fun <In : Closeable, Out : Closeable> withInOut(
-    var entry: ZipEntry? = nextEntry
+    input: In,
-    while (entry != null) {
+    output: Out,
-        callback(entry)
-        entry = nextEntry
-    }
-}
-
-inline fun <In : InputStream, Out : OutputStream> withStreams(
-    inStream: In,
-    outStream: Out,
     withBoth: (In, Out) -> Unit
 ) {
-    inStream.use { reader ->
+    input.use { reader ->
-        outStream.use { writer ->
+        output.use { writer ->
             withBoth(reader, writer)
         }
     }
@@ -64,7 +55,7 @@ suspend inline fun InputStream.copyAndClose(
     out: OutputStream,
     bufferSize: Int = DEFAULT_BUFFER_SIZE,
     dispatcher: CoroutineDispatcher = Dispatchers.IO
-) = withStreams(this, out) { i, o -> i.copyAll(o, bufferSize, dispatcher) }
+) = withInOut(this, out) { i, o -> i.copyAll(o, bufferSize, dispatcher) }
 
 @Throws(IOException::class)
 suspend inline fun InputStream.writeTo(

app/core/src/main/java/com/topjohnwu/magisk/core/utils/Desugar.java

@@ -2,6 +2,10 @@ package com.topjohnwu.magisk.core.utils;
 
 import android.os.Build;
 
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
+import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
+import org.apache.commons.compress.archivers.zip.ZipUtil;
+
 import java.nio.file.attribute.FileTime;
 import java.util.zip.ZipEntry;
 
@@ -29,4 +33,15 @@ public class Desugar {
             return null;
         }
     }
+
+    /**
+     * Within {@link ZipArchiveOutputStream#copyFromZipInputStream}, we redirect the method call
+     * {@link ZipUtil#checkRequestedFeatures} to this method. This is safe because the only usage
+     * of copyFromZipInputStream is in {@link ZipArchiveOutputStream#addRawArchiveEntry},
+     * which does not need to actually understand the content of the zip entry. By removing
+     * this feature check, we can modify zip files using unsupported compression methods.
+     */
+    public static void checkRequestedFeatures(final ZipArchiveEntry ze) {
+        // No-op
+    }
 }

buildSrc/src/main/java/DesugarClassVisitorFactory.kt

@@ -7,8 +7,10 @@ import org.objectweb.asm.MethodVisitor
 import org.objectweb.asm.Opcodes
 import org.objectweb.asm.Opcodes.ASM9
 
-private const val ZIP_ENTRY_CLASS_NAME = "java.util.zip.ZipEntry"
 private const val DESUGAR_CLASS_NAME = "com.topjohnwu.magisk.core.utils.Desugar"
+private const val ZIP_ENTRY_CLASS_NAME = "java.util.zip.ZipEntry"
+private const val ZIP_OUT_STREAM_CLASS_NAME = "org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream"
+private const val ZIP_UTIL_CLASS_NAME = "org/apache/commons/compress/archivers/zip/ZipUtil"
 private const val ZIP_ENTRY_GET_TIME_DESC = "()Ljava/nio/file/attribute/FileTime;"
 private const val DESUGAR_GET_TIME_DESC =
     "(Ljava/util/zip/ZipEntry;)Ljava/nio/file/attribute/FileTime;"
@@ -20,27 +22,29 @@ abstract class DesugarClassVisitorFactory : AsmClassVisitorFactory<Instrumentati
         classContext: ClassContext,
         nextClassVisitor: ClassVisitor
     ): ClassVisitor {
-        return DesugarClassVisitor(classContext, nextClassVisitor)
+        return if (classContext.currentClassData.className == ZIP_OUT_STREAM_CLASS_NAME) {
+            ZipEntryPatcher(classContext, ZipOutputStreamPatcher(nextClassVisitor))
+        } else {
+            ZipEntryPatcher(classContext, nextClassVisitor)
+        }
     }
 
     override fun isInstrumentable(classData: ClassData) = classData.className != DESUGAR_CLASS_NAME
 
-    class DesugarClassVisitor(private val classContext: ClassContext, cv: ClassVisitor) :
+    // Patch ALL references to ZipEntry#getXXXTime
-        ClassVisitor(ASM9, cv) {
+    class ZipEntryPatcher(
+        private val classContext: ClassContext,
+        cv: ClassVisitor
+    ) : ClassVisitor(ASM9, cv) {
         override fun visitMethod(
             access: Int,
             name: String?,
             descriptor: String?,
             signature: String?,
             exceptions: Array<out String>?
-        ): MethodVisitor {
+        ) = MethodPatcher(super.visitMethod(access, name, descriptor, signature, exceptions))
-            return DesugarMethodVisitor(
-                super.visitMethod(access, name, descriptor, signature, exceptions)
-            )
-        }
 
-        inner class DesugarMethodVisitor(mv: MethodVisitor?) :
+        inner class MethodPatcher(mv: MethodVisitor?) : MethodVisitor(ASM9, mv) {
-            MethodVisitor(ASM9, mv) {
             override fun visitMethodInsn(
                 opcode: Int,
                 owner: String,
@@ -75,4 +79,44 @@ abstract class DesugarClassVisitorFactory : AsmClassVisitorFactory<Instrumentati
             }
         }
     }
+
+    // Patch ZipArchiveOutputStream#copyFromZipInputStream
+    class ZipOutputStreamPatcher(cv: ClassVisitor) : ClassVisitor(ASM9, cv) {
+        override fun visitMethod(
+            access: Int,
+            name: String,
+            descriptor: String,
+            signature: String?,
+            exceptions: Array<out String?>?
+        ): MethodVisitor? {
+            return if (name == "copyFromZipInputStream") {
+                MethodPatcher(super.visitMethod(access, name, descriptor, signature, exceptions))
+            } else {
+                super.visitMethod(access, name, descriptor, signature, exceptions)
+            }
+        }
+
+        class MethodPatcher(mv: MethodVisitor?) : MethodVisitor(ASM9, mv) {
+            override fun visitMethodInsn(
+                opcode: Int,
+                owner: String,
+                name: String,
+                descriptor: String?,
+                isInterface: Boolean
+            ) {
+                if (owner == ZIP_UTIL_CLASS_NAME && name == "checkRequestedFeatures") {
+                    // Redirect
+                    mv.visitMethodInsn(
+                        Opcodes.INVOKESTATIC,
+                        DESUGAR_CLASS_NAME.replace('.', '/'),
+                        name,
+                        descriptor,
+                        false
+                    )
+                } else {
+                    super.visitMethodInsn(opcode, owner, name, descriptor, isInterface)
+                }
+            }
+        }
+    }
 }