Treat opaque file URIs as errors (#1087)

Opaque file URIs are URIs whose scheme-specific part does not start with `/`.
For example, `file:foo/bar.txt` is an opaque URI.

Currently, this has the unintentional behavior of: look for file `foo/bar.txt` from the process working directory.
These are effectively dynamics imports; from a single import, we can't statically analyze what it resolves as.

According to RFC-8089, File URIs must have paths that start with `/`. So, these are actually _not valid URIs_.
See the grammar defined in https://datatracker.ietf.org/doc/html/rfc8089#section-2

This changes Pkl's behavior so that these URIs are treated as errors.
This commit is contained in:
Daniel Chao
2025-06-03 14:57:22 -07:00
committed by GitHub
parent 4eeb61dc74
commit 2bc9c2f424
16 changed files with 109 additions and 9 deletions
@@ -1,5 +1,5 @@
/* /*
* Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@@ -75,7 +75,7 @@ public class ImportGlobNode extends AbstractImportNode {
CompilerDirectives.transferToInterpreterAndInvalidate(); CompilerDirectives.transferToInterpreterAndInvalidate();
var context = VmContext.get(this); var context = VmContext.get(this);
try { try {
var moduleKey = context.getModuleResolver().resolve(importUri); var moduleKey = context.getModuleResolver().resolve(importUri, this);
if (!moduleKey.isGlobbable()) { if (!moduleKey.isGlobbable()) {
throw exceptionBuilder() throw exceptionBuilder()
.evalError("cannotGlobUri", importUri, importUri.getScheme()) .evalError("cannotGlobUri", importUri, importUri.getScheme())
@@ -173,7 +173,7 @@ public final class ModuleKeyFactories {
private static class File implements ModuleKeyFactory { private static class File implements ModuleKeyFactory {
@Override @Override
public Optional<ModuleKey> create(URI uri) { public Optional<ModuleKey> create(URI uri) throws URISyntaxException {
// skip loading providers if the scheme is `file`. // skip loading providers if the scheme is `file`.
if (uri.getScheme().equalsIgnoreCase("file")) { if (uri.getScheme().equalsIgnoreCase("file")) {
return Optional.of(ModuleKeys.file(uri)); return Optional.of(ModuleKeys.file(uri));
@@ -25,8 +25,10 @@ import java.net.URISyntaxException;
import java.net.http.HttpRequest; import java.net.http.HttpRequest;
import java.net.http.HttpResponse.BodyHandlers; import java.net.http.HttpResponse.BodyHandlers;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import org.pkl.core.PklBugException;
import org.pkl.core.SecurityManager; import org.pkl.core.SecurityManager;
import org.pkl.core.SecurityManagerException; import org.pkl.core.SecurityManagerException;
import org.pkl.core.externalreader.ExternalModuleResolver; import org.pkl.core.externalreader.ExternalModuleResolver;
@@ -90,10 +92,20 @@ public final class ModuleKeys {
} }
/** Creates a module key for a {@code file:} module. */ /** Creates a module key for a {@code file:} module. */
public static ModuleKey file(URI uri) { public static ModuleKey file(URI uri) throws URISyntaxException {
return new File(uri); return new File(uri);
} }
/** Creates a module key for a {@code file:} module. */
public static ModuleKey file(Path path) {
try {
return new File(path.toAbsolutePath().toUri());
} catch (URISyntaxException e) {
// impossible, we started with a path to begin with.
throw PklBugException.unreachableCode();
}
}
/** /**
* Creates a module key for a {@code modulepath:} module to be resolved with the given resolver. * Creates a module key for a {@code modulepath:} module to be resolved with the given resolver.
*/ */
@@ -299,7 +311,8 @@ public final class ModuleKeys {
private static class File implements ModuleKey { private static class File implements ModuleKey {
final URI uri; final URI uri;
File(URI uri) { File(URI uri) throws URISyntaxException {
IoUtils.validateFileUri(uri);
this.uri = uri; this.uri = uri;
} }
@@ -1,5 +1,5 @@
/* /*
* Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved. * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@@ -443,7 +443,7 @@ public final class ProjectPackager {
private @Nullable List<ImportsAndReadsParser.Entry> getImportsAndReads(Path pklModulePath) { private @Nullable List<ImportsAndReadsParser.Entry> getImportsAndReads(Path pklModulePath) {
try { try {
var moduleKey = ModuleKeys.file(pklModulePath.toUri()); var moduleKey = ModuleKeys.file(pklModulePath);
var resolvedModuleKey = ResolvedModuleKeys.file(moduleKey, moduleKey.getUri(), pklModulePath); var resolvedModuleKey = ResolvedModuleKeys.file(moduleKey, moduleKey.getUri(), pklModulePath);
return ImportsAndReadsParser.parse(moduleKey, resolvedModuleKey); return ImportsAndReadsParser.parse(moduleKey, resolvedModuleKey);
} catch (IOException e) { } catch (IOException e) {
@@ -254,6 +254,12 @@ public final class ResourceReaders {
private static final class FileResource extends UrlResource { private static final class FileResource extends UrlResource {
static final ResourceReader INSTANCE = new FileResource(); static final ResourceReader INSTANCE = new FileResource();
@Override
public Optional<Object> read(URI uri) throws IOException, URISyntaxException {
IoUtils.validateFileUri(uri);
return super.read(uri);
}
@Override @Override
public String getUriScheme() { public String getUriScheme() {
return "file"; return "file";
@@ -324,7 +330,7 @@ public final class ResourceReaders {
private abstract static class UrlResource implements ResourceReader { private abstract static class UrlResource implements ResourceReader {
@Override @Override
public Optional<Object> read(URI uri) throws IOException { public Optional<Object> read(URI uri) throws IOException, URISyntaxException {
if (HttpUtils.isHttpUrl(uri)) { if (HttpUtils.isHttpUrl(uri)) {
var httpClient = VmContext.get(null).getHttpClient(); var httpClient = VmContext.get(null).getHttpClient();
var request = HttpRequest.newBuilder(uri).build(); var request = HttpRequest.newBuilder(uri).build();
@@ -436,7 +436,8 @@ public final class GlobResolver {
/** Split a glob pattern into the base, non-wildcard parts, and the wildcard parts. */ /** Split a glob pattern into the base, non-wildcard parts, and the wildcard parts. */
private static Pair<String, String[]> splitGlobPatternIntoBaseAndWildcards( private static Pair<String, String[]> splitGlobPatternIntoBaseAndWildcards(
ReaderBase reader, String globPattern, boolean hasAbsoluteGlob) { ReaderBase reader, String globPattern, boolean hasAbsoluteGlob)
throws InvalidGlobPatternException {
var effectiveGlobPattern = globPattern; var effectiveGlobPattern = globPattern;
var basePathSb = new StringBuilder(); var basePathSb = new StringBuilder();
if (hasAbsoluteGlob) { if (hasAbsoluteGlob) {
@@ -446,6 +447,10 @@ public final class GlobResolver {
basePathSb.append(IoUtils.stripFragment(globUri)).append('#'); basePathSb.append(IoUtils.stripFragment(globUri)).append('#');
} else { } else {
effectiveGlobPattern = globUri.getPath(); effectiveGlobPattern = globUri.getPath();
if (effectiveGlobPattern == null) {
throw new InvalidGlobPatternException(
ErrorMessages.create("invalidGlobNonHierarchicalUri", globUri.getScheme()));
}
basePathSb.append(globUri.getScheme()).append(':'); basePathSb.append(globUri.getScheme()).append(':');
} }
} }
@@ -835,4 +835,10 @@ public final class IoUtils {
} }
return index; return index;
} }
public static void validateFileUri(URI uri) throws URISyntaxException {
if (!uri.getSchemeSpecificPart().startsWith("/")) {
throw new URISyntaxException(uri.toString(), ErrorMessages.create("invalidOpaqueFileUri"));
}
}
} }
@@ -661,6 +661,9 @@ Invalid escape character `\\{0}`.
invalidGlobTooComplex=\ invalidGlobTooComplex=\
The glob pattern is too complex. The glob pattern is too complex.
invalidGlobNonHierarchicalUri=\
Scheme `{0}` requires a hierarchical path (there must be a `/` after the first colon).
# used as {1} in invalidModuleUri and invalidResourceUri # used as {1} in invalidModuleUri and invalidResourceUri
invalidTripleDotSyntax=\ invalidTripleDotSyntax=\
expected `...` or `.../path/to/my_module.pkl` expected `...` or `.../path/to/my_module.pkl`
@@ -1057,3 +1060,7 @@ External {0} reader does not support scheme `{1}`.
externalReaderAlreadyTerminated=\ externalReaderAlreadyTerminated=\
External reader process has already terminated. External reader process has already terminated.
invalidOpaqueFileUri=\
File URIs must have a path that starts with `/` (e.g. file:/path/to/my_module.pkl).\n\
To resolve relative paths, remove the scheme prefix (remove "file:").
@@ -0,0 +1 @@
res = read("file:path/to/foo.txt")
@@ -0,0 +1 @@
res = read*("file:path/to/foo.txt")
@@ -0,0 +1 @@
res = import("file:path/to/foo.pkl")
@@ -0,0 +1,9 @@
// Ideally, this should have the same error message as `invalidFileUri2`; the error is that the glob
// pattern is invalid.
//
// But, this is somewhat challenging to fix, because the `URISyntaxException` gets thrown before
// `ImportGlobNode` is initialized.
//
// Regardless, this error is good enough (given this error message, users know what to do), and we
// can afford to be pragmatic here.
res = import*("file:path/to/foo.pkl")
@@ -0,0 +1,13 @@
–– Pkl Error ––
Resource URI `file:path/to/foo.txt` has invalid syntax.
x | res = read("file:path/to/foo.txt")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at invalidFileUri1#res (file:///$snippetsDir/input/errors/invalidFileUri1.pkl)
File URIs must have a path that starts with `/` (e.g. file:/path/to/my_module.pkl).
To resolve relative paths, remove the scheme prefix (remove "file:").
xxx | text = renderer.renderDocument(value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at pkl.base#Module.output.text (pkl:base)
@@ -0,0 +1,12 @@
–– Pkl Error ––
Invalid glob pattern `file:path/to/foo.txt`.
x | res = read*("file:path/to/foo.txt")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at invalidFileUri2#res (file:///$snippetsDir/input/errors/invalidFileUri2.pkl)
Scheme `file` requires a hierarchical path (there must be a `/` after the first colon).
xxx | text = renderer.renderDocument(value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at pkl.base#Module.output.text (pkl:base)
@@ -0,0 +1,13 @@
–– Pkl Error ––
Module URI `file:path/to/foo.pkl` has invalid syntax.
x | res = import("file:path/to/foo.pkl")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at invalidFileUri3#res (file:///$snippetsDir/input/errors/invalidFileUri3.pkl)
File URIs must have a path that starts with `/` (e.g. file:/path/to/my_module.pkl).
To resolve relative paths, remove the scheme prefix (remove "file:").
xxx | text = renderer.renderDocument(value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at pkl.base#Module.output.text (pkl:base)
@@ -0,0 +1,13 @@
–– Pkl Error ––
Module URI `file:path/to/foo.pkl` has invalid syntax.
x | res = import*("file:path/to/foo.pkl")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at invalidFileUri4#res (file:///$snippetsDir/input/errors/invalidFileUri4.pkl)
File URIs must have a path that starts with `/` (e.g. file:/path/to/my_module.pkl).
To resolve relative paths, remove the scheme prefix (remove "file:").
xxx | text = renderer.renderDocument(value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at pkl.base#Module.output.text (pkl:base)