diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bf2403..c8adf02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,18 @@ upload needs a **higher `versionCode`** than every previous upload. gesture that had already proved flaky; the note that this path "is still worth a manual pass" turned out to be exactly right. +- **`.tar.bz2` and `.tar.xz` did not unpack, unlike `.tar.gz`.** `ArchiveFormats.formatOf` + matched `.tar.gz`/`.tgz` as a combined format but let `.tar.bz2` and `.tar.xz` fall through to + the bare `.bz2` and `.xz` wrappers, so extracting one decompressed a single layer and left the + user holding a `.tar` to extract again — while the same archive named `.tar.gz` unpacked in one + step. The store listing advertised "extract ZIP, TAR, TAR.GZ, GZ, BZ2, XZ and 7z", which reads + as though all of them behave alike. Found by extracting one of every supported format through + the UI on a device instead of trusting the unit tests, which is also how the `.xz` crash was + found. `TAR_BZIP2` and `TAR_XZ` now sit alongside `TAR_GZIP`, detection consumes the whole tar + suffix before the bare compressor, and round-trip tests cover all four tar spellings — and the + suite had a gap there worth naming: neither plain `TAR` nor `TAR_GZIP` had a round-trip test + either, so the asymmetry had nothing to fail against. + - **`.xz` extraction would have crashed on a real device.** Commons Compress declares its LZMA/LZMA2 support — the `.xz` compressor and the 7z format — against `org.tukaani:xz` as an **optional** dependency. Optional means it is not pulled in transitively, so nothing diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/core/ArchiveFormats.kt b/app/src/main/java/de/softwareapp_hb/filemanager/core/ArchiveFormats.kt index 1a3afe8..cecfbfd 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/core/ArchiveFormats.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/core/ArchiveFormats.kt @@ -18,6 +18,8 @@ enum class ArchiveFormat { ZIP, TAR, TAR_GZIP, + TAR_BZIP2, + TAR_XZ, GZIP, BZIP2, XZ, @@ -42,7 +44,14 @@ object ArchiveFormats { fun formatOf(fileName: String): ArchiveFormat { val lower = fileName.lowercase() return when { + // The tar wrappers are matched before the bare compressors, and in the order + // `.tar.gz`, `.tar.bz2`, `.tar.xz`: a name has to lose the whole tar suffix to + // be recognised as a tar, otherwise `notes.txt.gz` would look like an archive + // of files rather than a compressed file. `.tgz`, `.tbz2` and `.txz` are the + // usual short spellings of the same three. lower.endsWith(".tar.gz") || lower.endsWith(".tgz") -> ArchiveFormat.TAR_GZIP + lower.endsWith(".tar.bz2") || lower.endsWith(".tbz2") -> ArchiveFormat.TAR_BZIP2 + lower.endsWith(".tar.xz") || lower.endsWith(".txz") -> ArchiveFormat.TAR_XZ lower.endsWith(".bz2") -> ArchiveFormat.BZIP2 lower.endsWith(".xz") -> ArchiveFormat.XZ lower.endsWith(".tar") -> ArchiveFormat.TAR @@ -67,15 +76,27 @@ object ArchiveFormats { else -> true } - /** `file.txt.gz` -> `file.txt`, so a single-file wrapper loses its own suffix. */ + /** + * The name of the thing [fileName] yields once its own wrapper is gone. + * + * A single-file wrapper loses its suffix (`file.txt.gz` -> `file.txt`); a compressed tar + * becomes the tar inside it (`data.tar.bz2` -> `data.tar`), which is the name the + * extraction folder is derived from. + */ fun innerName(fileName: String, format: ArchiveFormat): String = when (format) { - ArchiveFormat.TAR_GZIP -> fileName.removeSuffixIgnoreCase(".tgz").removeSuffixIgnoreCase(".tar.gz") + ".tar" + ArchiveFormat.TAR_GZIP -> tarInside(fileName, ".tgz", ".tar.gz") + ArchiveFormat.TAR_BZIP2 -> tarInside(fileName, ".tbz2", ".tar.bz2") + ArchiveFormat.TAR_XZ -> tarInside(fileName, ".txz", ".tar.xz") ArchiveFormat.GZIP -> fileName.removeSuffixIgnoreCase(".gz") ArchiveFormat.BZIP2 -> fileName.removeSuffixIgnoreCase(".bz2") ArchiveFormat.XZ -> fileName.removeSuffixIgnoreCase(".xz") else -> fileName } + /** `data.tbz2` -> `data.tar`, dispatching on whichever of the two spellings is present. */ + private fun tarInside(fileName: String, short: String, long: String): String = + fileName.removeSuffixIgnoreCase(short).removeSuffixIgnoreCase(long) + ".tar" + private fun String.removeSuffixIgnoreCase(suffix: String): String = if (endsWith(suffix, ignoreCase = true)) dropLast(suffix.length) else this } diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/domain/ArchiveEngine.kt b/app/src/main/java/de/softwareapp_hb/filemanager/domain/ArchiveEngine.kt index 5238465..25344cd 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/domain/ArchiveEngine.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/domain/ArchiveEngine.kt @@ -138,6 +138,18 @@ class ArchiveEngine(private val source: StorageSource) { state, ) + ArchiveFormat.TAR_BZIP2 -> readArchive( + TarArchiveInputStream(BZip2CompressorInputStream(input)), + destination, + state, + ) + + ArchiveFormat.TAR_XZ -> readArchive( + TarArchiveInputStream(XZCompressorInputStream(input)), + destination, + state, + ) + ArchiveFormat.GZIP -> readSingle( GzipCompressorInputStream(input), ArchiveFormats.innerName(archive.name, format), diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/core/ArchivePathTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/core/ArchivePathTest.kt index 9f0be91..b8f9ce0 100644 --- a/app/src/test/java/de/softwareapp_hb/filemanager/core/ArchivePathTest.kt +++ b/app/src/test/java/de/softwareapp_hb/filemanager/core/ArchivePathTest.kt @@ -120,6 +120,32 @@ class ArchiveFormatsTest { assertEquals("data.tar", ArchiveFormats.innerName("data.tgz", ArchiveFormat.TAR_GZIP)) } + /** + * A `.tar.bz2` and a `.tar.xz` used to be read as bare compressed files, so extracting one + * left the user holding a `.tar` to extract again — while the same archive as `.tar.gz` + * unpacked in one step. Driving all three through the UI on a device is what surfaced it. + */ + @Test + fun `a compressed tar is recognised as a tar, not as a bare compressed file`() { + assertEquals(ArchiveFormat.TAR_BZIP2, ArchiveFormats.formatOf("backup.tar.bz2")) + assertEquals(ArchiveFormat.TAR_BZIP2, ArchiveFormats.formatOf("backup.tbz2")) + assertEquals(ArchiveFormat.TAR_XZ, ArchiveFormats.formatOf("backup.tar.xz")) + assertEquals(ArchiveFormat.TAR_XZ, ArchiveFormats.formatOf("backup.txz")) + + // The bare spellings still mean one compressed file, not an archive of files. + assertEquals(ArchiveFormat.BZIP2, ArchiveFormats.formatOf("notes.txt.bz2")) + assertEquals(ArchiveFormat.XZ, ArchiveFormats.formatOf("notes.txt.xz")) + assertEquals(ArchiveFormat.TAR_GZIP, ArchiveFormats.formatOf("backup.tar.gz")) + } + + @Test + fun `every compressed tar names the tar inside it`() { + assertEquals("data.tar", ArchiveFormats.innerName("data.tar.bz2", ArchiveFormat.TAR_BZIP2)) + assertEquals("data.tar", ArchiveFormats.innerName("data.tbz2", ArchiveFormat.TAR_BZIP2)) + assertEquals("data.tar", ArchiveFormats.innerName("data.tar.xz", ArchiveFormat.TAR_XZ)) + assertEquals("data.tar", ArchiveFormats.innerName("data.txz", ArchiveFormat.TAR_XZ)) + } + @Test fun `only zip can be created`() { assertEquals(setOf(ArchiveFormat.ZIP), ArchiveFormats.creatable) diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/domain/ArchiveEngineTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/domain/ArchiveEngineTest.kt index 53455af..be1fcc5 100644 --- a/app/src/test/java/de/softwareapp_hb/filemanager/domain/ArchiveEngineTest.kt +++ b/app/src/test/java/de/softwareapp_hb/filemanager/domain/ArchiveEngineTest.kt @@ -5,6 +5,9 @@ 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.archivers.tar.TarArchiveEntry +import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream +import org.apache.commons.compress.compressors.bzip2.BZip2CompressorOutputStream import org.apache.commons.compress.compressors.gzip.GzipCompressorOutputStream import org.apache.commons.compress.compressors.xz.XZCompressorOutputStream import org.apache.commons.compress.utils.SeekableInMemoryByteChannel @@ -13,6 +16,7 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test import java.io.ByteArrayOutputStream +import java.io.OutputStream import java.util.zip.ZipEntry import java.util.zip.ZipOutputStream @@ -125,6 +129,81 @@ class ArchiveEngineTest { assertEquals("hello xz", storage.contentOf(out.child("notes.txt"))) } + /** + * A tar holding one file, wrapped by [wrap] — the identity for a plain tar. + * + * Written with the same library that reads it, like the 7z fixture below, so it cannot + * drift from the format the way a byte literal would. + */ + private fun tarOf( + name: String, + content: String, + wrap: (OutputStream) -> OutputStream, + ): ByteArray = ByteArrayOutputStream().let { bytes -> + wrap(bytes).use { wrapped -> + TarArchiveOutputStream(wrapped).use { tar -> + val body = content.toByteArray() + tar.putArchiveEntry(TarArchiveEntry(name).apply { size = body.size.toLong() }) + tar.write(body) + tar.closeArchiveEntry() + } + } + bytes.toByteArray() + } + + @Test + fun `a plain tar round-trips its files`() = runBlocking { + val storage = storage() + val archive = StoragePath.of("/src/backup.tar") + storage.putBytes(archive, tarOf("a.txt", "plain tar") { it }) + + val result = ArchiveEngine(storage).extract(archive, out) + + assertTrue(result.succeeded) + assertEquals("plain tar", storage.contentOf(out.child("a.txt"))) + } + + @Test + fun `a gzip tar round-trips its files`() = runBlocking { + val storage = storage() + val archive = StoragePath.of("/src/backup.tar.gz") + storage.putBytes(archive, tarOf("a.txt", "gzipped tar") { GzipCompressorOutputStream(it) }) + + val result = ArchiveEngine(storage).extract(archive, out) + + assertTrue(result.succeeded) + assertEquals("gzipped tar", storage.contentOf(out.child("a.txt"))) + } + + /** + * `.tar.bz2` and `.tar.xz` used to be read as *bare* compressed files, so extracting one + * left a `.tar` rather than the files inside it, while `.tar.gz` unpacked in one step. + * Driving all three through the UI on a device is what surfaced the asymmetry. + */ + @Test + fun `a bzip2 tar round-trips its files`() = runBlocking { + val storage = storage() + val archive = StoragePath.of("/src/backup.tar.bz2") + storage.putBytes(archive, tarOf("a.txt", "bzipped tar") { BZip2CompressorOutputStream(it) }) + + val result = ArchiveEngine(storage).extract(archive, out) + + assertTrue(result.succeeded) + assertEquals("bzipped tar", storage.contentOf(out.child("a.txt"))) + } + + @Test + fun `an xz tar round-trips its files`() = runBlocking { + val storage = storage() + val archive = StoragePath.of("/src/backup.tar.xz") + storage.putBytes(archive, tarOf("a.txt", "xz tar") { XZCompressorOutputStream(it) }) + + val result = ArchiveEngine(storage).extract(archive, out) + + assertTrue(result.succeeded) + assertEquals("xz tar", storage.contentOf(out.child("a.txt"))) + } + @Test fun `an unsupported format is refused rather than guessed at`() = runBlocking { val storage = storage() diff --git a/docs/EDGE_CASE_TESTING.md b/docs/EDGE_CASE_TESTING.md index 85e4416..5d8509c 100644 --- a/docs/EDGE_CASE_TESTING.md +++ b/docs/EDGE_CASE_TESTING.md @@ -66,6 +66,7 @@ track to report back. | The progress notification | **Fixed, then verified.** After the fix, a real 220 MB copy posts notification `1001` on channel `file-operations` with `ONGOING_EVENT\|NO_CLEAR\|FOREGROUND_SERVICE` and `category=progress`, then removes it and stops the service when the copy finishes. Before the fix the app's notification importance was `NONE`. | | Copy to completion | The staged copy landed at `/sdcard/bigfile.bin`, byte-count identical, via the overflow menu's "Paste here". | | Compress and extract from the UI | **Found a bug, fixed.** Long-pressing a folder and choosing "Compress to ZIP" produced a `tocompress.zip` that the host's `unzip` read back with the right relative paths and contents. Selecting that `.zip` then exposed the selection bar clipping its own actions — see below. | +| One archive of **every** supported format | **Found a bug, fixed.** A real `.zip`, `.7z`, `.tar`, `.tar.gz`, `.tar.bz2`, `.txt.xz`, `.txt.gz` and `.txt.bz2` were built on the host and all eight extracted through the UI. Each landed with the right entry names and byte-for-byte content — except `.tar.bz2`, which produced a bare `.tar`, recorded below. | | Long-press from adb | `input swipe x y x y 700` on a single point *did* open the selection bar and reach Copy. The same gesture has been flaky on the MIUI phone, so it still deserves a manual pass. | ### The bugs this session found @@ -100,6 +101,16 @@ everything at once, which is honest but not ideal — moving the two least-used Clear selection) into the overflow menu would be the better fix, and is left as deliberate follow-up rather than bundled in here. +**`.tar.bz2` and `.tar.xz` only half-extracted.** `ArchiveFormats.formatOf` tested `.tar.gz`/`.tgz` +first, as a combined format, but then matched `.bz2` and `.xz` *before* it ever considered the +`.tar` in the middle of the name — so a `.tar.gz` unpacked to its files while a `.tar.bz2` was +read as one compressed file and yielded a `.tar`. Driving the sweep made it exact: extracting +`z4.tar.bz2` left `z4/z4.tar`, raw `ustar` headers and all, beside `z3.tar.gz`'s fully unpacked +`z3/a.txt` and `z3/sub/b.txt`. `TAR_BZIP2` and `TAR_XZ` now exist alongside `TAR_GZIP`, and +detection consumes the whole tar suffix before the bare compressor. The suite could not have +caught this: only ZIP, GZIP, XZ and 7z had round-trip tests, so neither plain `TAR` nor +`TAR_GZIP` did, and there was nothing for the asymmetry to contradict. All four have one now. + The lesson is the one the phone already taught twice: a green test suite says the code does what the tests describe, not that the app does anything on a device. Both times the gap was a user-visible promise the code no longer kept — once in a dialog, once in a permission. @@ -118,8 +129,10 @@ user-visible promise the code no longer kept — once in a dialog, once in a per - **The foreground-service notification** during a long copy. **Now observed** on the API 35 AVD (see above), including its teardown — so what is left unverified is a *physical* device, where the system may defer or kill a background `dataSync` service on its own schedule. -- **Archive creation/extraction against real files.** Now driven end-to-end on the API 35 AVD: a - folder of three files compressed to a `.zip` that the host's `unzip` read back with the right - relative paths and contents, and that `.zip` then selected for extraction. What is still - untested is the exotic end of the format list — 7z, `.xz`, the tar wrappers — against real - files rather than the fixtures the JVM tests build. +- **Archive creation/extraction against real files.** Driven end-to-end on the API 35 AVD, one + archive of every supported format: `ZIP`, `7z`, `TAR`, `TAR.GZ`, `TAR.BZ2`, `TAR.XZ`, `GZ`, + `BZ2` and `XZ` all extracted through the UI with the right entry names and byte-for-byte + contents, and a folder compressed to a `.zip` that the host's `unzip` read back. That sweep is + what found the `.tar.bz2` asymmetry fixed above. Still untested: a *hostile* archive on a + device — the zip-slip and 7z-slip cases are JVM-only — and RAR/CAB/ISO/WIM, which the app + refuses by design. diff --git a/docs/PLAY_RELEASE_CHECKLIST.md b/docs/PLAY_RELEASE_CHECKLIST.md index 7abe480..f7f2c15 100644 --- a/docs/PLAY_RELEASE_CHECKLIST.md +++ b/docs/PLAY_RELEASE_CHECKLIST.md @@ -42,20 +42,23 @@ rest are things you must do in the Play Console or with your own accounts. - [x] **[code] re-verified 2026-10-15.** `bundleRelease` and `assembleRelease` both succeed with R8 minification and resource shrinking on. - - `app-release.aab` is **4 837 465 bytes** - (`sha256 9c2b3a007784297511f29b107a3be40e648709d7702315c53b93ef18f5979070`). - - `app-release.apk` is **2 155 455 bytes**, signed by the upload key above. + - `app-release.aab` is **4 838 235 bytes** + (`sha256 e1d23130b307345b1147e5cf54413d42ed53a1693e0682464cd1b64552e0ea78`). + - `app-release.apk` is **2 155 455 bytes** + (`sha256 37fb46cb3ef4908b73a9147442ff3f93905a893b8ddd27abb9e02610139a8cee`), + signed by the upload key above. - Both report `versionCode=1 versionName=1.0.0 targetSdk=36` and exactly the permission set in step 4 — **no `INTERNET`**, so "nothing leaves the device" holds in the shipped artefact and not just in the source manifest. - The figures barely moved again, but the permission set did: the four `READ_MEDIA_*` declarations are gone, removed once the Android 15 pass showed the app never - requested them and never read through `MediaStore` (see below). Four manifest lines - are worth only a few hundred bytes, hence 4 837 465 rather than a visible drop; the - first measurement was 4 682 814 / 2 081 955, before the LZMA library behind `.xz` - and 7z. The permission set has only ever shrunk: the share target, the file picker, - the trash index and the archive work all arrived without a single new permission, - which is the point of having designed them that way. + requested them and never read through `MediaStore` (see below), together with the + archive fix for `.tar.bz2`/`.tar.xz`. Small source changes barely register in a + bundle dominated by libraries, hence 4 838 235 rather than a visible drop; the first + measurement was 4 682 814 / 2 081 955, before the LZMA library behind `.xz` and 7z. + The permission set has only ever shrunk: the share target, the file picker, the + trash index and the archive work all arrived without a single new permission, which + is the point of having designed them that way. - ⚠️ **Re-measure after any further change.** These numbers are a record of a verified artefact, not a constant — a stale hash here is worse than no hash, because it looks checked. diff --git a/docs/STORE_LISTING.md b/docs/STORE_LISTING.md index 0e7914b..65ca869 100644 --- a/docs/STORE_LISTING.md +++ b/docs/STORE_LISTING.md @@ -114,7 +114,7 @@ FIND OUT WHERE THE SPACE WENT • Spot the folder that is actually eating your storage, then act on it ARCHIVES -• Create ZIP archives, and extract ZIP, TAR, TAR.GZ, GZ, BZ2, XZ and 7z +• Create ZIP archives, and extract ZIP, TAR, TAR.GZ, TAR.BZ2, TAR.XZ, GZ, BZ2, XZ and 7z • RAR is not supported — it needs native code this app does not ship, and it says so rather than failing with a strange error