Skip to content

Commit 9b7da27

Browse files
tomasstupkatomasstupka
committed
[GR-60061] Handle VFS extracted files like symlinks.
PullRequest: graalpython/3612
2 parents 16c70c6 + ffdf454 commit 9b7da27

File tree

5 files changed

+261
-97
lines changed

5 files changed

+261
-97
lines changed

‎graalpython/com.oracle.graal.python.test/src/org/graalvm/python/embedding/utils/test/VirtualFileSystemTest.java‎

Lines changed: 76 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,15 @@
5555
import java.lang.reflect.Modifier;
5656
import java.net.URI;
5757
import java.nio.ByteBuffer;
58+
import java.nio.channels.NonWritableChannelException;
5859
import java.nio.channels.SeekableByteChannel;
5960
import java.nio.file.AccessMode;
6061
import java.nio.file.DirectoryStream;
6162
import java.nio.file.Files;
63+
import java.nio.file.LinkOption;
6264
import java.nio.file.NoSuchFileException;
6365
import java.nio.file.NotDirectoryException;
66+
import java.nio.file.NotLinkException;
6467
import java.nio.file.OpenOption;
6568
import java.nio.file.Path;
6669
import java.nio.file.StandardCopyOption;
@@ -85,6 +88,7 @@
8588
import static org.graalvm.python.embedding.utils.VirtualFileSystem.HostIO.READ_WRITE;
8689
import static org.junit.Assert.assertEquals;
8790
import static org.junit.Assert.assertFalse;
91+
import static org.junit.Assert.assertNotEquals;
8892
import static org.junit.Assert.assertTrue;
8993
import static org.junit.Assert.fail;
9094

@@ -175,8 +179,10 @@ public void toRealPath() throws Exception {
175179
private static void toRealPathVFS(FileSystem fs, String pathPrefix) throws IOException {
176180
assertEquals(Path.of(VFS_MOUNT_POINT, "dir1"), fs.toRealPath(Path.of(pathPrefix, "dir1")));
177181
assertEquals(Path.of(VFS_MOUNT_POINT, "SomeFile"), fs.toRealPath(Path.of(pathPrefix, "SomeFile")));
178-
assertEquals(Path.of(VFS_MOUNT_POINT, "does-not-exist", "extractme"), fs.toRealPath(Path.of(pathPrefix, "does-not-exist", "extractme")));
182+
assertEquals(Path.of(VFS_MOUNT_POINT, "does-not-exist"), fs.toRealPath(Path.of(pathPrefix, "does-not-exist")));
183+
assertEquals(Path.of(VFS_MOUNT_POINT, "extractme"), fs.toRealPath(Path.of(pathPrefix, "extractme"), LinkOption.NOFOLLOW_LINKS));
179184
checkExtractedFile(fs.toRealPath(Path.of(pathPrefix, "extractme")), new String[]{"text1", "text2"});
185+
checkException(NoSuchFileException.class, () -> fs.toRealPath(Path.of(pathPrefix, "does-not-exist", "extractme")));
180186
}
181187

182188
@Test
@@ -383,10 +389,23 @@ private static void checkAccessVFS(FileSystem fs, String pathPrefix) throws IOEx
383389
fs.checkAccess(Path.of(pathPrefix, "dir1"), Set.of(AccessMode.READ));
384390
// check regular resource file
385391
fs.checkAccess(Path.of(pathPrefix, "SomeFile"), Set.of(AccessMode.READ));
392+
386393
// check to be extracted file
394+
fs.checkAccess(Path.of(pathPrefix, "extractme"), Set.of(AccessMode.READ), LinkOption.NOFOLLOW_LINKS);
395+
checkException(SecurityException.class, () -> fs.checkAccess(Path.of(pathPrefix, "extractme"), Set.of(AccessMode.WRITE), LinkOption.NOFOLLOW_LINKS));
387396
fs.checkAccess(Path.of(pathPrefix, "extractme"), Set.of(AccessMode.READ));
397+
// even though extracted -> FS is read-only and we are limiting the access to read-only also
398+
// for extracted files
399+
checkException(IOException.class, () -> fs.checkAccess(Path.of(pathPrefix, "extractme"), Set.of(AccessMode.WRITE)));
400+
401+
checkException(NoSuchFileException.class, () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exits", "extractme"), Set.of(AccessMode.READ), LinkOption.NOFOLLOW_LINKS));
402+
checkException(NoSuchFileException.class, () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exits", "extractme"), Set.of(AccessMode.READ)));
388403

389404
checkException(SecurityException.class, () -> fs.checkAccess(Path.of(pathPrefix, "SomeFile"), Set.of(AccessMode.WRITE)), "write access should not be possible with VFS");
405+
checkException(SecurityException.class, () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exist"), Set.of(AccessMode.WRITE)), "execute access should not be possible with VFS");
406+
checkException(SecurityException.class, () -> fs.checkAccess(Path.of(pathPrefix, "SomeFile"), Set.of(AccessMode.EXECUTE)), "execute access should not be possible with VFS");
407+
checkException(SecurityException.class, () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exist"), Set.of(AccessMode.EXECUTE)), "execute access should not be possible with VFS");
408+
390409
checkException(NoSuchFileException.class, () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exits"), Set.of(AccessMode.READ)),
391410
"should not be able to access a file which does not exist in VFS");
392411
checkException(NoSuchFileException.class, () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exits", "extractme"), Set.of(AccessMode.READ)),
@@ -485,6 +504,11 @@ public void newByteChannel() throws Exception {
485504

486505
newByteChannelVFS(fs, VFS_MOUNT_POINT);
487506
withCWD(fs, VFS_ROOT_PATH, (fst) -> newByteChannelVFS(fst, ""));
507+
508+
checkException(NullPointerException.class, () -> fs.newByteChannel(Path.of(VFS_MOUNT_POINT, "does-not-exist"), null));
509+
withCWD(fs, VFS_ROOT_PATH, (fst) -> checkException(NullPointerException.class, () -> fst.newByteChannel(Path.of("does-not-exist"), null)));
510+
checkException(NullPointerException.class, () -> fs.newByteChannel(Path.of(VFS_MOUNT_POINT, "does-not-exist", "extractme"), null));
511+
withCWD(fs, VFS_ROOT_PATH, (fst) -> checkException(NullPointerException.class, () -> fst.newByteChannel(Path.of("does-not-exist", "extractme"), null)));
488512
}
489513

490514
// from real FS
@@ -502,25 +526,34 @@ public void newByteChannel() throws Exception {
502526
}
503527

504528
private static void newByteChannelVFS(FileSystem fs, String pathPrefix) throws IOException {
505-
Path path = Path.of(pathPrefix, "file1");
506-
for (StandardOpenOption o : StandardOpenOption.values()) {
529+
Path file1 = Path.of(pathPrefix, "file1");
530+
Path extractable = Path.of(pathPrefix, "extractme");
531+
for (StandardOpenOption o : new StandardOpenOption[]{StandardOpenOption.WRITE, StandardOpenOption.READ}) {
507532
if (o == StandardOpenOption.READ) {
508-
SeekableByteChannel bch = fs.newByteChannel(path, Set.of(o));
509-
ByteBuffer buffer = ByteBuffer.allocate(1024);
510-
bch.read(buffer);
511-
String s = new String(buffer.array());
512-
String[] ss = s.split(System.lineSeparator());
513-
assertTrue(ss.length >= 2);
514-
assertEquals("text1", ss[0]);
515-
assertEquals("text2", ss[1]);
516-
517-
checkException(IOException.class, () -> bch.write(buffer), "should not be able to write to VFS");
518-
checkException(IOException.class, () -> bch.truncate(0), "should not be able to write to VFS");
533+
newByteChannelVFS(fs, file1, Set.of(o));
534+
newByteChannelVFS(fs, file1, Set.of(o, LinkOption.NOFOLLOW_LINKS));
535+
newByteChannelVFS(fs, extractable, Set.of(o));
536+
checkException(IOException.class, () -> fs.newByteChannel(extractable, Set.of(o, LinkOption.NOFOLLOW_LINKS)));
519537
} else {
520-
checkCanOnlyRead(fs, path, o);
538+
checkCanOnlyRead(fs, file1, o);
539+
checkCanOnlyRead(fs, extractable, o);
521540
}
522541
}
523-
checkCanOnlyRead(fs, path, StandardOpenOption.READ, StandardOpenOption.WRITE);
542+
checkCanOnlyRead(fs, file1, StandardOpenOption.READ, StandardOpenOption.WRITE);
543+
checkCanOnlyRead(fs, extractable, StandardOpenOption.READ, StandardOpenOption.WRITE);
544+
}
545+
546+
private static void newByteChannelVFS(FileSystem fs, Path path, Set<OpenOption> options) throws IOException {
547+
SeekableByteChannel bch = fs.newByteChannel(path, options);
548+
ByteBuffer buffer = ByteBuffer.allocate(1024);
549+
bch.read(buffer);
550+
String s = new String(buffer.array());
551+
String[] ss = s.split(System.lineSeparator());
552+
assertTrue(ss.length >= 2);
553+
assertEquals("text1", ss[0]);
554+
assertEquals("text2", ss[1]);
555+
checkException(NonWritableChannelException.class, () -> bch.write(buffer), "should not be able to write to VFS");
556+
checkException(NonWritableChannelException.class, () -> bch.truncate(0), "should not be able to write to VFS");
524557
}
525558

526559
private static void newByteChannelRealFS(FileSystem fs, Path path, String expectedText) throws IOException {
@@ -622,6 +655,19 @@ private static void readAttributesVFS(FileSystem fs, String pathPrefix) throws I
622655
Map<String, Object> attrs = fs.readAttributes(Path.of(pathPrefix, "dir1"), "creationTime");
623656
assertEquals(FileTime.fromMillis(0), attrs.get("creationTime"));
624657

658+
attrs = fs.readAttributes(Path.of(pathPrefix, "extractme"), "creationTime,isSymbolicLink,isRegularFile", LinkOption.NOFOLLOW_LINKS);
659+
assertEquals(FileTime.fromMillis(0), attrs.get("creationTime"));
660+
assertTrue((Boolean) attrs.get("isSymbolicLink"));
661+
assertFalse((Boolean) attrs.get("isRegularFile")); //
662+
663+
attrs = fs.readAttributes(Path.of(pathPrefix, "extractme"), "creationTime,isSymbolicLink,isRegularFile");
664+
assertNotEquals(FileTime.fromMillis(0), attrs.get("creationTime"));
665+
assertFalse((Boolean) attrs.get("isSymbolicLink"));
666+
assertTrue((Boolean) attrs.get("isRegularFile"));
667+
668+
checkException(NoSuchFileException.class, () -> fs.readAttributes(Path.of(pathPrefix, "does-not-exist", "extractme"), "creationTime", LinkOption.NOFOLLOW_LINKS));
669+
checkException(NoSuchFileException.class, () -> fs.readAttributes(Path.of(pathPrefix, "does-not-exist", "extractme"), "creationTime"));
670+
625671
checkException(NoSuchFileException.class, () -> fs.readAttributes(Path.of(pathPrefix, "does-not-exist"), "creationTime"), "");
626672
checkException(UnsupportedOperationException.class, () -> fs.readAttributes(Path.of(pathPrefix, "file1"), "unix:creationTime"), "");
627673
}
@@ -876,7 +922,6 @@ public void createAndReadSymbolicLink() throws Exception {
876922
checkException(IOException.class, () -> fs.createSymbolicLink(VFS_ROOT_PATH.resolve("link1"), realFSLinkTarget));
877923

878924
checkException(SecurityException.class, () -> fs.createSymbolicLink(VFS_ROOT_PATH, VFS_ROOT_PATH.resolve("link")));
879-
checkException(SecurityException.class, () -> fs.readSymbolicLink(VFS_ROOT_PATH.resolve("link1")));
880925
}
881926
checkException(SecurityException.class, () -> rHostIOVFS.createSymbolicLink(realFSDir.resolve("link2"), realFSLinkTarget));
882927
checkException(SecurityException.class, () -> noHostIOVFS.createSymbolicLink(realFSDir.resolve("link3"), realFSLinkTarget));
@@ -902,6 +947,20 @@ private void checkSymlink(Path dir, Path target, Path symlink) throws Exception
902947
}
903948
}
904949

950+
@Test
951+
public void readSymbolicLink() throws Exception {
952+
for (FileSystem fs : new FileSystem[]{rwHostIOVFS, rHostIOVFS, noHostIOVFS}) {
953+
readSymbolicLink(fs, VFS_MOUNT_POINT);
954+
withCWD(fs, VFS_ROOT_PATH, (fst) -> readSymbolicLink(fst, ""));
955+
}
956+
}
957+
958+
private static void readSymbolicLink(FileSystem fs, String vfsPrefix) throws IOException {
959+
checkException(NotLinkException.class, () -> fs.readSymbolicLink(Path.of(vfsPrefix, "file1")));
960+
checkException(NoSuchFileException.class, () -> fs.readSymbolicLink(Path.of(vfsPrefix, "does-not-exist")));
961+
checkExtractedFile(fs.readSymbolicLink(Path.of(vfsPrefix, "extractme")), new String[]{"text1", "text2"});
962+
}
963+
905964
@Test
906965
public void move() throws Exception {
907966
Path realFSDir = Files.createTempDirectory("graalpy.vfs.test");

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
import java.io.PrintStream;
5454
import java.lang.invoke.VarHandle;
5555
import java.lang.ref.WeakReference;
56-
import java.nio.file.LinkOption;
5756
import java.util.ArrayList;
5857
import java.util.Arrays;
5958
import java.util.HashMap;
@@ -819,7 +818,7 @@ public static CApiContext ensureCapiWasLoaded(Node node, PythonContext context,
819818
TruffleFile homePath = env.getInternalTruffleFile(context.getCAPIHome().toJavaStringUncached());
820819
// e.g. "libpython-native.so"
821820
String libName = context.getLLVMSupportExt("python");
822-
TruffleFile capiFile = homePath.resolve(libName).getCanonicalFile(LinkOption.NOFOLLOW_LINKS);
821+
TruffleFile capiFile = homePath.resolve(libName).getCanonicalFile();
823822
try {
824823
SourceBuilder capiSrcBuilder;
825824
final boolean useNative = PythonOptions.NativeModules.getValue(env.getOptions());

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/common/CExtContext.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import static com.oracle.graal.python.util.PythonUtils.tsLiteral;
5353

5454
import java.io.IOException;
55-
import java.nio.file.LinkOption;
5655
import java.util.Set;
5756
import java.util.logging.Level;
5857

@@ -320,7 +319,7 @@ public static Object loadCExtModule(Node location, PythonContext context, Module
320319
InteropLibrary interopLib;
321320

322321
if (cApiContext.useNativeBackend) {
323-
TruffleFile realPath = context.getPublicTruffleFileRelaxed(spec.path, context.getSoAbi()).getCanonicalFile(LinkOption.NOFOLLOW_LINKS);
322+
TruffleFile realPath = context.getPublicTruffleFileRelaxed(spec.path, context.getSoAbi()).getCanonicalFile();
324323
getLogger().config(String.format("loading module %s (real path: %s) as native", spec.path, realPath));
325324
String loadExpr = String.format("load(%s) \"%s\"", dlopenFlagsToString(context.getDlopenFlags()), realPath);
326325
if (PythonOptions.UsePanama.getValue(context.getEnv().getOptions())) {
@@ -366,7 +365,7 @@ public static Object loadLLVMLibrary(Node location, PythonContext context, Truff
366365
Env env = context.getEnv();
367366
try {
368367
TruffleString extSuffix = context.getSoAbi();
369-
TruffleFile realPath = context.getPublicTruffleFileRelaxed(path, extSuffix).getCanonicalFile(LinkOption.NOFOLLOW_LINKS);
368+
TruffleFile realPath = context.getPublicTruffleFileRelaxed(path, extSuffix).getCanonicalFile();
370369
CallTarget callTarget = env.parseInternal(Source.newBuilder(J_LLVM_LANGUAGE, realPath).build());
371370
return callTarget.call();
372371
} catch (SecurityException e) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/hpy/jni/GraalHPyJNIContext.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@
5757
import static com.oracle.graal.python.util.PythonUtils.tsLiteral;
5858

5959
import java.io.IOException;
60-
import java.nio.file.LinkOption;
6160
import java.util.Arrays;
6261
import java.util.LinkedList;
6362
import java.util.List;
@@ -461,7 +460,7 @@ public static String getJNILibrary() {
461460
PythonContext context = PythonContext.get(null);
462461
TruffleFile libPath = context.getPublicTruffleFileRelaxed(context.getJNIHome()).resolve(PythonContext.J_PYTHON_JNI_LIBRARY_NAME).getAbsoluteFile();
463462
try {
464-
return libPath.getCanonicalFile(LinkOption.NOFOLLOW_LINKS).toString();
463+
return libPath.getCanonicalFile().toString();
465464
} catch (IOException e) {
466465
LOGGER.severe(String.format("Cannot determine canonical path for %s: %s", libPath, e));
467466
throw new IllegalStateException(e);

0 commit comments

Comments
 (0)