skolson / KmpIO

Kotlin multi-platform simple File I/O library
Other
43 stars 3 forks source link

ZIP file creation does not work #13

Open mipastgt opened 2 months ago

mipastgt commented 2 months ago

The following test should create a ZIP file of a directory but it just fails with an internal error instead.

import com.oldguy.common.io.File
import com.oldguy.common.io.FileMode
import com.oldguy.common.io.ZipFile
import kotlinx.coroutines.runBlocking
import okio.FileSystem
import okio.Path.Companion.toPath
import okio.SYSTEM
import kotlin.test.Test
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class ZipDirTest {

    val SystemFileSystem = FileSystem.SYSTEM

    val srcDirPath = "data".toPath()
    val outputDirPath = "testresults".toPath()
    val zipFilePath = outputDirPath.resolve("TestFile.zip")
    val zipFileParentPath = zipFilePath.parent!!

    @Test
    fun test() {
        assertTrue(SystemFileSystem.exists(srcDirPath), "'$srcDirPath' doesn't exist.")
        assertTrue(SystemFileSystem.exists(outputDirPath), "'$outputDirPath' doesn't exist.")
        assertTrue(SystemFileSystem.exists(zipFileParentPath), "'$zipFileParentPath' doesn't exist.")

        if (SystemFileSystem.exists(zipFilePath)) SystemFileSystem.delete(zipFilePath)
        assertFalse(SystemFileSystem.exists(zipFilePath), "'$zipFilePath' already exists before test.")

        runBlocking {
            val srcDir = File(srcDirPath.toString())
            val zipFile = File(zipFilePath.toString())
            ZipFile(zipFile, FileMode.Write).use {
                it.isZip64 = true
                it.zipDirectory(srcDir, shallow = false)
            }
        }

        assertTrue(SystemFileSystem.exists(zipFilePath), "'$zipFilePath' doesn't exist after test.")
    }

}

The error is

java.lang.IllegalArgumentException
    at java.base/sun.nio.ch.FileChannelImpl.position(FileChannelImpl.java:358)
    at com.oldguy.common.io.RawFile.setPosition-VKZWuLQ(Files.kt:176)
    at com.oldguy.common.io.ZipFile.saveDirectory(ZipFile.kt:772)
    at com.oldguy.common.io.ZipFile.finish(ZipFile.kt:376)
    at com.oldguy.common.io.ZipFile.close(ZipFile.kt:369)
    at com.oldguy.common.io.ZipFile.use(ZipFile.kt:514)
    at de.mpmediasoft.core.files.ZipDirTest$test$1.invokeSuspend(ZipDirTest.kt:35)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
    at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:277)
    at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:95)
    at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:69)
    at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
    at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:48)
    at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
    at de.mpmediasoft.core.files.ZipDirTest.test(ZipDirTest.kt:32)
...

I have tested this on a Mac and the test fails for desktop, android and iOS.

mipastgt commented 2 months ago

Further investigations revealed that the test only works if shallow = true and the source directory also does not contain any sub-directories and isZip64 = false. In all other cases the test fails. Zipping a directory recursively is thus not possible.

mipastgt commented 2 months ago

The code has several problems which I tried to fix in file ZipFile.kt in the common code.

  1. You tried to write a directory as a file which cannot work.
  2. You did not set the entry names to their correct path relative to the ZIP root in order to preserve the files hierarchy.
  3. There was no option to specify whether the parent folder of the ZIP content should be included in the hierarchy or not.
  4. You did not replace the Windows file separators with a '/' to make the ZIP file cross-platform.

With these changes the code works now for me on the Java VM and if I set isZip64 to false. On iOS I am getting strange memory deallocation errors and the resulting ZIP file is corrupt. Maybe you make something from this input.

I did not create a pull request because the whole thing still does not work and I have also not tested the other platforms.

    suspend fun zipDirectory(directory: File,
                             shallow: Boolean = false,
                             withParent: Boolean = false,
                             filter: ((pathName: String) -> Boolean)? = null)
    override suspend fun zipDirectory(
        directory: File,
        shallow: Boolean,
        withParent: Boolean,
        filter: ((pathName: String) -> Boolean)?
    ) {
        if (!directory.isDirectory)
            throw IllegalArgumentException("Path ${file.file.path} is not a directory")
        val directoryPath = if (withParent) {
            directory.fullPath.dropLast(directory.name.length + 1)
        } else {
            directory.fullPath
        }
        val parentPath = if (directoryPath.endsWith(File.pathSeparator))
            directoryPath
        else
            directoryPath + File.pathSeparator
        zipOneDirectory(directory, shallow, parentPath, filter)
    }
    private suspend fun zipOneDirectory(directory: File,
                                        shallow: Boolean,
                                        parentPath: String,
                                        filter: ((pathName: String) -> Boolean)?)
    {
        directory.listFiles.forEach { path ->
            val name = (if (path.isDirectory && !path.fullPath.endsWith(File.pathSeparator))
                path.fullPath + File.pathSeparator
            else
                path.fullPath).replace(parentPath, "")
            if (filter?.invoke(name) != false) {
                if (path.isDirectory) {
                    if (!shallow) zipOneDirectory(path, shallow, parentPath, filter)
                } else {
                    zipFile(path, path.fullPath.replace(parentPath, "").replace('\\','/'))
                }
            }
        }
    }