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.
This commit is contained in:
1 parent
a9b7fe3a33
commit
ffd8a5d484
2 files changed
+31
-59
No files matched your search
@@ -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 {
|
||||
* <p>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.
|
||||
*
|
||||
* <p>If {@code isWindows}, then {@code %5c} is also treated as a slash.
|
||||
* <p>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<String>(segments.length);
|
||||
for (var i = 0; i < segments.length; i++) {
|
||||
var segment = segments[i];
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user