-
Notifications
You must be signed in to change notification settings - Fork 326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
File.new
crashes if unexpected characters are encountered in the path
#9723
Comments
Indeed, on Linux it seems that such weird paths are accepted:
|
Possible fix: diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoFile.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoFile.java
index 5d2de7614..a674321f2 100644
--- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoFile.java
+++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoFile.java
@@ -451,7 +451,12 @@ public final class EnsoFile implements EnsoObject {
return noSuchFileException;
}
- var parent = fromString(EnsoContext.get(null), path).truffleFile.getParent();
+ var parent =
+ switch (fromString(EnsoContext.get(null), path)) {
+ case EnsoFile f -> f.truffleFile.getParent();
+ case null -> null;
+ default -> null;
+ };
// Unknown parent, so the heuristic cannot be applied - return the original.
if (parent == null) {
return noSuchFileException;
@@ -486,7 +491,12 @@ public final class EnsoFile implements EnsoObject {
// On Linux, when creating a directory tree `foo/my-file.txt/a/b/c`, the operation fails with
// `FileSystemException` with the full path (`foo/my-file.txt/a/b/c`). So we need to traverse
// this path to find the actually problematic part.
- var file = fromString(EnsoContext.get(null), path).truffleFile;
+ var file =
+ switch (fromString(EnsoContext.get(null), path)) {
+ case EnsoFile f -> f.truffleFile;
+ case null -> null;
+ default -> null;
+ };
// We try to find the first file that exists on the path.
while (file != null && !file.exists()) {
file = file.getParent();
@@ -630,9 +640,17 @@ public final class EnsoFile implements EnsoObject {
autoRegister = false)
@Builtin.Specialize
@TruffleBoundary
- public static EnsoFile fromString(EnsoContext context, String path) {
- TruffleFile file = context.getPublicTruffleFile(path);
- return new EnsoFile(file);
+ public static EnsoObject fromString(EnsoContext context, String path)
+ throws IllegalArgumentException {
+ try {
+ TruffleFile file = context.getPublicTruffleFile(path);
+ return new EnsoFile(file);
+ } catch (IllegalArgumentException | UnsupportedOperationException ex) {
+ return context
+ .getBuiltins()
+ .error()
+ .makeUnsupportedArgumentsError(new Object[] {Text.create(path)}, ex.getMessage());
+ }
}
@Builtin.Method(
@@ -652,7 +670,7 @@ public final class EnsoFile implements EnsoObject {
autoRegister = false)
@Builtin.Specialize
@TruffleBoundary
- public static EnsoFile userHome(EnsoContext context) {
+ public static EnsoObject userHome(EnsoContext context) {
return fromString(context, System.getProperty("user.home"));
}
diff --git a/lib/java/test-utils/src/main/java/org/enso/test/utils/ProjectUtils.java b/lib/java/test-utils/src/main/java/org/enso/test/utils/ProjectUtils.java
index 878d0b663..b4abccba3 100644
--- a/lib/java/test-utils/src/main/java/org/enso/test/utils/ProjectUtils.java
+++ b/lib/java/test-utils/src/main/java/org/enso/test/utils/ProjectUtils.java
@@ -53,7 +53,8 @@ public class ProjectUtils {
name: %s
version: 0.0.1
prefer-local-libraries: true
- """.formatted(projName);
+ """
+ .formatted(projName);
var yamlPath = projDir.resolve("package.yaml");
Files.writeString(yamlPath, projYaml);
assert yamlPath.toFile().exists();
diff --git a/lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/BuiltinsProcessor.java b/lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/BuiltinsProcessor.java
index 17806a3d2..fde48a191 100644
--- a/lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/BuiltinsProcessor.java
+++ b/lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/BuiltinsProcessor.java
@@ -126,7 +126,8 @@ public class BuiltinsProcessor extends AbstractProcessor {
+ "\")";
out.println(builtinTypeAnnotation);
out.println("public class " + builtinType.name() + " extends Builtin {");
- out.println("""
+ out.println(
+ """
@Override
public boolean containsValues() {
""");
diff --git a/test/Base_Tests/src/System/File_Spec.enso b/test/Base_Tests/src/System/File_Spec.enso
index d14eb3031..c7f54ed73 100644
--- a/test/Base_Tests/src/System/File_Spec.enso
+++ b/test/Base_Tests/src/System/File_Spec.enso
@@ -1,6 +1,7 @@
from Standard.Base import all
import Standard.Base.Errors.Common.Forbidden_Operation
import Standard.Base.Errors.Common.Dry_Run_Operation
+import Standard.Base.Errors.Common.Unsupported_Argument_Types
import Standard.Base.Errors.Encoding_Error.Encoding_Error
import Standard.Base.Errors.File_Error.File_Error
import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
@@ -41,6 +42,10 @@ add_specs suite_builder =
path = sample_file.path
File.new path
+ group_builder.specify "invalid char in path" <|
+ err = File.new "C:\dev:a"
+ err . should_fail_with Unsupported_Argument_Types
+
group_builder.specify "should have `new` be a no-op on a file" <|
file = File.new sample_file
file . should_equal sample_file |
2 tasks
github-project-automation
bot
moved this from 🌟 Q/A review
to 🟢 Accepted
in Issues Board
Jun 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
See the followin REPL session:
Expected behaviour
A harmless dataflow error should be raised, telling that the path is invalid (e.g.
Illegal_Argument
).Actual behaviour
The operation crashes. I think this is not even a panic but interpreter crash.
It may be that this only occurs on Windows as the stack trace hints Windows-specific methods.
The text was updated successfully, but these errors were encountered: