avoid inconsistencies after crazy renames/moves files onto themselves

platform/platform-impl/src/com/intellij/openapi/vfs/impl/FilePointerPartNode.java

@@ -35,7 +35,7 @@ import java.util.List;
 // all file pointers we store in the tree with nodes corresponding to the file structure on disk
 class FilePointerPartNode {
   private static final FilePointerPartNode[] EMPTY_ARRAY = new FilePointerPartNode[0];
-  @NotNull private String part; // common prefix of all file pointers beneath
+  @NotNull String part; // common prefix of all file pointers beneath
   @NotNull FilePointerPartNode[] children;
   FilePointerPartNode parent;
   // file pointers for this exact path (e.g. concatenation of all "part" fields down from the root).
@@ -69,8 +69,6 @@ class FilePointerPartNode {
                        boolean separator,
                        @NotNull CharSequence childName,
                        @NotNull FilePointerPartNode[] outNode) {
-    checkConsistency();
-
     int partStart;
     if (parent == null) {
       partStart = 0;
@@ -137,20 +135,30 @@ class FilePointerPartNode {
   private static final boolean UNIT_TEST = ApplicationManager.getApplication().isUnitTestMode();
   void checkConsistency() {
     if (UNIT_TEST && !ApplicationInfoImpl.isInPerformanceTest()) {
-      doCheckConsistency();
+      doCheckConsistency(false);
     }
   }
 
-  private void doCheckConsistency() {
+  private void doCheckConsistency(boolean dotDotOccurred) {
+    dotDotOccurred |= part.contains(".."); // part must not contain ".." (except when the file pointer was created from URL with ".." inside)
     int childSum = 0;
     for (FilePointerPartNode child : children) {
       childSum += child.pointersUnder;
-      child.doCheckConsistency();
+      child.doCheckConsistency(dotDotOccurred);
       assert child.parent == this;
     }
     childSum += leavesNumber();
     assert (useCount == 0) == (leaves == null) : useCount + " - " + (leaves instanceof VirtualFilePointerImpl ? leaves : Arrays.toString((VirtualFilePointerImpl[])leaves));
     assert pointersUnder == childSum : "expected: "+pointersUnder+"; actual: "+childSum;
+    Pair<VirtualFile, String> fileAndUrl = myFileAndUrl;
+    if (fileAndUrl != null && fileAndUrl.second != null) {
+      String url = fileAndUrl.second;
+      assert url.endsWith(part) : "part is: '"+part +"' but url is: '"+url+"'";
+    }
+    boolean hasFile = fileAndUrl != null && fileAndUrl.first != null;
+
+    // when the node contains real file its path should be canonical
+    assert !hasFile || !dotDotOccurred : "Path is not canonical: '"+getUrl()+"'; my part: '"+part+"'";
   }
 
   @NotNull
@@ -331,6 +339,10 @@ class FilePointerPartNode {
     return leaves == null ? null : leaves instanceof VirtualFilePointerImpl ? (VirtualFilePointerImpl)leaves : ((VirtualFilePointerImpl[])leaves)[0];
   }
 
+  private String getUrl() {
+    return parent == null ? part : parent.getUrl() + part;
+  }
+
   private int leavesNumber() {
     Object leaves = this.leaves;
     return leaves == null ? 0 : leaves instanceof VirtualFilePointerImpl ? 1 : ((VirtualFilePointerImpl[])leaves).length;

platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerManagerImpl.java

@@ -199,6 +199,14 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
         url = VirtualFileManager.constructUrl(protocol, cleanPath);
         path = cleanPath;
       }
+      if (url.contains("..")) {
+        // the url of the form "/x/../y" should resolve to "/y" (or something else in the case of symlinks)
+        file = VirtualFileManager.getInstance().findFileByUrl(url);
+        if (file != null) {
+          url = file.getUrl();
+          path = file.getPath();
+        }
+      }
     }
     // else url has come from VirtualFile.getPath() and is good enough
 
@@ -210,12 +218,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
   private final Map<String, IdentityVirtualFilePointer> myUrlToIdentity = new THashMap<>();
   @NotNull
   private IdentityVirtualFilePointer getOrCreateIdentity(@NotNull String url, @Nullable VirtualFile found) {
-    IdentityVirtualFilePointer pointer = myUrlToIdentity.get(url);
-    if (pointer == null) {
-      pointer = new IdentityVirtualFilePointer(found, url);
-      myUrlToIdentity.put(url, pointer);
-    }
-    return pointer;
+    return myUrlToIdentity.computeIfAbsent(url, __ -> new IdentityVirtualFilePointer(found, url));
   }
 
   @NotNull
@@ -463,6 +466,14 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
 
     myNodesToFire = toFireEvents;
     myNodesToUpdateUrl = toUpdateUrl;
+
+    assertConsistency();
+  }
+
+  void assertConsistency() {
+    for (FilePointerPartNode root : myPointers.values()) {
+      root.checkConsistency();
+    }
   }
 
   @Override
@@ -474,7 +485,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
         String urlBefore = node.myFileAndUrl.second;
         Pair<VirtualFile,String> after = node.update();
         String urlAfter = after.second;
-        if (URL_COMPARATOR.compare(urlBefore, urlAfter) != 0) {
+        if (URL_COMPARATOR.compare(urlBefore, urlAfter) != 0 || !urlAfter.endsWith(node.part)) {
           List<VirtualFilePointerImpl> myPointers = new SmartList<>();
           node.addAllPointersTo(myPointers);
 
@@ -514,9 +525,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
     myNodesToUpdateUrl = Collections.emptyList();
     myEvents = Collections.emptyList();
     myNodesToFire = Collections.emptyList();
-    for (FilePointerPartNode root : myPointers.values()) {
-      root.checkConsistency();
-    }
+    assertConsistency();
   }
 
   void removeNode(@NotNull FilePointerPartNode node, VirtualFilePointerListener listener) {
@@ -525,9 +534,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
     if (rootNodeEmpty) {
       myPointers.remove(listener);
     }
-    else {
-      myPointers.get(listener).checkConsistency();
-    }
+    assertConsistency();
   }
 
   @Override
@@ -538,8 +545,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
   }
 
   private static class DelegatingDisposable implements Disposable {
-    private static final ConcurrentMap<Disposable, DelegatingDisposable> ourInstances =
-      ContainerUtil.newConcurrentMap(ContainerUtil.<Disposable>identityStrategy());
+    private static final ConcurrentMap<Disposable, DelegatingDisposable> ourInstances = ContainerUtil.newConcurrentMap(ContainerUtil.<Disposable>identityStrategy());
     private final TObjectIntHashMap<VirtualFilePointerImpl> myCounts = new TObjectIntHashMap<>();
     private final Disposable myParent;
 

platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/VirtualFilePointerTest.java

@@ -45,6 +45,7 @@ import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.ui.UIUtil;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.io.File;
 import java.io.IOException;
@@ -521,7 +522,7 @@ public class VirtualFilePointerTest extends PlatformTestCase {
     }
   }
 
-  private VirtualFilePointer createPointerByFile(final File file, final VirtualFilePointerListener fileListener) throws IOException {
+  private VirtualFilePointer createPointerByFile(@NotNull File file, @Nullable VirtualFilePointerListener fileListener) throws IOException {
     final String url = VirtualFileManager.constructUrl(LocalFileSystem.PROTOCOL, file.getCanonicalPath().replace(File.separatorChar, '/'));
     final VirtualFile vFile = refreshAndFind(url);
     return vFile == null
@@ -721,7 +722,7 @@ public class VirtualFilePointerTest extends PlatformTestCase {
     }).useLegacyScaling().assertTiming();
   }
 
-  public void testCidrConfusions() throws IOException {
+  public void testCidrCrazyAddCreateRenames() throws IOException {
     File tempDirectory = createTempDirectory();
     final VirtualFile root = LocalFileSystem.getInstance().refreshAndFindFileByIoFile(tempDirectory);
 
@@ -737,8 +738,9 @@ public class VirtualFilePointerTest extends PlatformTestCase {
     delete(dir1);
     assertSourceIs(null); // srcDir deleted, no more sources
     assertLibIs(dir2);    // libDir stays the same
-
+    myVirtualFilePointerManager.assertConsistency();
     rename(dir2, "dir1");
+    myVirtualFilePointerManager.assertConsistency();
     assertSourceIs(dir2); // srcDir re-appeared, sources are "dir1"
     assertLibIs(dir2);    // libDir renamed, libs are "dir1" now
 
@@ -837,4 +839,16 @@ public class VirtualFilePointerTest extends PlatformTestCase {
       PsiTestUtil.removeAllRoots(getModule(), null);
     }
   }
+
+  public void testDotDot() throws IOException {
+    File tempDirectory = createTempDirectory();
+    final VirtualFile root = LocalFileSystem.getInstance().refreshAndFindFileByIoFile(tempDirectory);
+
+    VirtualFile dir1 = createChildDirectory(root, "dir1");
+    VirtualFile dir2 = createChildDirectory(root, "dir2");
+    VirtualFile file = createChildData(dir1, "x.txt");
+
+    VirtualFilePointer pointer = myVirtualFilePointerManager.create(dir2.getUrl() + "/../" + dir1.getName() + "/" + file.getName(), disposable, null);
+    assertEquals(file, pointer.getFile());
+  }
 }