Encode filepaths to be safe for Windows

pkl-cli/src/test/kotlin/org/pkl/cli/CliEvaluatorTest.kt

@@ -1164,7 +1164,7 @@ result = someLib.x
  """
  .trimIndent()
  )
- assertThat(tempDir.resolve("package-1")).doesNotExist()
+ assertThat(tempDir.resolve("package-2")).doesNotExist()
  }
 
  @Test

pkl-cli/src/test/kotlin/org/pkl/cli/CliPackageDownloaderTest.kt

@@ -56,12 +56,12 @@ class CliPackageDownloaderTest {
  noTransitive = true
  )
  cmd.run()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
  }
 
  @Test
@@ -90,9 +90,9 @@ class CliPackageDownloaderTest {
  noTransitive = true
  )
  cmd.run()
- assertThat(tempDir.resolve(".my-cache/package-1/localhost:0/[email protected]/[email protected]"))
+ assertThat(tempDir.resolve(".my-cache/package-2/localhost(3a)0/[email protected]/[email protected]"))
  .exists()
- assertThat(tempDir.resolve(".my-cache/package-1/localhost:0/[email protected]/[email protected]"))
+ assertThat(tempDir.resolve(".my-cache/package-2/localhost(3a)0/[email protected]/[email protected]"))
  .exists()
  }
 
@@ -113,8 +113,8 @@ class CliPackageDownloaderTest {
  noTransitive = true
  )
  cmd.run()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
  }
 
  @Test
@@ -228,9 +228,9 @@ class CliPackageDownloaderTest {
  noTransitive = false
  )
  .run()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
- assertThat(tempDir.resolve("package-1/localhost:0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
+ assertThat(tempDir.resolve("package-2/localhost(3a)0/[email protected]/[email protected]")).exists()
  }
 }

pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/KotlinCodeGenerator.kt

@@ -22,6 +22,7 @@ import java.net.URI
 import java.util.*
 import org.pkl.core.*
 import org.pkl.core.util.CodeGeneratorUtils
+import org.pkl.core.util.IoUtils
 
 data class KotlinCodegenOptions(
  /** The characters to use for indenting generated Kotlin code. */
@@ -89,7 +90,7 @@ class KotlinCodeGenerator(
 
  private val propertyFileName: String
  get() =
- "resources/META-INF/org/pkl/config/java/mapper/classes/${moduleSchema.moduleName}.properties"
+ "resources/META-INF/org/pkl/config/java/mapper/classes/${IoUtils.encodePath(moduleSchema.moduleName)}.properties"
 
  private val propertiesFile: String
  get() {
@@ -195,7 +196,7 @@ class KotlinCodeGenerator(
  }
 
  private fun relativeOutputPathFor(moduleName: String): String {
- val nameParts = moduleName.split(".")
+ val nameParts = moduleName.split(".").map(IoUtils::encodePath)
  val dirPath = nameParts.dropLast(1).joinToString("/")
  val fileName = nameParts.last().replaceFirstChar { it.titlecaseChar() }
  return if (dirPath.isEmpty()) {

pkl-codegen-kotlin/src/test/kotlin/org/pkl/codegen/kotlin/KotlinCodeGeneratorTest.kt

@@ -1501,6 +1501,24 @@ class KotlinCodeGeneratorTest {
  confirmSerDe(bigStruct)
  }
 
+ @Test
+ fun `encoded file paths`(@TempDir path: Path) {
+ val kotlinCode =
+ generateKotlinFiles(
+ path,
+ PklModule(
+ "FooBar.pkl",
+ """
+ module `Foo*Bar`
+
+ someProp: String
+ """
+ .trimIndent()
+ )
+ )
+ assertThat(kotlinCode).containsKey("kotlin/Foo(2a)Bar.kt")
+ }
+
  private fun generateFiles(tempDir: Path, vararg pklModules: PklModule): Map<String, String> {
  val pklFiles = pklModules.map { it.writeToDisk(tempDir.resolve("pkl/${it.name}.pkl")) }
  val evaluator = Evaluator.preconfigured()

pkl-commons-test/src/main/kotlin/org/pkl/commons/test/PackageServer.kt

@@ -51,7 +51,14 @@ class PackageServer : AutoCloseable {
  const val FRUIT_1_1_SHA = "8d982761d182f2185e4180c82190791d9a60c721cb3393bb2e946fab90131e8c"
 
  fun populateCacheDir(cacheDir: Path) {
- val basePath = cacheDir.resolve("package-1/localhost:$PORT")
+ doPopulateCacheDir(cacheDir.resolve("package-2/localhost(3a)$PORT"))
+ }
+
+ fun populateLegacyCacheDir(cacheDir: Path) {
+ doPopulateCacheDir(cacheDir.resolve("package-1/localhost:$PORT"))
+ }
+
+ private fun doPopulateCacheDir(basePath: Path) {
  basePath.deleteRecursively()
  Files.walk(packagesDir).use { stream ->
  stream.forEach { source ->

pkl-config-java/src/main/java/org/pkl/config/java/mapper/ClassRegistry.java

@@ -22,6 +22,7 @@
 import java.util.Set;
 import org.pkl.config.java.InvalidMappingException;
 import org.pkl.core.PClassInfo;
+import org.pkl.core.util.IoUtils;
 import org.pkl.core.util.Nullable;
 
 /**
@@ -77,7 +78,7 @@ private static void initClassMappings(String pklModuleName) {
  loadedModules.add(pklModuleName);
  var url =
  ClassRegistry.class.getResourceAsStream(
- CLASSES_DIRECTORY + "/" + pklModuleName + ".properties");
+ CLASSES_DIRECTORY + "/" + IoUtils.encodePath(pklModuleName) + ".properties");
  if (url == null) {
  return;
  }

pkl-core/src/main/java/org/pkl/core/packages/PackageResolvers.java

@@ -413,7 +413,7 @@ static final class DiskCachedPackageResolver extends AbstractPackageResolver {
 
  private final Path tmpDir;
 
- private static final String CACHE_DIR_PREFIX = "package-1";
+ private static final String CACHE_DIR_PREFIX = "package-2";
 
  @GuardedBy("lock")
  private final EconomicMap<PackageUri, FileSystem> fileSystems = EconomicMaps.create();
@@ -438,12 +438,14 @@ private String getEffectivePackageUriPath(PackageUri packageUri) {
  return path;
  }
  var checksumIdx = path.lastIndexOf("::");
- return path.substring(0, checksumIdx);
+ return IoUtils.encodePath(path.substring(0, checksumIdx));
  }
 
  private Path getRelativePath(PackageUri uri) {
  return Path.of(
- CACHE_DIR_PREFIX, uri.getUri().getAuthority(), getEffectivePackageUriPath(uri));
+ CACHE_DIR_PREFIX,
+ IoUtils.encodePath(uri.getUri().getAuthority()),
+ getEffectivePackageUriPath(uri));
  }
 
  private String getLastSegmentName(PackageUri packageUri) {

pkl-core/src/main/java/org/pkl/core/util/IoUtils.java

@@ -597,6 +597,30 @@ public static URI fixTripleSlashUri(URI baseUri, URI newUri) {
  return newUri;
  }
 
+ /**
+ * Windows reserves characters {@code <>:"\|?*} in filenames.
+ *
+ * <p>For any such characters, enclose their decimal character code with parentheses. Verbatim
+ * {@code (} is encoded as {@code ((}.
+ */
+ public static String encodePath(String path) {
+ if (path.isEmpty()) return path;
+ var sb = new StringBuilder();
+ for (var i = 0; i < path.length(); i++) {
+ var character = path.charAt(i);
+ switch (character) {
+ case '<', '>', ':', '"', '\\', '|', '?', '*' -> {
+ sb.append('(');
+ sb.append(ByteArrayUtils.toHex(new byte[] {(byte) character}));
+ sb.append(")");
+ }
+ case '(' -> sb.append("((");
+ default -> sb.append(path.charAt(i));
+ }
+ }
+ return sb.toString();
+ }
+
  private static int getExclamationMarkIndex(String jarUri) {
  var index = jarUri.indexOf('!');
  if (index == -1) {

pkl-core/src/test/kotlin/org/pkl/core/util/IoUtilsTest.kt

@@ -431,4 +431,14 @@ class IoUtilsTest {
  IoUtils.readString(URI("http://example.com").toURL())
  }
  }
+
+ @Test
+ fun `encodePath encodes characters reserved on windows`() {
+ assertThat(IoUtils.encodePath("foo:bar")).isEqualTo("foo(3a)bar")
+ assertThat(IoUtils.encodePath("<>:\"\\|?*")).isEqualTo("(3c)(3e)(3a)(22)(5c)(7c)(3f)(2a)")
+ assertThat(IoUtils.encodePath("foo(3a)bar")).isEqualTo("foo((3a)bar")
+ assertThat(IoUtils.encodePath("(")).isEqualTo("((")
+ assertThat(IoUtils.encodePath("3a)")).isEqualTo("3a)")
+ assertThat(IoUtils.encodePath("foo/bar/baz")).isEqualTo("foo/bar/baz")
+ }
 }

pkl-doc/src/main/kotlin/org/pkl/doc/DocGenerator.kt

@@ -121,7 +121,7 @@ class DocGenerator(
 
  private fun createSymlinks(currentPackagesData: List<PackageData>) {
  for (packageData in currentPackagesData) {
- val basePath = outputDir.resolve(packageData.ref.pkg)
+ val basePath = outputDir.resolve(packageData.ref.pkg.pathEncoded)
  val src = basePath.resolve(packageData.ref.version)
  val dest = basePath.resolve("current")
  if (dest.exists() && dest.isSameFileAs(src)) continue

pkl-doc/src/main/kotlin/org/pkl/doc/DocPackageInfo.kt

@@ -203,9 +203,9 @@ data class DocPackageInfo(
  when {
  !moduleName.startsWith(prefix) -> null
  else -> {
- val modulePath = moduleName.substring(prefix.length).replace('.', '/')
+ val modulePath = moduleName.substring(prefix.length).replace('.', '/').pathEncoded
  if (documentation == null) {
- "$name/$version/$modulePath/index.html".toUri()
+ "${name.pathEncoded}/$version/$modulePath/index.html".toUri()
  } else {
  documentation.resolve("$modulePath/index.html")
  }

pkl-doc/src/main/kotlin/org/pkl/doc/DocScope.kt

@@ -71,10 +71,7 @@ internal sealed class DocScope {
  fun resolveModuleNameToRelativeDocUrl(name: String): URI? =
  resolveModuleNameToDocUrl(name)?.let { IoUtils.relativize(it, url) }
 
- abstract fun resolveModuleNameToSourceUrl(
- name: String,
- sourceLocation: Member.SourceLocation
- ): URI?
+ abstract fun resolveModuleNameToSourceUrl(name: String, sourceLocation: SourceLocation): URI?
 
  /** Resolves the given method name relative to this scope. */
  abstract fun resolveMethod(name: String): MethodScope?
@@ -207,10 +204,7 @@ internal class SiteScope(
  else -> null
  }
 
- override fun resolveModuleNameToSourceUrl(
- name: String,
- sourceLocation: Member.SourceLocation
- ): URI? =
+ override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: SourceLocation): URI? =
  when {
  name.startsWith("pkl.") -> {
  val path = "/stdlib/${name.substring(4)}.pkl"
@@ -253,7 +247,9 @@ internal class PackageScope(
  private val moduleScopes: Map<String, ModuleScope> by lazy {
  modules.associate { module ->
  val docUrl =
- url.resolve(getModulePath(module.moduleName, modulePrefix).uriEncoded + "/index.html")
+ url.resolve(
+ getModulePath(module.moduleName, modulePrefix).pathEncoded.uriEncoded + "/index.html"
+ )
  module.moduleName to ModuleScope(module, docUrl, this)
  }
  }
@@ -262,9 +258,11 @@ internal class PackageScope(
  ModuleScope(pklBaseModule, resolveModuleNameToDocUrl("pkl.base")!!, null)
  }
 
- override val url: URI by lazy { parent.url.resolve("./$name/$version/index.html") }
+ override val url: URI by lazy { parent.url.resolve("./${name.pathEncoded}/$version/index.html") }
 
- override val dataUrl: URI by lazy { parent.url.resolve("./data/$name/$version/index.js") }
+ override val dataUrl: URI by lazy {
+ parent.url.resolve("./data/${name.pathEncoded}/$version/index.js")
+ }
 
  fun getModule(name: String): ModuleScope = moduleScopes.getValue(name)
 
@@ -387,11 +385,11 @@ internal class ClassScope(
  override val url: URI by lazy {
  // `isModuleClass` distinction is relevant when this scope is a link target
  if (clazz.isModuleClass) parentUrl
- else parentUrl.resolve("${clazz.simpleName.uriEncodedComponent}.html")
+ else parentUrl.resolve("${clazz.simpleName.pathEncoded.uriEncodedComponent}.html")
  }
 
  override val dataUrl: URI by lazy {
- parent!!.dataUrl.resolve("${clazz.simpleName.uriEncodedComponent}.js")
+ parent!!.dataUrl.resolve("${clazz.simpleName.pathEncoded.uriEncodedComponent}.js")
  }
 
  override fun getMethod(name: String): MethodScope? =
@@ -403,10 +401,8 @@ internal class ClassScope(
  override fun resolveModuleNameToDocUrl(name: String): URI? =
  parent!!.resolveModuleNameToDocUrl(name)
 
- override fun resolveModuleNameToSourceUrl(
- name: String,
- sourceLocation: Member.SourceLocation
- ): URI? = parent!!.resolveModuleNameToSourceUrl(name, sourceLocation)
+ override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: SourceLocation): URI? =
+ parent!!.resolveModuleNameToSourceUrl(name, sourceLocation)
 
  override fun resolveMethod(name: String): MethodScope? =
  clazz.methods[name]?.let { MethodScope(it, this) }
@@ -438,7 +434,7 @@ internal class TypeAliasScope(
  // only used for page scopes
  throw UnsupportedOperationException("resolveModuleNameToDocUrl")
 
- override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: Member.SourceLocation) =
+ override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: SourceLocation) =
  // only used for page scopes
  throw UnsupportedOperationException("resolveModuleNameToSourceUrl")
 
@@ -464,7 +460,7 @@ internal class MethodScope(val method: PClass.Method, override val parent: DocSc
  // only used for page scopes
  throw UnsupportedOperationException("resolveModuleNameToDocUrl")
 
- override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: Member.SourceLocation) =
+ override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: SourceLocation) =
  // only used for page scopes
  throw UnsupportedOperationException("resolveModuleNameToSourceUrl")
 
@@ -494,7 +490,7 @@ internal class PropertyScope(
  // only used for page scopes
  throw UnsupportedOperationException("resolveModuleNameToDocUrl")
 
- override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: Member.SourceLocation) =
+ override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: SourceLocation) =
  // only used for page scopes
  throw UnsupportedOperationException("resolveModuleNameToSourceUrl")
 
@@ -525,7 +521,7 @@ internal class ParameterScope(val name: String, override val parent: DocScope) :
  // only used for page scopes
  throw UnsupportedOperationException("resolveModuleNameToDocUrl")
 
- override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: Member.SourceLocation) =
+ override fun resolveModuleNameToSourceUrl(name: String, sourceLocation: SourceLocation) =
  // only used for page scopes
  throw UnsupportedOperationException("resolveModuleNameToSourceUrl")
 

pkl-doc/src/main/kotlin/org/pkl/doc/PackageDataGenerator.kt

@@ -37,9 +37,11 @@ import org.pkl.core.util.IoUtils
 internal class PackageDataGenerator(private val outputDir: Path) {
  fun generate(pkg: DocPackage) {
  val path =
- outputDir.resolve(pkg.name).resolve(pkg.version).resolve("package-data.json").apply {
- createParentDirectories()
- }
+ outputDir
+ .resolve(pkg.name.pathEncoded)
+ .resolve(pkg.version)
+ .resolve("package-data.json")
+ .apply { createParentDirectories() }
  PackageData(pkg).write(path)
  }