From ffd8a5d4840a06c6cc17daabb03f607acc126c4f Mon Sep 17 00:00:00 2001 From: Daniel Chao Date: Tue, 3 Nov 2026 05:21:36 -0800 Subject: [PATCH] Remove treatment of `%5c` as a slash on Windows (#1954) We reject any filepath that contains a `\` character; all Pkl code must use `/` as the path separator. So, we don't need to normalize `%5c` as a path sep on Windows. --- .../main/java/org/pkl/core/util/IoUtils.java | 13 ++-- .../kotlin/org/pkl/core/util/IoUtilsTest.kt | 77 ++++++------------- 2 files changed, 31 insertions(+), 59 deletions(-) diff --git a/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java b/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java index d23a68408..c9fb0916b 100644 --- a/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java +++ b/pkl-core/src/main/java/org/pkl/core/util/IoUtils.java @@ -88,7 +88,6 @@ public final class IoUtils { private static final Pattern DOT_SEGMENT = Pattern.compile("\\.|%2[eE]"); private static final Pattern DOT_DOT_SEGMENT = Pattern.compile("(?:\\.|%2[eE]){2}"); private static final Pattern PATH_SEPARATOR = Pattern.compile("/|%2[fF]"); - private static final Pattern WINDOWS_PATH_SEPARATOR = Pattern.compile("/|%2[fF]|%5[cC]"); private IoUtils() {} @@ -483,7 +482,7 @@ public final class IoUtils { if (assetPath == null || !assetPath.startsWith("/")) { return uri; } - var normalizedPath = normalizeRawPath(assetPath, true, isWindows()); + var normalizedPath = normalizeRawPath(assetPath, true); if (normalizedPath.equals(uri.getRawFragment())) { return uri; } @@ -512,7 +511,7 @@ public final class IoUtils { pathStart += 2 + (rawAuthority == null ? 0 : rawAuthority.length()); } assert uriString.startsWith(rawPath, pathStart); - var normalizedPath = normalizeRawPath(rawPath, decodeSlash, isWindows()); + var normalizedPath = normalizeRawPath(rawPath, decodeSlash); if (normalizedPath.equals(rawPath)) { return uri; } @@ -552,13 +551,13 @@ public final class IoUtils { *

If {@code decodeSlash} is true, treats either {@code /} or {@code %2F} as the path * separator, and replaces {@code %2F} with {@code /}. Otherwise, {@code %2F} is kept as is. * - *

If {@code isWindows}, then {@code %5c} is also treated as a slash. + *

Don't need to treat {@code %5F} ({@code \}) as a slash on Windows; we reject any {@code \} + * as a path separator (see {@code org.pkl.core.module.ModuleKeys.File#resolve}). */ - static String normalizeRawPath(String rawPath, boolean decodeSlash, boolean isWindows) { + static String normalizeRawPath(String rawPath, boolean decodeSlash) { var isAbsolute = rawPath.startsWith("/"); var rawSegments = (isAbsolute ? rawPath.substring(1) : rawPath); - var separator = isWindows ? WINDOWS_PATH_SEPARATOR : PATH_SEPARATOR; - var segments = decodeSlash ? separator.split(rawSegments, -1) : rawSegments.split("/", -1); + var segments = decodeSlash ? PATH_SEPARATOR.split(rawSegments, -1) : rawSegments.split("/", -1); var result = new ArrayList(segments.length); for (var i = 0; i < segments.length; i++) { var segment = segments[i]; diff --git a/pkl-core/src/test/kotlin/org/pkl/core/util/IoUtilsTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/util/IoUtilsTest.kt index 10299f222..d46540851 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/util/IoUtilsTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/util/IoUtilsTest.kt @@ -558,71 +558,44 @@ class IoUtilsTest { @Test fun `normalizeRawPath removes dot segments`() { - for (isWindows in listOf(false, true)) { - for (decodeSlash in listOf(false, true)) { - fun normalize(path: String) = IoUtils.normalizeRawPath(path, decodeSlash, isWindows) - assertThat(normalize("/a/b/../c")).isEqualTo("/a/c") - assertThat(normalize("/a/%2e%2E/b")).isEqualTo("/b") - assertThat(normalize("/a/./b/.")).isEqualTo("/a/b/") - assertThat(normalize("/a/b/..")).isEqualTo("/a/") - assertThat(normalize("/../x")).isEqualTo("/../x") - assertThat(normalize("/a/../..")).isEqualTo("/../") - assertThat(normalize("//etc/../x")).isEqualTo("//x") - assertThat(normalize("")).isEqualTo("") - assertThat(normalize("/")).isEqualTo("/") - } + for (decodeSlash in listOf(false, true)) { + fun normalize(path: String) = IoUtils.normalizeRawPath(path, decodeSlash) + assertThat(normalize("/a/b/../c")).isEqualTo("/a/c") + assertThat(normalize("/a/%2e%2E/b")).isEqualTo("/b") + assertThat(normalize("/a/./b/.")).isEqualTo("/a/b/") + assertThat(normalize("/a/b/..")).isEqualTo("/a/") + assertThat(normalize("/../x")).isEqualTo("/../x") + assertThat(normalize("/a/../..")).isEqualTo("/../") + assertThat(normalize("//etc/../x")).isEqualTo("//x") + assertThat(normalize("")).isEqualTo("") + assertThat(normalize("/")).isEqualTo("/") } } @Test fun `normalizeRawPath treats 2f as a path separator only if decodeSlash`() { - for (isWindows in listOf(false, true)) { - assertThat(IoUtils.normalizeRawPath("/a%2f..%2fb", true, isWindows)).isEqualTo("/b") - assertThat(IoUtils.normalizeRawPath("/a%2F..%2Fb", true, isWindows)).isEqualTo("/b") - assertThat(IoUtils.normalizeRawPath("/a%2fb", true, isWindows)).isEqualTo("/a/b") - assertThat(IoUtils.normalizeRawPath("/a/.%2f./b", true, isWindows)).isEqualTo("/a/b") - assertThat(IoUtils.normalizeRawPath("/%2f..%2fx", true, isWindows)).isEqualTo("/x") - // `%2F` is distinct from `/` if not decodeSlash - assertThat(IoUtils.normalizeRawPath("/a%2f..%2fb", false, isWindows)).isEqualTo("/a%2f..%2fb") - assertThat(IoUtils.normalizeRawPath("/a%2fb/./c", false, isWindows)).isEqualTo("/a%2fb/c") - } + assertThat(IoUtils.normalizeRawPath("/a%2f..%2fb", true)).isEqualTo("/b") + assertThat(IoUtils.normalizeRawPath("/a%2F..%2Fb", true)).isEqualTo("/b") + assertThat(IoUtils.normalizeRawPath("/a%2fb", true)).isEqualTo("/a/b") + assertThat(IoUtils.normalizeRawPath("/a/.%2f./b", true)).isEqualTo("/a/b") + assertThat(IoUtils.normalizeRawPath("/%2f..%2fx", true)).isEqualTo("/x") + // `%2F` is distinct from `/` if not decodeSlash + assertThat(IoUtils.normalizeRawPath("/a%2f..%2fb", false)).isEqualTo("/a%2f..%2fb") + assertThat(IoUtils.normalizeRawPath("/a%2fb/./c", false)).isEqualTo("/a%2fb/c") } @Test fun `normalizeRawPath does not decode 2f twice`() { - assertThat(IoUtils.normalizeRawPath("/a%252f..%2fb", true, false)).isEqualTo("/a%252f../b") - assertThat(IoUtils.normalizeRawPath("/a%255c..%5cb", true, true)).isEqualTo("/a%255c../b") - } - - @Test - fun `normalizeRawPath treats 5c as a path separator only on Windows`() { - // on Windows, `\` is a path separator - assertThat(IoUtils.normalizeRawPath("/a%5c..%5cb", true, true)).isEqualTo("/b") - assertThat(IoUtils.normalizeRawPath("/a%5C..%5Cb", true, true)).isEqualTo("/b") - assertThat(IoUtils.normalizeRawPath("/a%5cb", true, true)).isEqualTo("/a/b") - assertThat(IoUtils.normalizeRawPath("/a/..%5c..%5cb", true, true)).isEqualTo("/../b") - assertThat(IoUtils.normalizeRawPath("/a%5c.%2f..%2fb", true, true)).isEqualTo("/b") - // on other platforms, `\` is a regular file name character - assertThat(IoUtils.normalizeRawPath("/a%5c..%5cb", true, false)).isEqualTo("/a%5c..%5cb") - assertThat(IoUtils.normalizeRawPath("/a%5C..%5Cb/./c", true, false)).isEqualTo("/a%5C..%5Cb/c") - } - - @Test - fun `normalizeRawPath does not treat 5c as a path separator if not decodeSlash`() { - for (isWindows in listOf(false, true)) { - assertThat(IoUtils.normalizeRawPath("/a%5c..%5cb", false, isWindows)).isEqualTo("/a%5c..%5cb") - } + assertThat(IoUtils.normalizeRawPath("/a%252f..%2fb", true)).isEqualTo("/a%252f../b") } @Test fun `normalizeRawPath is idempotent`() { - for (isWindows in listOf(false, true)) { - for (decodeSlash in listOf(false, true)) { - for (path in - listOf("/a/../b", "/a%2f..%2fb", "/a%5c..%5cb", "/a/.%5c./b", "//x/../y", "/../../a")) { - val once = IoUtils.normalizeRawPath(path, decodeSlash, isWindows) - assertThat(IoUtils.normalizeRawPath(once, decodeSlash, isWindows)).isEqualTo(once) - } + for (decodeSlash in listOf(false, true)) { + for (path in + listOf("/a/../b", "/a%2f..%2fb", "/a%5c..%5cb", "/a/.%5c./b", "//x/../y", "/../../a")) { + val once = IoUtils.normalizeRawPath(path, decodeSlash) + assertThat(IoUtils.normalizeRawPath(once, decodeSlash)).isEqualTo(once) } } }