Support 7z extraction, and fix .xz extraction which would have crashed at runtime

Commons Compress declares its LZMA/LZMA2 support against org.tukaani:xz as an *optional* dependency, so nothing pulled it in transitively and nothing failed at build time. The app nevertheless advertised .xz as extractable, so the first user to open one would have got NoClassDefFoundError: org/tukaani/xz/FilterOptions. XZCompressorInputStream references org/tukaani/xz/XZInputStream directly. The library is now declared, and a round-trip test covers the path that was silently missing. That same library is what 7z needs, and Commons Compress already ships a pure-Java 7z reader, so 7z extraction costs no native code and no further dependency. The docs had assumed the format was off limits and said so in three places, including the customer-facing store copy. 7z is the only format here that cannot arrive through a stream: its header sits at the end of the file, so the reader has to seek. That is now a capability the backend advertises, StorageSource.openSeekable: the all-files backend answers with a real FileChannel, the SAF fallback with null, so folder mode reports needs-a-seekable-file instead of failing obscurely. Entry names go through the same ArchivePath.safeRelativePath zip-slip guard. 7z creation stays deferred.
2026-10-15 08:47:37 +02:00
parent a461be2a66
commit ba92cf392d
13 changed files with 326 additions and 51 deletions
@@ -181,6 +181,9 @@ dependencies {

    // ZIP/TAR/GZ archive support. Pure Java — no native code, no NDK.
    implementation(libs.commons.compress)
+    // Commons Compress's JVM-only dependencies for LZMA/LZMA2: `.xz` and 7z. Optional
+    // upstream, so it has to be declared here or those formats fail at runtime.
+    implementation(libs.xz)

    debugImplementation(libs.androidx.compose.ui.tooling)

@@ -3,10 +3,16 @@ package de.softwareapp_hb.filemanager.core
 /**
 * Which archive formats the app can handle, by file name.
 *
- * The list is deliberately honest: ZIP, TAR, the compression wrappers, and single-file
- * gzip/bzip2/xz all come from `java.util.zip` and Commons Compress, both of which are
- * pure Java. RAR and 7z are recognised so the UI can say so instead of failing with a
- * confusing error — they need native code, which this project does not ship.
+ * The list is deliberately honest about what the pure-Java libraries underneath can
+ * actually do. ZIP, TAR, the compression wrappers and 7z all come from `java.util.zip`
+ * and Commons Compress; RAR, CAB, ISO and WIM are recognised so the UI can say they are
+ * not supported instead of failing with a confusing error.
+ *
+ * 7z is worth calling out, because it does **not** need native code — Commons Compress
+ * ships a pure-Java 7z reader, `SevenZFile`. What it does need is **seekable** input:
+ * unlike ZIP, a 7z header sits at the *end* of the file and the reader has to seek to it.
+ * So 7z is only extractable on a backend that can supply a channel, and the engine says
+ * so plainly when one cannot rather than failing obscurely.
 */
 enum class ArchiveFormat {
    ZIP,
@@ -15,8 +21,9 @@ enum class ArchiveFormat {
    GZIP,
    BZIP2,
    XZ,
+    SEVEN_ZIP,

-    /** Recognised but not supported (needs native code). */
+    /** Recognised but not supported — these really do need native code. */
    UNSUPPORTED,

    /** Not an archive name we know. */
@@ -29,7 +36,8 @@ object ArchiveFormats {
    val creatable: Set<ArchiveFormat> = setOf(ArchiveFormat.ZIP)

    private val ZIP_NAMES = setOf("zip", "jar", "cbz")
-    private val UNSUPPORTED_NAMES = setOf("7z", "rar", "cab", "iso", "wim")
+    private val SEVEN_ZIP_NAMES = setOf("7z")
+    private val UNSUPPORTED_NAMES = setOf("rar", "cab", "iso", "wim")

    fun formatOf(fileName: String): ArchiveFormat {
        val lower = fileName.lowercase()
@@ -43,6 +51,7 @@ object ArchiveFormats {
                val extension = FileKind.extensionOf(lower)
                when {
                    extension in ZIP_NAMES -> ArchiveFormat.ZIP
+                    extension in SEVEN_ZIP_NAMES -> ArchiveFormat.SEVEN_ZIP
                    extension in UNSUPPORTED_NAMES -> ArchiveFormat.UNSUPPORTED
                    else -> ArchiveFormat.UNKNOWN
                }
@@ -10,6 +10,9 @@ import java.io.FileInputStream
 import java.io.FileOutputStream
 import java.io.InputStream
 import java.io.OutputStream
+import java.nio.channels.FileChannel
+import java.nio.channels.SeekableByteChannel
+import java.nio.file.StandardOpenOption

 /**
 * The "All files access" backend: plain `java.io.File` over absolute paths.
@@ -76,6 +79,17 @@ class AllFilesSource(
        FileInputStream(File(path.value))
    }

+    /**
+     * A real `FileChannel`, because this backend works on actual filesystem paths.
+     *
+     * This is what makes 7z extractable in all-files mode, and what the SAF fallback
+     * cannot offer.
+     */
+    override suspend fun openSeekable(path: StoragePath): SeekableByteChannel? = withContext(Dispatchers.IO) {
+        val file = File(path.value)
+        if (!file.isFile) null else FileChannel.open(file.toPath(), StandardOpenOption.READ)
+    }
+
    @SuppressLint("Recycle")
    override suspend fun openWrite(path: StoragePath, append: Boolean): OutputStream =
        withContext(Dispatchers.IO) {
@@ -4,6 +4,7 @@ import de.softwareapp_hb.filemanager.core.FileEntry
 import de.softwareapp_hb.filemanager.core.StoragePath
 import java.io.InputStream
 import java.io.OutputStream
+import java.nio.channels.SeekableByteChannel

 /** A backend operation failed for a reason worth surfacing to the user. */
 class StorageException(message: String, cause: Throwable? = null) : Exception(message, cause)
@@ -57,6 +58,18 @@ interface StorageSource {
    /** Open [path] for reading. The caller closes the stream. */
    suspend fun openRead(path: StoragePath): InputStream

+    /**
+     * Open [path] as a random-access channel, or `null` when this backend cannot.
+     *
+     * Only 7z needs this. Unlike ZIP, a 7z header sits at the *end* of the file, so it
+     * cannot be read from a forward-only stream — the reader has to seek. [AllFilesSource]
+     * hands out a real `FileChannel`; [SafSource] returns `null`, because a document URI
+     * has no seekable representation. The default here is that honest `null`, so a new
+     * backend without seekable support degrades to "7z is not extractable here" rather
+     * than to a corrupt read.
+     */
+    suspend fun openSeekable(path: StoragePath): SeekableByteChannel? = null
+
    /** Open [path] for writing, optionally appending. The caller closes the stream. */
    suspend fun openWrite(path: StoragePath, append: Boolean = false): OutputStream

@@ -8,6 +8,7 @@ import de.softwareapp_hb.filemanager.data.StorageSource
 import kotlinx.coroutines.currentCoroutineContext
 import kotlinx.coroutines.isActive
 import org.apache.commons.compress.archivers.ArchiveInputStream
+import org.apache.commons.compress.archivers.sevenz.SevenZFile
 import org.apache.commons.compress.archivers.tar.TarArchiveInputStream
 import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream
 import org.apache.commons.compress.compressors.bzip2.BZip2CompressorInputStream
@@ -16,6 +17,7 @@ import org.apache.commons.compress.compressors.xz.XZCompressorInputStream
 import java.io.BufferedInputStream
 import java.io.BufferedOutputStream
 import java.io.InputStream
+import java.nio.channels.SeekableByteChannel
 import java.util.zip.ZipEntry
 import java.util.zip.ZipOutputStream
 import kotlin.coroutines.cancellation.CancellationException
@@ -24,8 +26,10 @@ import kotlin.coroutines.cancellation.CancellationException
 * Creates and extracts archives, on top of the storage backends.
 *
 * Everything here is pure Java — `java.util.zip` for writing, Commons Compress for
- * reading — so the app ships no native code and all of this is testable on the JVM. RAR
- * and 7z are recognised but refused (see [ArchiveFormats]) rather than half-supported.
+ * reading — so the app ships no native code and all of this is testable on the JVM. RAR is
+ * recognised but refused (see [ArchiveFormats]) rather than half-supported. 7z *is* read,
+ * and needs no native code either, but it is the one format that cannot arrive through a
+ * stream: its header lives at the end of the file, so it needs a seekable channel.
 *
 * The security-critical part is that every entry name goes through
 * [ArchivePath.safeRelativePath] before it becomes a path: an archive must never be able
@@ -105,41 +109,58 @@ class ArchiveEngine(private val source: StorageSource) {

        try {
            source.createDirectory(destination)
-            source.openRead(archive).use { raw ->
-                BufferedInputStream(raw).use { input ->
-                    when (format) {
-                        ArchiveFormat.ZIP -> readArchive(ZipArchiveInputStream(input), destination, state)

-                        ArchiveFormat.TAR -> readArchive(TarArchiveInputStream(input), destination, state)
+            // 7z is the one format that cannot come in through a stream, so it is handled
+            // before the stream is even opened. Only a backend with real filesystem paths
+            // can hand out a channel; the SAF fallback cannot, and says so plainly rather
+            // than failing with something obscure.
+            if (format == ArchiveFormat.SEVEN_ZIP) {
+                val channel = source.openSeekable(archive)
+                if (channel == null) {
+                    state.failure(
+                        archive,
+                        "7z needs a seekable file, which this storage mode cannot provide",
+                    )
+                } else {
+                    readSevenZip(channel, destination, state)
+                }
+            } else {
+                source.openRead(archive).use { raw ->
+                    BufferedInputStream(raw).use { input ->
+                        when (format) {
+                            ArchiveFormat.ZIP -> readArchive(ZipArchiveInputStream(input), destination, state)

-                        ArchiveFormat.TAR_GZIP -> readArchive(
-                            TarArchiveInputStream(GzipCompressorInputStream(input)),
-                            destination,
-                            state,
-                        )
+                            ArchiveFormat.TAR -> readArchive(TarArchiveInputStream(input), destination, state)

-                        ArchiveFormat.GZIP -> readSingle(
-                            GzipCompressorInputStream(input),
-                            ArchiveFormats.innerName(archive.name, format),
-                            destination,
-                            state,
-                        )
+                            ArchiveFormat.TAR_GZIP -> readArchive(
+                                TarArchiveInputStream(GzipCompressorInputStream(input)),
+                                destination,
+                                state,
+                            )

-                        ArchiveFormat.BZIP2 -> readSingle(
-                            BZip2CompressorInputStream(input),
-                            ArchiveFormats.innerName(archive.name, format),
-                            destination,
-                            state,
-                        )
+                            ArchiveFormat.GZIP -> readSingle(
+                                GzipCompressorInputStream(input),
+                                ArchiveFormats.innerName(archive.name, format),
+                                destination,
+                                state,
+                            )

-                        ArchiveFormat.XZ -> readSingle(
-                            XZCompressorInputStream(input),
-                            ArchiveFormats.innerName(archive.name, format),
-                            destination,
-                            state,
-                        )
+                            ArchiveFormat.BZIP2 -> readSingle(
+                                BZip2CompressorInputStream(input),
+                                ArchiveFormats.innerName(archive.name, format),
+                                destination,
+                                state,
+                            )

-                        else -> state.failure(archive, "This archive format is not supported")
+                            ArchiveFormat.XZ -> readSingle(
+                                XZCompressorInputStream(input),
+                                ArchiveFormats.innerName(archive.name, format),
+                                destination,
+                                state,
+                            )
+
+                            else -> state.failure(archive, "This archive format is not supported")
+                        }
                    }
                }
            }
@@ -182,6 +203,60 @@ class ArchiveEngine(private val source: StorageSource) {
        }
    }

+    /**
+     * Read a 7z container from [channel].
+     *
+     * 7z entries are not exposed as an [ArchiveInputStream]: the format decompresses in
+     * solid blocks that can span entry boundaries, so the entry loop drives [SevenZFile]
+     * directly and reads each entry's bytes from whatever the current one is.
+     *
+     * Every name still goes through [ArchivePath.safeRelativePath] before it becomes a
+     * path, exactly as the streaming readers do — a 7z entry name is no more trustworthy
+     * than a ZIP one, and this is the same zip-slip defence.
+     */
+    private suspend fun readSevenZip(
+        channel: SeekableByteChannel,
+        destination: StoragePath,
+        state: ArchiveState,
+    ) {
+        // Both are closed deliberately. SevenZFile does close a channel it was handed, but
+        // a second close on a FileChannel is a documented no-op, so relying on both means
+        // the channel cannot leak whichever way that behaves.
+        channel.use { seekable ->
+            SevenZFile.builder().setSeekableByteChannel(seekable).get().use { seven ->
+                val buffer = ByteArray(BUFFER_BYTES)
+                while (currentCoroutineContext().isActive) {
+                    val entry = seven.nextEntry ?: break
+
+                    val relative = ArchivePath.safeRelativePath(entry.name)
+                    if (relative == null) {
+                        // Recorded rather than thrown: one hostile or malformed entry must
+                        // not abandon the rest of the archive.
+                        state.failure(destination, "Skipped an unsafe entry: ${entry.name}")
+                        continue
+                    }
+
+                    val target = destination.resolve(relative)
+                    if (entry.isDirectory) {
+                        source.createDirectory(target)
+                    } else {
+                        target.parent?.let { source.createDirectory(it) }
+                        source.createFile(target)
+                        source.openWrite(target).use { output ->
+                            while (true) {
+                                val read = seven.read(buffer)
+                                if (read < 0) break
+                                output.write(buffer, 0, read)
+                            }
+                        }
+                        state.advance(target)
+                    }
+                }
+                if (!currentCoroutineContext().isActive) state.cancel()
+            }
+        }
+    }
+
    private suspend fun readSingle(
        decompressed: InputStream,
        name: String,
@@ -80,12 +80,20 @@ class ArchiveFormatsTest {
        assertEquals(ArchiveFormat.XZ, ArchiveFormats.formatOf("a.xz"))
    }

+    @Test
+    fun `detects 7z, which is readable without native code`() {
+        assertEquals(ArchiveFormat.SEVEN_ZIP, ArchiveFormats.formatOf("pack.7z"))
+        assertTrue(ArchiveFormats.isExtractable("pack.7z"))
+        assertTrue(ArchiveFormats.isRecognised("pack.7z"))
+    }
+
    @Test
    fun `recognises but refuses formats that would need native code`() {
-        assertEquals(ArchiveFormat.UNSUPPORTED, ArchiveFormats.formatOf("pack.7z"))
        assertEquals(ArchiveFormat.UNSUPPORTED, ArchiveFormats.formatOf("pack.rar"))
+        assertEquals(ArchiveFormat.UNSUPPORTED, ArchiveFormats.formatOf("disc.iso"))
+        assertEquals(ArchiveFormat.UNSUPPORTED, ArchiveFormats.formatOf("setup.cab"))
        assertFalse(ArchiveFormats.isExtractable("pack.rar"))
-        assertFalse(ArchiveFormats.isExtractable("pack.7z"))
+        assertFalse(ArchiveFormats.isExtractable("disc.iso"))
        assertTrue(ArchiveFormats.isRecognised("pack.rar"))
    }

@@ -101,6 +109,7 @@ class ArchiveFormatsTest {
        assertEquals(ArchiveFormat.ZIP, ArchiveFormats.formatOf("A.ZIP"))
        assertEquals(ArchiveFormat.TAR_GZIP, ArchiveFormats.formatOf("A.TGZ"))
        assertEquals(ArchiveFormat.GZIP, ArchiveFormats.formatOf("A.GZ"))
+        assertEquals(ArchiveFormat.SEVEN_ZIP, ArchiveFormats.formatOf("A.7Z"))
    }

    @Test
@@ -3,7 +3,11 @@ package de.softwareapp_hb.filemanager.domain
 import de.softwareapp_hb.filemanager.core.StoragePath
 import de.softwareapp_hb.filemanager.domain.testutil.InMemoryStorageSource
 import kotlinx.coroutines.runBlocking
+import org.apache.commons.compress.archivers.sevenz.SevenZArchiveEntry
+import org.apache.commons.compress.archivers.sevenz.SevenZOutputFile
 import org.apache.commons.compress.compressors.gzip.GzipCompressorOutputStream
+import org.apache.commons.compress.compressors.xz.XZCompressorOutputStream
+import org.apache.commons.compress.utils.SeekableInMemoryByteChannel
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
@@ -99,11 +103,33 @@ class ArchiveEngineTest {
        assertEquals("hello gzip", storage.contentOf(out.child("notes.txt")))
    }

+    /**
+     * LZMA — and therefore `.xz` and 7z — needs Commons Compress's *optional* `xz`
+     * library, which is not pulled in transitively. Without it declared, this fails with
+     * `NoClassDefFoundError: org/tukaani/xz/FilterOptions` at runtime, on a device, with
+     * nothing warning at build time. This test is what notices.
+     */
+    @Test
+    fun `a single-file xz is unwrapped, which needs the optional xz library`() = runBlocking {
+        val storage = storage()
+        val compressed = ByteArrayOutputStream().also { bytes ->
+            XZCompressorOutputStream(bytes).use { it.write("hello xz".toByteArray()) }
+        }.toByteArray()
+
+        val archive = StoragePath.of("/src/notes.txt.xz")
+        storage.putBytes(archive, compressed)
+
+        val result = ArchiveEngine(storage).extract(archive, out)
+
+        assertTrue(result.succeeded)
+        assertEquals("hello xz", storage.contentOf(out.child("notes.txt")))
+    }
+
    @Test
    fun `an unsupported format is refused rather than guessed at`() = runBlocking {
        val storage = storage()
-        val archive = StoragePath.of("/src/pack.7z")
-        storage.addFile(archive, "not really a 7z")
+        val archive = StoragePath.of("/src/pack.rar")
+        storage.addFile(archive, "not really a rar")

        val result = ArchiveEngine(storage).extract(archive, out)

@@ -112,9 +138,81 @@ class ArchiveEngineTest {
        assertEquals(0, result.completed)
    }

+    @Test
+    fun `a 7z round-trips its files`() = runBlocking {
+        val storage = storage()
+        val archive = StoragePath.of("/src/photos.7z")
+        storage.putBytes(archive, sevenZip("b.txt" to "beta", "folder/a.txt" to "alpha"))
+
+        val result = ArchiveEngine(storage).extract(archive, out)
+
+        assertTrue(result.succeeded)
+        assertEquals("alpha", storage.contentOf(out.child("folder/a.txt")))
+        assertEquals("beta", storage.contentOf(out.child("b.txt")))
+        assertEquals(2, result.completed)
+    }
+
+    @Test
+    fun `a 7z-slip entry cannot escape the destination`() = runBlocking {
+        val storage = storage()
+        val archive = StoragePath.of("/src/crafted.7z")
+        storage.putBytes(archive, sevenZip("../evil.txt" to "pwned", "good.txt" to "fine"))
+
+        val result = ArchiveEngine(storage).extract(archive, out)
+
+        // The hostile entry is refused, and it did not land anywhere outside the folder.
+        assertFalse(storage.contains(StoragePath.of("/evil.txt")))
+        assertFalse(storage.contains(StoragePath.of("/src/evil.txt")))
+        assertEquals(1, result.failures.size)
+
+        // ...while the sane entry in the same archive is still extracted.
+        assertEquals("fine", storage.contentOf(out.child("good.txt")))
+    }
+
+    @Test
+    fun `7z is refused with a readable reason when the backend cannot seek`() = runBlocking {
+        // This is the shape of the SAF fallback, which has no seekable channel to offer.
+        val storage = storage().apply { seekableSupported = false }
+        val archive = StoragePath.of("/src/photos.7z")
+        storage.putBytes(archive, sevenZip("b.txt" to "beta"))
+
+        val result = ArchiveEngine(storage).extract(archive, out)
+
+        assertFalse(result.succeeded)
+        assertEquals(1, result.failures.size)
+        assertTrue(result.failures.single().reason.contains("seekable"))
+    }
+
    @Test
    fun `extracting something that is not there is reported`() = runBlocking {
        val result = ArchiveEngine(storage()).extract(StoragePath.of("/src/ghost.zip"), out)
        assertFalse(result.succeeded)
    }
+
+    /**
+     * A genuine 7z, built in memory rather than hand-written as bytes.
+     *
+     * Writing the fixture with the same library that reads it means it cannot drift from
+     * the format the way a byte literal would.
+     */
+    private fun sevenZip(vararg entries: Pair<String, String>): ByteArray =
+        SeekableInMemoryByteChannel().let { channel ->
+            SevenZOutputFile(channel).use { archive ->
+                for ((name, content) in entries) {
+                    val body = content.toByteArray()
+                    val entry = SevenZArchiveEntry().apply {
+                        this.name = name
+                        isDirectory = false
+                        size = body.size.toLong()
+                    }
+                    archive.putArchiveEntry(entry)
+                    archive.write(body)
+                    archive.closeArchiveEntry()
+                }
+                archive.finish()
+                // Read inside the `use`: SevenZOutputFile closes the channel it was handed,
+                // and finish() has already written the header the reader looks for.
+                channel.array().copyOf(channel.size().toInt())
+            }
+        }
 }
@@ -4,10 +4,12 @@ import de.softwareapp_hb.filemanager.core.FileEntry
 import de.softwareapp_hb.filemanager.core.StoragePath
 import de.softwareapp_hb.filemanager.data.StorageException
 import de.softwareapp_hb.filemanager.data.StorageSource
+import org.apache.commons.compress.utils.SeekableInMemoryByteChannel
 import java.io.ByteArrayInputStream
 import java.io.ByteArrayOutputStream
 import java.io.InputStream
 import java.io.OutputStream
+import java.nio.channels.SeekableByteChannel

 /**
 * A complete [StorageSource] backed by maps, for exercising the operations engine
@@ -32,6 +34,9 @@ class InMemoryStorageSource(
    /** When `false`, [rename] refuses, the way the SAF backend does across directories. */
    var renameSupported: Boolean = true

+    /** When `false`, [openSeekable] returns `null`, the way the SAF backend always does. */
+    var seekableSupported: Boolean = true
+
    /** Paths whose [openRead] throws, to exercise the per-item failure path. */
    val unreadable = mutableSetOf<StoragePath>()

@@ -100,6 +105,18 @@ class InMemoryStorageSource(
        return ByteArrayInputStream(bytes)
    }

+    /**
+     * Seekable access over the stored bytes, which is what 7z extraction needs.
+     *
+     * Every archive here already lives in memory, so seeking is free — which is why this
+     * fake can exercise 7z end to end while the real SAF backend cannot.
+     */
+    override suspend fun openSeekable(path: StoragePath): SeekableByteChannel? {
+        if (!seekableSupported) return null
+        val bytes = contents[path] ?: return null
+        return SeekableInMemoryByteChannel(bytes)
+    }
+
    /**
     * Writes are committed on [OutputStream.close], which is how the engine uses it
     * (`openWrite(...).use { ... }`), so a partial write never looks like a complete one.