Resolve relative paths in symlinks correctly

Fixes #153
marschall · Dec 29, 2023 · f52e47c · f52e47c
1 parent c1ad849
commit f52e47c
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 30 deletions.
diff --git a/src/main/java/com/github/marschall/memoryfilesystem/MemoryFileSystem.java b/src/main/java/com/github/marschall/memoryfilesystem/MemoryFileSystem.java
@@ -604,21 +604,8 @@ private AbstractPath toRealPath(MemoryDirectory root, AbstractPath path, Set<Mem
             if (!encounteredLinks.add(link)) {
               throw new FileSystemLoopException(path.toString());
             }
-            Path symLinkTarget = link.getTarget();
-            Path newLookUpPath;
-            if (symLinkTarget.isAbsolute()) {
-              newLookUpPath = symLinkTarget;
-            } else {
-              newLookUpPath = path.getRoot();
-              for (int j = 0; j < i; ++j) {
-                newLookUpPath = newLookUpPath.resolve(nameElements.get(j));
-              }
-              newLookUpPath = newLookUpPath.resolve(symLinkTarget);
-            }
-            for (int j = i + 1; j < pathElementCount; ++j) {
-              newLookUpPath = newLookUpPath.resolve(nameElements.get(j));
-            }
-            return this.toRealPath(root, (AbstractPath) newLookUpPath, encounteredLinks, followSymLinks);
+            Path newLookUpPath = this.resolveSymlink(path, link, nameElements, pathElementCount, i);
+            return this.toRealPath(root, (AbstractPath) newLookUpPath.normalize(), encounteredLinks, followSymLinks);
           }
 
           if (current instanceof MemoryDirectory) {
@@ -787,19 +774,7 @@ private <R> R withLockDo(MemoryDirectory root, AbstractPath path, Set<MemorySymb
             if (!encounteredLinks.add(link)) {
               throw new FileSystemLoopException(path.toString());
             }
-            Path symLinkTarget = link.getTarget();
-            if (symLinkTarget.isAbsolute()) {
-              newLookUpPath = symLinkTarget;
-            } else {
-              newLookUpPath = path.getRoot();
-              for (int j = 0; j < i; ++j) {
-                newLookUpPath = newLookUpPath.resolve(nameElements.get(j));
-              }
-              newLookUpPath = newLookUpPath.resolve(symLinkTarget);
-            }
-            for (int j = i + 1; j < pathElementCount; ++j) {
-              newLookUpPath = newLookUpPath.resolve(nameElements.get(j));
-            }
+            newLookUpPath = this.resolveSymlink(path, link, nameElements, pathElementCount, i);
             break;
           }
 
@@ -821,14 +796,38 @@ private <R> R withLockDo(MemoryDirectory root, AbstractPath path, Set<MemorySymb
       if (newLookUpPath == null) {
         return result;
       } else {
-        return this.withLockDo(root, (AbstractPath) newLookUpPath, encounteredLinks, followSymLinks, lockType, callback);
+        return this.withLockDo(root, (AbstractPath) newLookUpPath.normalize(), encounteredLinks, followSymLinks, lockType, callback);
       }
 
     } else {
       throw new IllegalArgumentException("unknown path type" + path);
     }
   }
 
+  private Path resolveSymlink(AbstractPath path, MemorySymbolicLink link, List<String> nameElements, int pathElementCount, int currentNameElementIndex) {
+    Path newLookUpPath;
+    Path symLinkTarget = link.getTarget();
+    if (symLinkTarget.isAbsolute()) {
+      newLookUpPath = symLinkTarget;
+      // we can't return and have to keep appending
+    } else {
+      // start an entire new lookup
+      // start from the root
+      newLookUpPath = path.getRoot();
+      for (int j = 0; j < currentNameElementIndex; ++j) {
+        // append everything we traversed so far
+        newLookUpPath = newLookUpPath.resolve(nameElements.get(j));
+      }
+      // append the relative symlink
+      newLookUpPath = newLookUpPath.resolve(symLinkTarget);
+    }
+    // append the elements not yet traversed
+    for (int j = currentNameElementIndex + 1; j < pathElementCount; ++j) {
+      newLookUpPath = newLookUpPath.resolve(nameElements.get(j));
+    }
+    return newLookUpPath;
+  }
+
   private MemoryDirectory getRootDirectory(AbstractPath path) throws IOException {
     Path root = path.getRoot();
     MemoryDirectory directory = this.roots.get(root);

diff --git a/src/test/java/com/github/marschall/memoryfilesystem/FileSystemCompatibilityTest.java b/src/test/java/com/github/marschall/memoryfilesystem/FileSystemCompatibilityTest.java
@@ -1,6 +1,7 @@
 package com.github.marschall.memoryfilesystem;
 
 import static com.github.marschall.memoryfilesystem.FileContentsMatcher.hasContents;
+import static com.github.marschall.memoryfilesystem.FileExistsMatcher.exists;
 import static com.github.marschall.memoryfilesystem.FileUtility.setContents;
 import static com.github.marschall.memoryfilesystem.IsAbsoluteMatcher.isAbsolute;
 import static com.github.marschall.memoryfilesystem.IsAbsoluteMatcher.isRelative;
@@ -309,4 +310,52 @@ void newDirectoryStreamFollowSymlinks(boolean useDefault) throws IOException {
     }
   }
 
+  @CompatibilityTest
+  void manyDots(boolean useDefault) throws IOException {
+    FileSystem fileSystem = this.getFileSystem(useDefault);
+    Path target = fileSystem.getPath("target");
+    if (!useDefault) {
+      Files.createDirectory(target);
+    }
+    int dotDotCount = target.toAbsolutePath().getNameCount() + 1;
+    String lotsOfDotDot = String.join("/", ParentReferenceList.create(dotDotCount));
+    Path shouldBeRoot = target.resolve(lotsOfDotDot);
+    assertThat(shouldBeRoot, exists());
+    assertEquals(target.toAbsolutePath().getRoot(), shouldBeRoot.toAbsolutePath().normalize());
+  }
+
+  @CompatibilityTest
+  void relativeSymlinks(boolean useDefault) throws IOException {
+    FileSystem fileSystem = this.getFileSystem(useDefault);
+    Path target = fileSystem.getPath("target");
+    if (!useDefault) {
+      Files.createDirectory(target);
+    }
+    Path symlinktests = Files.createDirectory(target.resolve("symlinktests"));
+    Path root = Files.createDirectory(symlinktests.resolve("root"));
+    Path directory1 = Files.createDirectory(root.resolve("directory1"));
+    Path directory2 = Files.createDirectory(root.resolve("directory2"));
+    assertThat(directory1, exists());
+    assertThat(directory2, exists());
+
+    Path link = null;
+    try {
+      Path originalLinkTarget = fileSystem.getPath("root/directory1/./../directory2");
+      link = Files.createSymbolicLink(symlinktests.resolve("link"), originalLinkTarget);
+      Path actualLinkTarget = Files.readSymbolicLink(link);
+      assertEquals(originalLinkTarget, actualLinkTarget);
+      assertThat(link, exists());
+      assertEquals(directory2.toAbsolutePath(), link.toRealPath());
+      //      Files.delete(link);
+    } finally {
+      Files.deleteIfExists(directory1);
+      Files.deleteIfExists(directory2);
+      if (link != null) {
+        Files.deleteIfExists(link);
+      }
+      Files.deleteIfExists(root);
+      Files.deleteIfExists(symlinktests);
+    }
+  }
+
 }
diff --git a/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemTest.java b/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemTest.java
@@ -686,7 +686,7 @@ void transferFromLarger() throws IOException {
     assertEquals(expectedSize, Files.size(from));
 
     try (SeekableByteChannel fromChannel = Files.newByteChannel(from);
-         FileChannel toChannel = FileChannel.open(to, READ, WRITE, CREATE_NEW)) {
+            FileChannel toChannel = FileChannel.open(to, READ, WRITE, CREATE_NEW)) {
       long count = toChannel.transferFrom(fromChannel, 0, content.length * 2);
 
       assertEquals(expectedSize, count);
@@ -1979,6 +1979,15 @@ void outputStreamDoubleClose() throws IOException {
     }
   }
 
+  @Test
+  void inputStreamNormalizePath() throws IOException {
+    Path file = this.extension.getFileSystem().getPath("/file.txt");
+    Files.createFile(file);
+    try (InputStream inputStream = Files.newInputStream(this.extension.getFileSystem().getPath("/./file.txt/../file.txt"), READ)) {
+      assertNotNull(inputStream);
+    }
+  }
+
   @Test
   void byteChannelDoubleClose() throws IOException {
     // regression test for
@@ -2031,6 +2040,15 @@ void channelWriteTruncateExisting() throws IOException {
     }
   }
 
+  @Test
+  void channelNormalizePath() throws IOException {
+    Path file = this.extension.getFileSystem().getPath("/file.txt");
+    Files.createFile(file);
+    try (SeekableByteChannel channel = Files.newByteChannel(this.extension.getFileSystem().getPath("/./file.txt/../file.txt"), WRITE)) {
+      assertNotNull(channel);
+    }
+  }
+
   @Test
   void outputStreamWriteTruncateExisting() throws IOException {
     Path file = this.extension.getFileSystem().getPath("/file.txt");
@@ -2042,6 +2060,14 @@ void outputStreamWriteTruncateExisting() throws IOException {
     }
   }
 
+  @Test
+  void outputStreamNormalizePath() throws IOException {
+    Path file = this.extension.getFileSystem().getPath("/file.txt");
+    Files.createFile(file);
+    try (OutputStream outputStream = Files.newOutputStream(this.extension.getFileSystem().getPath("/./file.txt/../file.txt"), WRITE)) {
+      assertNotNull(outputStream);
+    }
+  }
 
   @Test
   void fromUriSingleSlash() throws IOException {