From f9fe226eba4a0f1a9f672584a4a009b072516d4b Mon Sep 17 00:00:00 2001 From: Josh B <421772+HT154@users.noreply.github.com> Date: Wed, 23 Oct 2024 20:52:40 -0700 Subject: [PATCH] Fix handling of `file:` module URIs with non-ASCII characters (#696) Addresses an issue where Pkl cannot evaluate files with non-ASCII characters. --- .../kotlin/org/pkl/cli/CliEvaluatorTest.kt | 56 +++++++++++++++++++ .../org/pkl/core/StackFrameTransformers.java | 3 +- .../org/pkl/core/module/FileResolver.java | 5 +- .../java/org/pkl/core/module/ModuleKeys.java | 4 +- .../main/java/org/pkl/core/util/IoUtils.java | 14 +++++ .../input/modules/日本語.pkl | 6 ++ .../input/modules/日本語_error.pkl | 2 + .../output/modules/日本語.pcf | 21 +++++++ .../output/modules/日本語_error.err | 10 ++++ .../pkl/core/LanguageSnippetTestsEngine.kt | 9 ++- 10 files changed, 121 insertions(+), 9 deletions(-) create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/modules/日本語.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/modules/日本語_error.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/output/modules/日本語.pcf create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/output/modules/日本語_error.err diff --git a/pkl-cli/src/test/kotlin/org/pkl/cli/CliEvaluatorTest.kt b/pkl-cli/src/test/kotlin/org/pkl/cli/CliEvaluatorTest.kt index 5066fb91..7974d856 100644 --- a/pkl-cli/src/test/kotlin/org/pkl/cli/CliEvaluatorTest.kt +++ b/pkl-cli/src/test/kotlin/org/pkl/cli/CliEvaluatorTest.kt @@ -1457,6 +1457,62 @@ result = someLib.x assertThat(output).isEqualTo("result = 1\n") } + @Test + fun `eval file with non-ASCII name`() { + val tempDirUri = tempDir.toUri() + val dir = tempDir.resolve("🤬").createDirectory() + val file = + writePklFile( + dir.resolve("日本語.pkl").toString(), + """ + 日本語 = "Japanese language" + readDir = read(".").text + readDirFile = read("$tempDirUri🤬").text + readOne = read("日本語.pkl").text.split("\n").first + readOneFile = read("$tempDirUri🤬/日本語.pkl").text.split("\n").first + readGlob = read*("./日*.pkl").keys + readGlobFile = read*("$tempDirUri**/*.pkl").keys.map((it) -> it.replaceAll("$tempDirUri".replaceAll("///", "/"), "")) + importOne = import("日本語.pkl").readOne + importOneFile = import("$tempDirUri🤬/日本語.pkl").日本語 + importGlob = import*("./日*.pkl").keys + importGlobFile = import*("$tempDirUri**/*.pkl").keys.map((it) -> it.replaceAll("$tempDirUri".replaceAll("///", "/"), "")) + """ + .trimIndent() + ) + val output = + evalToConsole( + CliEvaluatorOptions( + CliBaseOptions(sourceModules = listOf(file)), + ) + ) + + val tripleQuote = "\"\"\"" + assertThat(output) + .isEqualTo( + """ + 日本語 = "Japanese language" + readDir = $tripleQuote + 日本語.pkl + + $tripleQuote + readDirFile = $tripleQuote + 日本語.pkl + + $tripleQuote + readOne = "日本語 = \"Japanese language\"" + readOneFile = "日本語 = \"Japanese language\"" + readGlob = Set("./日本語.pkl") + readGlobFile = Set("🤬/日本語.pkl") + importOne = "日本語 = \"Japanese language\"" + importOneFile = "Japanese language" + importGlob = Set("./日本語.pkl") + importGlobFile = Set("🤬/日本語.pkl") + + """ + .trimIndent() + ) + } + private fun evalModuleThatImportsPackage(certsFile: Path?, testPort: Int = -1) { val moduleUri = writePklFile( diff --git a/pkl-core/src/main/java/org/pkl/core/StackFrameTransformers.java b/pkl-core/src/main/java/org/pkl/core/StackFrameTransformers.java index 4e220c75..ed1b7dcd 100644 --- a/pkl-core/src/main/java/org/pkl/core/StackFrameTransformers.java +++ b/pkl-core/src/main/java/org/pkl/core/StackFrameTransformers.java @@ -18,7 +18,6 @@ package org.pkl.core; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; -import java.nio.file.Path; import java.util.stream.StreamSupport; import org.pkl.core.packages.PackageAssetUri; import org.pkl.core.runtime.VmContext; @@ -93,7 +92,7 @@ public final class StackFrameTransformers { var uri = frame.getModuleUri(); if (!uri.startsWith("file:")) return frame; - return transformUri(frame, Path.of(URI.create(uri)).toString(), scheme); + return transformUri(frame, IoUtils.pathOf(URI.create(uri)).toString(), scheme); }; } diff --git a/pkl-core/src/main/java/org/pkl/core/module/FileResolver.java b/pkl-core/src/main/java/org/pkl/core/module/FileResolver.java index 1bd2381f..87c4f217 100644 --- a/pkl-core/src/main/java/org/pkl/core/module/FileResolver.java +++ b/pkl-core/src/main/java/org/pkl/core/module/FileResolver.java @@ -24,12 +24,13 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.pkl.core.util.IoUtils; public final class FileResolver { private FileResolver() {} public static List listElements(URI baseUri) throws IOException { - return listElements(Path.of(baseUri)); + return listElements(IoUtils.pathOf(baseUri)); } public static List listElements(Path path) throws IOException { @@ -49,7 +50,7 @@ public final class FileResolver { } public static boolean hasElement(URI elementUri) { - return Files.exists(Path.of(elementUri)); + return Files.exists(IoUtils.pathOf(elementUri)); } public static boolean hasElement(Path path) { diff --git a/pkl-core/src/main/java/org/pkl/core/module/ModuleKeys.java b/pkl-core/src/main/java/org/pkl/core/module/ModuleKeys.java index 2a496c02..b633a663 100644 --- a/pkl-core/src/main/java/org/pkl/core/module/ModuleKeys.java +++ b/pkl-core/src/main/java/org/pkl/core/module/ModuleKeys.java @@ -16,7 +16,6 @@ package org.pkl.core.module; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; -import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -26,7 +25,6 @@ import java.net.URISyntaxException; import java.net.http.HttpRequest; import java.net.http.HttpResponse.BodyHandlers; import java.nio.charset.StandardCharsets; -import java.nio.file.Path; import java.util.List; import java.util.Map; import org.pkl.core.SecurityManager; @@ -325,7 +323,7 @@ public final class ModuleKeys { if (java.io.File.separatorChar == '\\' && uriPath != null && uriPath.contains("\\")) { throw new FileNotFoundException(); } - var realPath = Path.of(uri).toRealPath(); + var realPath = IoUtils.pathOf(uri).toRealPath(); var resolvedUri = realPath.toUri(); securityManager.checkResolveModule(resolvedUri); return ResolvedModuleKeys.file(this, resolvedUri, realPath); 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 e32d245d..07174237 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,6 +88,20 @@ public final class IoUtils { return new URI(null, null, str, null); } + /** Converts a URI to a Path, normalizing any non-ASCII characters. */ + public static Path pathOf(URI uri) { + // Path.of(URI) throws on non-ASCII characters so the module URI here must be normalized to + // ASCII + // Unfortunately there's no way to go from URI -> ASCII URI directly + // so this must transform URI -> ASCII String -> ASCII URI + try { + return Path.of(new URI(uri.toASCIIString())); + } catch (URISyntaxException e) { + // impossible to get here; we started from a valid URI to begin with + throw PklBugException.unreachableCode(); + } + } + /** Like {@link #toUri(String)}, except without checked exceptions. */ public static URI createUri(String str) { try { diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/modules/日本語.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/modules/日本語.pkl new file mode 100644 index 00000000..2049fa4d --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/modules/日本語.pkl @@ -0,0 +1,6 @@ +// covers https://github.com/apple/pkl/issues/653 +日本語 = "Japanese language" +readOne = read("日本語.pkl").text +readGlob = read*("./日*.pkl").keys +importOne = import("日本語.pkl").readOne +importGlob = import*("./日*.pkl").keys diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/modules/日本語_error.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/modules/日本語_error.pkl new file mode 100644 index 00000000..2af8f636 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/modules/日本語_error.pkl @@ -0,0 +1,2 @@ +// covers https://github.com/apple/pkl/issues/653 +日本語 = throw("Error reporting should also handle unicode filenames!") diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/modules/日本語.pcf b/pkl-core/src/test/files/LanguageSnippetTests/output/modules/日本語.pcf new file mode 100644 index 00000000..97feec80 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/modules/日本語.pcf @@ -0,0 +1,21 @@ +日本語 = "Japanese language" +readOne = """ + // covers https://github.com/apple/pkl/issues/653 + 日本語 = "Japanese language" + readOne = read("日本語.pkl").text + readGlob = read*("./日*.pkl").keys + importOne = import("日本語.pkl").readOne + importGlob = import*("./日*.pkl").keys + + """ +readGlob = Set("./日本語.pkl", "./日本語_error.pkl") +importOne = """ + // covers https://github.com/apple/pkl/issues/653 + 日本語 = "Japanese language" + readOne = read("日本語.pkl").text + readGlob = read*("./日*.pkl").keys + importOne = import("日本語.pkl").readOne + importGlob = import*("./日*.pkl").keys + + """ +importGlob = Set("./日本語.pkl", "./日本語_error.pkl") diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/modules/日本語_error.err b/pkl-core/src/test/files/LanguageSnippetTests/output/modules/日本語_error.err new file mode 100644 index 00000000..6b4e13aa --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/modules/日本語_error.err @@ -0,0 +1,10 @@ +–– Pkl Error –– +Error reporting should also handle unicode filenames! + +x | 日本語 = throw("Error reporting should also handle unicode filenames!") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at 日本語_error#日本語 (file:///$snippetsDir/input/modules/%E6%97%A5%E6%9C%AC%E8%AA%9E_error.pkl) + +xxx | text = renderer.renderDocument(value) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at pkl.base#Module.output.text (pkl:base) diff --git a/pkl-core/src/test/kotlin/org/pkl/core/LanguageSnippetTestsEngine.kt b/pkl-core/src/test/kotlin/org/pkl/core/LanguageSnippetTestsEngine.kt index dec902d7..4d9282d4 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/LanguageSnippetTestsEngine.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/LanguageSnippetTestsEngine.kt @@ -301,9 +301,14 @@ class AlpineLanguageSnippetTestsEngine : AbstractNativeLanguageSnippetTestsEngin override val testClass: KClass<*> = AlpineLanguageSnippetTests::class } -// error message contains different file path on Windows private val windowsExcludedTests - get() = listOf(Regex(".*missingProjectDeps/bug\\.pkl")) + get() = + listOf( + // error message contains different file path on Windows + Regex(".*missingProjectDeps/bug\\.pkl"), + // URIs get rendered slightly differently (percent-encoded vs raw) + Regex(".*日本語_error\\.pkl") + ) class WindowsLanguageSnippetTestsEngine : AbstractNativeLanguageSnippetTestsEngine() { override val pklExecutablePath: Path = PklExecutablePaths.windowsAmd64