From f6d240020829001ed972d0d37092e731898d60e3 Mon Sep 17 00:00:00 2001 From: Ilya Gorbunov Date: Tue, 6 Oct 2020 21:34:02 +0300 Subject: [PATCH] Workaround bugs in Path.relativize Also add actual paths to the relativeTo error message. #KT-19192 --- .../stdlib/jdk7/src/kotlin/io/PathUtils.kt | 43 ++++++- .../stdlib/jdk7/test/PathExtensionsTest.kt | 115 +++++++++++------- 2 files changed, 105 insertions(+), 53 deletions(-) diff --git a/libraries/stdlib/jdk7/src/kotlin/io/PathUtils.kt b/libraries/stdlib/jdk7/src/kotlin/io/PathUtils.kt index 2d34729e147..a28d163a222 100644 --- a/libraries/stdlib/jdk7/src/kotlin/io/PathUtils.kt +++ b/libraries/stdlib/jdk7/src/kotlin/io/PathUtils.kt @@ -55,7 +55,11 @@ public val Path.nameWithoutExtension: String */ @SinceKotlin("1.4") @ExperimentalStdlibApi -public fun Path.relativeTo(base: Path): Path = base.relativize(this) +public fun Path.relativeTo(base: Path): Path = try { + PathRelativizer.tryRelativeTo(this, base) +} catch (e: IllegalArgumentException) { + throw java.lang.IllegalArgumentException(e.message + "\nthis path: $this\nbase path: $base", e) +} /** * Calculates the relative path for this path from a [base] path. @@ -78,11 +82,38 @@ public fun Path.relativeToOrSelf(base: Path): Path = */ @SinceKotlin("1.4") @ExperimentalStdlibApi -public fun Path.relativeToOrNull(base: Path): Path? { - return try { - base.relativize(this) - } catch (e: IllegalArgumentException) { - null +public fun Path.relativeToOrNull(base: Path): Path? = try { + PathRelativizer.tryRelativeTo(this, base) +} catch (e: IllegalArgumentException) { + null +} + +internal object PathRelativizer { + private val emptyPath = Paths.get("") + private val parentPath = Paths.get("..") + + // Workarounds some bugs in Path.relativize that were fixed only in JDK9 + fun tryRelativeTo(path: Path, base: Path): Path { + val bn = base.normalize() + val pn = path.normalize() + val rn = bn.relativize(pn) + // work around https://bugs.openjdk.java.net/browse/JDK-8066943 + for (i in 0 until minOf(bn.nameCount, pn.nameCount)) { + if (bn.getName(i) != parentPath) break + if (pn.getName(i) != parentPath) throw IllegalArgumentException("Unable to compute relative path") + } + // work around https://bugs.openjdk.java.net/browse/JDK-8072495 + val r = if (pn != bn && bn == emptyPath) { + pn + } else { + val rnString = rn.toString() + // drop invalid dangling separator from path string https://bugs.openjdk.java.net/browse/JDK-8140449 + if (rnString.endsWith(rn.fileSystem.separator)) + rn.fileSystem.getPath(rnString.dropLast(rn.fileSystem.separator.length)) + else + rn + } + return r } } diff --git a/libraries/stdlib/jdk7/test/PathExtensionsTest.kt b/libraries/stdlib/jdk7/test/PathExtensionsTest.kt index 2a3ed420bee..82a1ec089b4 100644 --- a/libraries/stdlib/jdk7/test/PathExtensionsTest.kt +++ b/libraries/stdlib/jdk7/test/PathExtensionsTest.kt @@ -222,48 +222,68 @@ class PathExtensionsTest { assertFailsWith { file.forEachDirectoryEntry { } } } + + private fun testRelativeTo(expected: String?, path: String, base: String) = + testRelativeTo(expected?.let { Paths.get(it) }, Paths.get(path), Paths.get(base)) + private fun testRelativeTo(expected: String, path: Path, base: Path) = + testRelativeTo(Paths.get(expected), path, base) + + private fun testRelativeTo(expected: Path?, path: Path, base: Path) { + val context = "path: '$path', base: '$base'" + if (expected != null) { + assertEquals(expected, path.relativeTo(base), context) + } else { + val e = assertFailsWith(context) { path.relativeTo(base) } + val message = assertNotNull(e.message) + assertTrue(path.toString() in message, message) + assertTrue(base.toString() in message, message) + } + assertEquals(expected, path.relativeToOrNull(base), context) + assertEquals(expected ?: path, path.relativeToOrSelf(base), context) + } + @Test fun relativeToRooted() { - val file1 = Paths.get("/foo/bar/baz") - val file2 = Paths.get("/foo/baa/ghoo") + val file1 = "/foo/bar/baz" + val file2 = "/foo/baa/ghoo" - assertEquals("../../bar/baz", file1.relativeTo(file2).invariantSeparatorsPath) + testRelativeTo("../../bar/baz", file1, file2) - val file3 = Paths.get("/foo/bar") + val file3 = "/foo/bar" - assertEquals("baz", file1.relativeTo(file3).toString()) - assertEquals("..", file3.relativeTo(file1).toString()) + testRelativeTo("baz", file1, file3) + testRelativeTo("..", file3, file1) - val file4 = Paths.get("/foo/bar/") + val file4 = "/foo/bar/" - assertEquals("baz", file1.relativeTo(file4).toString()) - assertEquals("..", file4.relativeTo(file1).toString()) - assertEquals("", file3.relativeTo(file4).toString()) - assertEquals("", file4.relativeTo(file3).toString()) + testRelativeTo("baz", file1, file4) + testRelativeTo("..", file4, file1) + testRelativeTo("", file3, file4) + testRelativeTo("", file4, file3) - val file5 = Paths.get("/foo/baran") + val file5 = "/foo/baran" - assertEquals("../bar", file3.relativeTo(file5).invariantSeparatorsPath) - assertEquals("../baran", file5.relativeTo(file3).invariantSeparatorsPath) - assertEquals("../bar", file4.relativeTo(file5).invariantSeparatorsPath) - assertEquals("../baran", file5.relativeTo(file4).invariantSeparatorsPath) + testRelativeTo("../bar", file3, file5) + testRelativeTo("../baran", file5, file3) + testRelativeTo("../bar", file4, file5) + testRelativeTo("../baran", file5, file4) if (isBackslashSeparator) { - val file6 = Paths.get("C:\\Users\\Me") - val file7 = Paths.get("C:\\Users\\Me\\Documents") + val file6 = "C:\\Users\\Me" + val file7 = "C:\\Users\\Me\\Documents" - assertEquals("..", file6.relativeTo(file7).toString()) - assertEquals("Documents", file7.relativeTo(file6).toString()) + testRelativeTo("..", file6, file7) + testRelativeTo("Documents", file7, file6) - val file8 = Paths.get("""\\my.host\home/user/documents/vip""") - val file9 = Paths.get("""\\my.host\home/other/images/nice""") + val file8 = """\\my.host\home/user/documents/vip""" + val file9 = """\\my.host\home/other/images/nice""" - assertEquals("../../../user/documents/vip", file8.relativeTo(file9).invariantSeparatorsPath) - assertEquals("../../../other/images/nice", file9.relativeTo(file8).invariantSeparatorsPath) + testRelativeTo("../../../user/documents/vip", file8, file9) + testRelativeTo("../../../other/images/nice", file9, file8) } if (isCaseInsensitiveFileSystem) { - assertEquals("bar", Paths.get("C:/bar").relativeTo(Paths.get("c:/")).toString()) + testRelativeTo("bar", "C:/bar", "c:/") } } @@ -272,30 +292,33 @@ class PathExtensionsTest { val nested = Paths.get("foo/bar") val base = Paths.get("foo") - assertEquals("bar", nested.relativeTo(base).toString()) - assertEquals("..", base.relativeTo(nested).toString()) + testRelativeTo("bar", nested, base) + testRelativeTo("..", base, nested) + val empty = Paths.get("") val current = Paths.get(".") val parent = Paths.get("..") val outOfRoot = Paths.get("../bar") - assertEquals(Paths.get("../../bar"), outOfRoot.relativeTo(base)) - assertEquals("bar", outOfRoot.relativeTo(parent).toString()) - assertEquals("..", parent.relativeTo(outOfRoot).toString()) + testRelativeTo("../bar", outOfRoot, empty) + testRelativeTo("../../bar", outOfRoot, base) + testRelativeTo("bar", outOfRoot, parent) + testRelativeTo("..", parent, outOfRoot) val root = Paths.get("/root") - val files = listOf(nested, base, outOfRoot, current, parent) - val bases = listOf(nested, base, current) + val files = listOf(nested, base, empty, outOfRoot, current, parent) + val bases = listOf(nested, base, empty, current) for (file in files) - assertEquals("", file.relativeTo(file).toString(), "file should have empty path relative to itself: $file") + // file should have empty path relative to itself + testRelativeTo("", file, file) for (file in files) { @Suppress("NAME_SHADOWING") for (base in bases) { val rootedFile = root.resolve(file) val rootedBase = root.resolve(base) - assertEquals(file.relativeTo(base), rootedFile.relativeTo(rootedBase), "nested: $file, base: $base") + assertEquals(rootedFile.relativeTo(rootedBase), file.relativeTo(base), "nested: $file, base: $base") } } } @@ -307,30 +330,28 @@ class PathExtensionsTest { val networkShare1 = Paths.get("""\\my.host\share1/folder""") val networkShare2 = Paths.get("""\\my.host\share2\folder""") - fun assertFailsRelativeTo(file: Path, base: Path) { - val e = assertFailsWith("file: $file, base: $base") { file.relativeTo(base) } - assertNotNull(e.message) - } - val allFiles = listOf(absolute, relative) + if (isBackslashSeparator) listOf(networkShare1, networkShare2) else emptyList() for (file in allFiles) { for (base in allFiles) { - if (file != base) assertFailsRelativeTo(file, base) + if (file != base) testRelativeTo(null, file, base) } } if (isBackslashSeparator) { - val fileOnC = Paths.get("C:/dir1") - val fileOnD = Paths.get("D:/dir2") - assertFailsRelativeTo(fileOnC, fileOnD) + testRelativeTo(null, "C:/dir1", "D:/dir2") } + + testRelativeTo(null, "foo", "..") + testRelativeTo(null, "../foo", "../..") } @Test fun relativeTo() { - assertEquals("kotlin", Paths.get("src/kotlin").relativeTo(Paths.get("src")).toString()) - assertEquals("", Paths.get("dir").relativeTo(Paths.get("dir")).toString()) - assertEquals("..", Paths.get("dir").relativeTo(Paths.get("dir/subdir")).toString()) - assertEquals(Paths.get("../../test"), Paths.get("test").relativeTo(Paths.get("dir/dir"))) + testRelativeTo("kotlin", "src/kotlin", "src") + testRelativeTo("", "dir", "dir") + testRelativeTo("..", "dir", "dir/subdir") + testRelativeTo("../../test", "test", "dir/dir") + testRelativeTo("foo/bar", "../../foo/bar", "../../sub/../.") + testRelativeTo(null, "../../foo/bar", "../../sub/../..") } }