Skip to content

Commit 80a1cbc

Browse files
fzhinkinfzhinkin
authored
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
1 parent 78d0fb1 commit 80a1cbc

File tree

16 files changed

+277
-58
lines changed

16 files changed

+277
-58
lines changed

‎core/common/src/files/FileSystem.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,3 +181,8 @@ public class FileMetadata(
181181
* Signals an I/O operation's failure due to a missing file or directory.
182182
*/
183183
public expect class FileNotFoundException(message: String?) : IOException
184+
185+
internal const val WindowsPathSeparator: Char = '\\'
186+
internal const val UnixPathSeparator: Char = '/'
187+
188+
internal expect val isWindows: Boolean

‎core/common/src/files/Paths.kt‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,48 @@ public fun Path.source(): Source = SystemFileSystem.source(this).buffered()
128128
)
129129
@JvmName("sinkDeprecated")
130130
public fun Path.sink(): Sink = SystemFileSystem.sink(this).buffered()
131+
132+
internal fun removeTrailingSeparators(path: String, /* only for testing */ isWindows_: Boolean = isWindows): String {
133+
if (isWindows_) {
134+
// don't trim the path separator right after the drive name
135+
val limit = if (path.length > 1) {
136+
if (path[1] == ':') {
137+
3
138+
} else if (isUnc(path)) {
139+
2
140+
} else {
141+
1
142+
}
143+
} else {
144+
1
145+
}
146+
return removeTrailingSeparatorsWindows(limit, path)
147+
}
148+
return removeTrailingSeparatorsUnix(path)
149+
}
150+
151+
private fun isUnc(path: String): Boolean {
152+
if (path.length < 2) return false
153+
if (path.startsWith("$WindowsPathSeparator$WindowsPathSeparator")) return true
154+
if (path.startsWith("$UnixPathSeparator$UnixPathSeparator")) return true
155+
return false
156+
}
157+
158+
private fun removeTrailingSeparatorsUnix(path: String): String {
159+
var idx = path.length
160+
while (idx > 1 && path[idx - 1] == UnixPathSeparator) {
161+
idx--
162+
}
163+
return path.substring(0, idx)
164+
}
165+
166+
private fun removeTrailingSeparatorsWindows(suffixLength: Int, path: String): String {
167+
require(suffixLength >= 1)
168+
var idx = path.length
169+
while (idx > suffixLength) {
170+
val c = path[idx - 1]
171+
if (c != WindowsPathSeparator && c != UnixPathSeparator) break
172+
idx--
173+
}
174+
return path.substring(0, idx)
175+
}

‎core/common/test/files/SmokeFileTest.kt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,14 @@ class SmokeFileTest {
163163
assertFailsWith<IOException> { SystemFileSystem.createDirectories(p2, false) }
164164
}
165165

166+
@Test
167+
fun trailingSeparatorsTrimming() {
168+
assertEquals(Path(".").toString(), Path(".///").toString())
169+
assertEquals(Path("/").toString(), Path("/").toString())
170+
assertEquals(Path("/..").toString(), Path("/../").toString())
171+
assertEquals(Path("/a/b/c").toString(), Path("/a/b/c").toString())
172+
}
173+
166174
@Test
167175
fun pathParent() {
168176
val p = Path(SystemPathSeparator.toString(), "a", "b", "c")
@@ -182,6 +190,15 @@ class SmokeFileTest {
182190
assertNull(Path(SystemPathSeparator.toString()).parent)
183191

184192
assertEquals("..", Path("..${SystemPathSeparator}..").parent?.toString())
193+
194+
assertEquals(" ", Path(SystemFileSystem.resolve(Path(".")), " ", "ws").parent?.name)
195+
assertEquals(" ", Path(" $SystemPathSeparator.").parent?.name)
196+
assertNull(Path(" ").parent)
197+
assertNull(Path(" /").parent)
198+
199+
assertNull(Path("path////").parent)
200+
assertEquals(Path("."), Path("./child").parent)
201+
assertNull(Path("./").parent)
185202
}
186203

187204
@Test
@@ -211,6 +228,8 @@ class SmokeFileTest {
211228
assertEquals("..", Path("..").name)
212229
assertEquals("hello.txt", Path("base", "hello.txt").name)
213230
assertEquals("dir", Path("dir${SystemPathSeparator}").name)
231+
assertEquals(" ", Path(" ").name)
232+
assertEquals(" ", Path(" / ").name)
214233
}
215234

216235
@Test

‎core/mingw/test/files/SmokeFileTestWindows.kt‎ renamed to ‎core/common/test/files/SmokeFileTestWindows.kt‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
2+
* Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
33
* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
44
*/
55

@@ -10,21 +10,36 @@ import kotlin.test.*
1010
class SmokeFileTestWindows {
1111
@Test
1212
fun isAbsolute() {
13+
if (!isWindows) return
14+
assertFalse(Path("C:").isAbsolute)
1315
assertTrue(Path("C:\\").isAbsolute)
1416
assertTrue(Path("C:/").isAbsolute)
1517
assertTrue(Path("C:/../").isAbsolute)
16-
// Well, it's a relative path, but Win32's PathIsRelative don't think so.
17-
assertTrue(Path("C:file").isAbsolute)
18+
assertFalse(Path("C:file").isAbsolute)
1819
assertFalse(Path("bla\\bla\\bla").isAbsolute)
1920
assertTrue(Path("\\\\server\\share").isAbsolute)
2021
}
2122

2223
@Test
2324
fun getParent() {
25+
if (!isWindows) return
26+
assertNull(Path("C:").parent)
2427
assertNull(Path("C:\\").parent)
2528
assertNull(Path("a\\b").parent?.parent)
26-
assertEquals(Path("\\\\server"), Path("\\\\server\\share").parent)
2729
assertEquals(Path("C:\\"), Path("C:\\Program Files").parent)
2830
assertEquals(Path("C:\\Program Files"), Path("C:\\Program Files/Java").parent)
2931
}
32+
33+
@Test
34+
fun trailingSeparatorsTrimming() {
35+
if (!isWindows) return
36+
assertEquals(".", Path(".\\").toString())
37+
assertEquals("C:\\", Path("C:\\").toString())
38+
assertEquals("C:\\", Path("C:\\\\").toString())
39+
assertEquals("\\\\", Path("\\\\").toString())
40+
assertEquals(".\\a", Path(".\\a\\//\\//\\\\////").toString())
41+
42+
// this path could be transformed to use canonical separator on JVM
43+
assertEquals(Path("//").toString(), Path("//").toString())
44+
}
3045
}

‎core/common/test/files/UtilsTest.kt‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
3+
* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
4+
*/
5+
6+
package kotlinx.io.files
7+
8+
import kotlin.test.Test
9+
import kotlin.test.assertEquals
10+
11+
class UtilsTest {
12+
private fun removeTrailingSeparatorsU(path: String): String = removeTrailingSeparators(path, false)
13+
private fun removeTrailingSeparatorsW(path: String): String = removeTrailingSeparators(path, true)
14+
15+
16+
@Test
17+
fun testPathTrimmingUnix() {
18+
assertEquals("", removeTrailingSeparatorsU(""))
19+
assertEquals("/", removeTrailingSeparatorsU("/"))
20+
assertEquals("/", removeTrailingSeparatorsU("//"))
21+
assertEquals("// ", removeTrailingSeparatorsU("// "))
22+
assertEquals("/", removeTrailingSeparatorsU("///"))
23+
assertEquals("@@@", removeTrailingSeparatorsU("@@@"))
24+
assertEquals("/a", removeTrailingSeparatorsU("/a/"))
25+
assertEquals("\\", removeTrailingSeparatorsU("\\"))
26+
assertEquals("\\\\", removeTrailingSeparatorsU("\\\\"))
27+
assertEquals("\\a\\", removeTrailingSeparatorsU("\\a\\"))
28+
assertEquals("/\\/ ", removeTrailingSeparatorsU("/\\/ "))
29+
30+
assertEquals("a//\\////\\\\//\\/\\", removeTrailingSeparatorsU("a//\\////\\\\//\\/\\"))
31+
assertEquals("C:\\", removeTrailingSeparatorsU("C:\\"))
32+
assertEquals("C:\\/\\", removeTrailingSeparatorsU("C:\\/\\"))
33+
}
34+
35+
@Test
36+
fun testPathTrimmingWindows() {
37+
assertEquals("", removeTrailingSeparatorsW(""))
38+
assertEquals("/", removeTrailingSeparatorsW("/"))
39+
assertEquals("//", removeTrailingSeparatorsW("//"))
40+
assertEquals("// ", removeTrailingSeparatorsW("// "))
41+
assertEquals("//", removeTrailingSeparatorsW("///"))
42+
assertEquals("@@@", removeTrailingSeparatorsW("@@@"))
43+
assertEquals("/a", removeTrailingSeparatorsW("/a/"))
44+
assertEquals("\\", removeTrailingSeparatorsW("\\"))
45+
assertEquals("\\\\", removeTrailingSeparatorsW("\\\\"))
46+
assertEquals("\\a", removeTrailingSeparatorsW("\\a\\"))
47+
assertEquals("\\a", removeTrailingSeparatorsW("\\a\\\\"))
48+
assertEquals("/\\/ ", removeTrailingSeparatorsW("/\\/ "))
49+
50+
assertEquals("a", removeTrailingSeparatorsW("a//\\////\\\\//\\/\\"))
51+
assertEquals("C:a", removeTrailingSeparatorsW("C:a//\\////\\\\//\\/\\"))
52+
assertEquals("C:\\", removeTrailingSeparatorsW("C:\\"))
53+
assertEquals("C:\\", removeTrailingSeparatorsW("C:\\/\\"))
54+
}
55+
}

‎core/js/src/files/FileSystemJs.kt‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,7 @@
55

66
package kotlinx.io.files
77

8-
import kotlinx.io.IOException
9-
import kotlinx.io.RawSink
10-
import kotlinx.io.RawSource
11-
12-
internal val fs: dynamic
13-
get(): dynamic {
14-
return try {
15-
js("require('fs')")
16-
} catch (t: Throwable) {
17-
null
18-
}
19-
}
20-
21-
internal val os: dynamic
22-
get(): dynamic {
23-
return try {
24-
js("require('os')")
25-
} catch (t: Throwable) {
26-
null
27-
}
28-
}
8+
import kotlinx.io.*
299

3010
public actual val SystemFileSystem: FileSystem = object : SystemFileSystemImpl() {
3111
override fun exists(path: Path): Boolean {
@@ -132,3 +112,5 @@ public actual val SystemTemporaryDirectory: Path
132112
public actual open class FileNotFoundException actual constructor(
133113
message: String?,
134114
) : IOException(message)
115+
116+
internal actual val isWindows = os.platform() == "win32"

‎core/js/src/files/PathsJs.kt‎

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,38 +7,26 @@ package kotlinx.io.files
77

88
import kotlinx.io.*
99

10-
internal val buffer: dynamic
11-
get(): dynamic {
12-
return try {
13-
js("require('buffer')")
14-
} catch (t: Throwable) {
15-
null
16-
}
17-
}
18-
19-
private val pathLib: dynamic
20-
get(): dynamic {
21-
return try {
22-
js("require('path')")
23-
} catch (t: Throwable) {
24-
null
25-
}
26-
}
27-
2810
public actual class Path internal constructor(
29-
internal val path: String,
11+
rawPath: String,
3012
@Suppress("UNUSED_PARAMETER") any: Any?
3113
) {
14+
internal val path: String = removeTrailingSeparators(rawPath)
15+
3216
public actual val parent: Path?
3317
get() {
3418
check(pathLib !== null) { "Path module not found" }
35-
when {
36-
path.isBlank() -> return null
37-
!path.contains(SystemPathSeparator) -> return null
19+
if (path.isEmpty()) return null
20+
if (isWindows) {
21+
if (!path.contains(UnixPathSeparator) && !path.contains(WindowsPathSeparator)) {
22+
return null
23+
}
24+
} else if (!path.contains(SystemPathSeparator)) {
25+
return null
3826
}
3927
val p = pathLib.dirname(path) as String?
4028
return when {
41-
p.isNullOrBlank() -> null
29+
p.isNullOrEmpty() -> null
4230
p == path -> null
4331
else -> Path(p)
4432
}
@@ -54,11 +42,11 @@ public actual class Path internal constructor(
5442
get() {
5543
check(pathLib !== null) { "Path module not found" }
5644
when {
57-
path.isBlank() -> return ""
45+
path.isNullOrEmpty() -> return ""
5846
}
5947
val p = pathLib.basename(path) as String?
6048
return when {
61-
p.isNullOrBlank() -> ""
49+
p.isNullOrEmpty() -> ""
6250
else -> p
6351
}
6452
}

‎core/js/src/imports.kt‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
3+
* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
4+
*/
5+
6+
package kotlinx.io
7+
8+
internal val os: dynamic
9+
get(): dynamic {
10+
return try {
11+
js("require('os')")
12+
} catch (t: Throwable) {
13+
null
14+
}
15+
}
16+
17+
internal val fs: dynamic
18+
get(): dynamic {
19+
return try {
20+
js("require('fs')")
21+
} catch (t: Throwable) {
22+
null
23+
}
24+
}
25+
26+
internal val buffer: dynamic
27+
get(): dynamic {
28+
return try {
29+
js("require('buffer')")
30+
} catch (t: Throwable) {
31+
null
32+
}
33+
}
34+
35+
internal val pathLib: dynamic
36+
get(): dynamic {
37+
return try {
38+
js("require('path')")
39+
} catch (t: Throwable) {
40+
null
41+
}
42+
}

‎core/js/test/files/SmokeFileTestWindowsJS.kt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/*
2+
* Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
3+
* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file.
4+
*/
5+
6+
package kotlinx.io.files
7+
8+
import kotlin.test.Test
9+
import kotlin.test.assertEquals
10+
import kotlin.test.assertNull
11+
12+
class SmokeFileTestWindowJS {
13+
@Test
14+
fun uncParent() {
15+
if (!isWindows) return
16+
// NodeJS, correctly, assume that it's a root path, that's where behavior diverge
17+
assertNull(Path("\\\\server\\share").parent)
18+
assertEquals(Path("\\\\server\\share"), Path("\\\\server\\share\\dir").parent)
19+
}
20+
}

‎core/jvm/src/files/FileSystemJvm.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,5 @@ public actual val SystemFileSystem: FileSystem = object : SystemFileSystemImpl()
102102
public actual val SystemTemporaryDirectory: Path = Path(System.getProperty("java.io.tmpdir"))
103103

104104
public actual typealias FileNotFoundException = java.io.FileNotFoundException
105+
106+
internal actual val isWindows: Boolean = System.getProperty("os.name")?.startsWith("Windows") ?: false

0 commit comments

Comments
 (0)