Correctly handle paths containing whitespace character (#248)

* Correctly handle paths containing whitespace character

* Use system path separator to make things work with JS on Windows

* Trim trailing path separators on Path creation to correctly handle "."'s parent

* Move Windows-specific test to a common source set to reuse it for JVM and JS

* Fixed relative path detection on Windows, split parent path test to platform specific tests as NodeJS handles UNC path differently

Fixes #246

core/common/src/files/FileSystem.kt

@@ -181,3 +181,8 @@ public class FileMetadata(
  * Signals an I/O operation's failure due to a missing file or directory.
  */
 public expect class FileNotFoundException(message: String?) : IOException
+
+internal const val WindowsPathSeparator: Char = '\\'
+internal const val UnixPathSeparator: Char = '/'
+
+internal expect val isWindows: Boolean

core/common/src/files/Paths.kt

@@ -128,3 +128,48 @@ public fun Path.source(): Source = SystemFileSystem.source(this).buffered()
 )
 @JvmName("sinkDeprecated")
 public fun Path.sink(): Sink = SystemFileSystem.sink(this).buffered()
+
+internal fun removeTrailingSeparators(path: String, /* only for testing */ isWindows_: Boolean = isWindows): String {
+    if (isWindows_) {
+        // don't trim the path separator right after the drive name
+        val limit = if (path.length > 1) {
+            if (path[1] == ':') {
+                3
+            } else if (isUnc(path)) {
+                2
+            } else {
+                1
+            }
+        } else {
+            1
+        }
+        return removeTrailingSeparatorsWindows(limit, path)
+    }
+    return removeTrailingSeparatorsUnix(path)
+}
+
+private fun isUnc(path: String): Boolean {
+    if (path.length < 2) return false
+    if (path.startsWith("$WindowsPathSeparator$WindowsPathSeparator")) return true
+    if (path.startsWith("$UnixPathSeparator$UnixPathSeparator")) return true
+    return false
+}
+
+private fun removeTrailingSeparatorsUnix(path: String): String {
+    var idx = path.length
+    while (idx > 1 && path[idx - 1] == UnixPathSeparator) {
+        idx--
+    }
+    return path.substring(0, idx)
+}
+
+private fun removeTrailingSeparatorsWindows(suffixLength: Int, path: String): String {
+    require(suffixLength >= 1)
+    var idx = path.length
+    while (idx > suffixLength) {
+        val c = path[idx - 1]
+        if (c != WindowsPathSeparator && c != UnixPathSeparator) break
+        idx--
+    }
+    return path.substring(0, idx)
+}

core/common/test/files/SmokeFileTest.kt

@@ -163,6 +163,14 @@ class SmokeFileTest {
         assertFailsWith<IOException> { SystemFileSystem.createDirectories(p2, false) }
     }
 
+    @Test
+    fun trailingSeparatorsTrimming() {
+        assertEquals(Path(".").toString(), Path(".///").toString())
+        assertEquals(Path("/").toString(), Path("/").toString())
+        assertEquals(Path("/..").toString(), Path("/../").toString())
+        assertEquals(Path("/a/b/c").toString(), Path("/a/b/c").toString())
+    }
+
     @Test
     fun pathParent() {
         val p = Path(SystemPathSeparator.toString(), "a", "b", "c")
@@ -182,6 +190,15 @@ class SmokeFileTest {
         assertNull(Path(SystemPathSeparator.toString()).parent)
 
         assertEquals("..", Path("..${SystemPathSeparator}..").parent?.toString())
+
+        assertEquals(" ", Path(SystemFileSystem.resolve(Path(".")), " ", "ws").parent?.name)
+        assertEquals(" ", Path(" $SystemPathSeparator.").parent?.name)
+        assertNull(Path(" ").parent)
+        assertNull(Path(" /").parent)
+
+        assertNull(Path("path////").parent)
+        assertEquals(Path("."), Path("./child").parent)
+        assertNull(Path("./").parent)
     }
 
     @Test
@@ -211,6 +228,8 @@ class SmokeFileTest {
         assertEquals("..", Path("..").name)
         assertEquals("hello.txt", Path("base", "hello.txt").name)
         assertEquals("dir", Path("dir${SystemPathSeparator}").name)
+        assertEquals(" ", Path(" ").name)
+        assertEquals("  ", Path(" /  ").name)
     }
 
     @Test

core/mingw/test/files/SmokeFileTestWindows.kt → core/common/test/files/SmokeFileTestWindows.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
  * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
  */
 
@@ -10,21 +10,36 @@ import kotlin.test.*
 class SmokeFileTestWindows {
     @Test
     fun isAbsolute() {
+        if (!isWindows) return
+        assertFalse(Path("C:").isAbsolute)
         assertTrue(Path("C:\\").isAbsolute)
         assertTrue(Path("C:/").isAbsolute)
         assertTrue(Path("C:/../").isAbsolute)
-        // Well, it's a relative path, but Win32's PathIsRelative don't think so.
-        assertTrue(Path("C:file").isAbsolute)
+        assertFalse(Path("C:file").isAbsolute)
         assertFalse(Path("bla\\bla\\bla").isAbsolute)
         assertTrue(Path("\\\\server\\share").isAbsolute)
     }
 
     @Test
     fun getParent() {
+        if (!isWindows) return
+        assertNull(Path("C:").parent)
         assertNull(Path("C:\\").parent)
         assertNull(Path("a\\b").parent?.parent)
-        assertEquals(Path("\\\\server"), Path("\\\\server\\share").parent)
         assertEquals(Path("C:\\"), Path("C:\\Program Files").parent)
         assertEquals(Path("C:\\Program Files"), Path("C:\\Program Files/Java").parent)
     }
+
+    @Test
+    fun trailingSeparatorsTrimming() {
+        if (!isWindows) return
+        assertEquals(".", Path(".\\").toString())
+        assertEquals("C:\\", Path("C:\\").toString())
+        assertEquals("C:\\", Path("C:\\\\").toString())
+        assertEquals("\\\\", Path("\\\\").toString())
+        assertEquals(".\\a", Path(".\\a\\//\\//\\\\////").toString())
+
+        // this path could be transformed to use canonical separator on JVM
+        assertEquals(Path("//").toString(), Path("//").toString())
+    }
 }

core/common/test/files/UtilsTest.kt

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
+ */
+
+package kotlinx.io.files
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class UtilsTest {
+    private fun removeTrailingSeparatorsU(path: String): String = removeTrailingSeparators(path, false)
+    private fun removeTrailingSeparatorsW(path: String): String = removeTrailingSeparators(path, true)
+
+
+    @Test
+    fun testPathTrimmingUnix() {
+        assertEquals("", removeTrailingSeparatorsU(""))
+        assertEquals("/", removeTrailingSeparatorsU("/"))
+        assertEquals("/", removeTrailingSeparatorsU("//"))
+        assertEquals("// ", removeTrailingSeparatorsU("// "))
+        assertEquals("/", removeTrailingSeparatorsU("///"))
+        assertEquals("@@@", removeTrailingSeparatorsU("@@@"))
+        assertEquals("/a", removeTrailingSeparatorsU("/a/"))
+        assertEquals("\\", removeTrailingSeparatorsU("\\"))
+        assertEquals("\\\\", removeTrailingSeparatorsU("\\\\"))
+        assertEquals("\\a\\", removeTrailingSeparatorsU("\\a\\"))
+        assertEquals("/\\/ ", removeTrailingSeparatorsU("/\\/ "))
+
+        assertEquals("a//\\////\\\\//\\/\\", removeTrailingSeparatorsU("a//\\////\\\\//\\/\\"))
+        assertEquals("C:\\", removeTrailingSeparatorsU("C:\\"))
+        assertEquals("C:\\/\\", removeTrailingSeparatorsU("C:\\/\\"))
+    }
+
+    @Test
+    fun testPathTrimmingWindows() {
+        assertEquals("", removeTrailingSeparatorsW(""))
+        assertEquals("/", removeTrailingSeparatorsW("/"))
+        assertEquals("//", removeTrailingSeparatorsW("//"))
+        assertEquals("// ", removeTrailingSeparatorsW("// "))
+        assertEquals("//", removeTrailingSeparatorsW("///"))
+        assertEquals("@@@", removeTrailingSeparatorsW("@@@"))
+        assertEquals("/a", removeTrailingSeparatorsW("/a/"))
+        assertEquals("\\", removeTrailingSeparatorsW("\\"))
+        assertEquals("\\\\", removeTrailingSeparatorsW("\\\\"))
+        assertEquals("\\a", removeTrailingSeparatorsW("\\a\\"))
+        assertEquals("\\a", removeTrailingSeparatorsW("\\a\\\\"))
+        assertEquals("/\\/ ", removeTrailingSeparatorsW("/\\/ "))
+
+        assertEquals("a", removeTrailingSeparatorsW("a//\\////\\\\//\\/\\"))
+        assertEquals("C:a", removeTrailingSeparatorsW("C:a//\\////\\\\//\\/\\"))
+        assertEquals("C:\\", removeTrailingSeparatorsW("C:\\"))
+        assertEquals("C:\\", removeTrailingSeparatorsW("C:\\/\\"))
+    }
+}

core/js/src/files/FileSystemJs.kt

@@ -5,27 +5,7 @@
 
 package kotlinx.io.files
 
-import kotlinx.io.IOException
-import kotlinx.io.RawSink
-import kotlinx.io.RawSource
-
-internal val fs: dynamic
-    get(): dynamic {
-        return try {
-            js("require('fs')")
-        } catch (t: Throwable) {
-            null
-        }
-    }
-
-internal val os: dynamic
-    get(): dynamic {
-        return try {
-            js("require('os')")
-        } catch (t: Throwable) {
-            null
-        }
-    }
+import kotlinx.io.*
 
 public actual val SystemFileSystem: FileSystem = object : SystemFileSystemImpl() {
     override fun exists(path: Path): Boolean {
@@ -132,3 +112,5 @@ public actual val SystemTemporaryDirectory: Path
 public actual open class FileNotFoundException actual constructor(
     message: String?,
 ) : IOException(message)
+
+internal actual val isWindows = os.platform() == "win32"

core/js/src/files/PathsJs.kt

@@ -7,38 +7,26 @@ package kotlinx.io.files
 
 import kotlinx.io.*
 
-internal val buffer: dynamic
-    get(): dynamic {
-        return try {
-            js("require('buffer')")
-        } catch (t: Throwable) {
-            null
-        }
-    }
-
-private val pathLib: dynamic
-    get(): dynamic {
-        return try {
-            js("require('path')")
-        } catch (t: Throwable) {
-            null
-        }
-    }
-
 public actual class Path internal constructor(
-    internal val path: String,
+    rawPath: String,
     @Suppress("UNUSED_PARAMETER") any: Any?
 ) {
+    internal val path: String = removeTrailingSeparators(rawPath)
+
     public actual val parent: Path?
         get() {
             check(pathLib !== null) { "Path module not found" }
-            when {
-                path.isBlank() -> return null
-                !path.contains(SystemPathSeparator) -> return null
+            if (path.isEmpty()) return null
+            if (isWindows) {
+                if (!path.contains(UnixPathSeparator) && !path.contains(WindowsPathSeparator)) {
+                    return null
+                }
+            } else if (!path.contains(SystemPathSeparator)) {
+                return null
             }
             val p = pathLib.dirname(path) as String?
             return when {
-                p.isNullOrBlank() -> null
+                p.isNullOrEmpty() -> null
                 p == path -> null
                 else -> Path(p)
             }
@@ -54,11 +42,11 @@ public actual class Path internal constructor(
         get() {
             check(pathLib !== null) { "Path module not found" }
             when {
-                path.isBlank() -> return ""
+                path.isNullOrEmpty() -> return ""
             }
             val p = pathLib.basename(path) as String?
             return when {
-                p.isNullOrBlank() -> ""
+                p.isNullOrEmpty() -> ""
                 else -> p
             }
         }

core/js/src/imports.kt

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
+ */
+
+package kotlinx.io
+
+internal val os: dynamic
+    get(): dynamic {
+        return try {
+            js("require('os')")
+        } catch (t: Throwable) {
+            null
+        }
+    }
+
+internal val fs: dynamic
+    get(): dynamic {
+        return try {
+            js("require('fs')")
+        } catch (t: Throwable) {
+            null
+        }
+    }
+
+internal val buffer: dynamic
+    get(): dynamic {
+        return try {
+            js("require('buffer')")
+        } catch (t: Throwable) {
+            null
+        }
+    }
+
+internal val pathLib: dynamic
+    get(): dynamic {
+        return try {
+            js("require('path')")
+        } catch (t: Throwable) {
+            null
+        }
+    }

core/js/test/files/SmokeFileTestWindowsJS.kt

@@ -0,0 +1,20 @@
+/*
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
+ */
+
+package kotlinx.io.files
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+
+class SmokeFileTestWindowJS  {
+    @Test
+    fun uncParent() {
+        if (!isWindows) return
+        // NodeJS, correctly, assume that it's a root path, that's where behavior diverge
+        assertNull(Path("\\\\server\\share").parent)
+        assertEquals(Path("\\\\server\\share"), Path("\\\\server\\share\\dir").parent)
+    }
+}

core/jvm/src/files/FileSystemJvm.kt

@@ -102,3 +102,5 @@ public actual val SystemFileSystem: FileSystem = object : SystemFileSystemImpl()
 public actual val SystemTemporaryDirectory: Path = Path(System.getProperty("java.io.tmpdir"))
 
 public actual typealias FileNotFoundException = java.io.FileNotFoundException
+
+internal actual val isWindows: Boolean = System.getProperty("os.name")?.startsWith("Windows") ?: false

core/jvm/test/files/SmokeFileTestWindowsJVM.kt

@@ -0,0 +1,18 @@
+/*
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
+ */
+
+package kotlinx.io.files
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class SmokeFileTestWindowsJVM {
+    @Test
+    fun uncParent() {
+        if (!isWindows) return
+        assertEquals(Path("\\\\server"), Path("\\\\server\\share").parent)
+        assertEquals(Path("\\\\server\\share"), Path("\\\\server\\share\\dir").parent)
+    }
+}