Merge pull request #1173 from atom/wl-no-recursive-copying

Do not allow moving a folder into itself

lib/tree-view.coffee

@@ -702,7 +702,7 @@ class TreeView
     window.localStorage.removeItem('tree-view:cutPath')
     window.localStorage['tree-view:copyPath'] = JSON.stringify(selectedPaths)
 
-  # Public: Copy the path of the selected entry element.
+  # Public: Cut the path of the selected entry element.
   #         Save the path in localStorage, so that cutting from 2 different
   #         instances of atom works as intended
   #
@@ -740,6 +740,15 @@ class TreeView
         basePath = path.dirname(basePath) if selectedEntry.classList.contains('file')
         newPath = path.join(basePath, path.basename(initialPath))
 
+        # Do not allow copying test/a/ into test/a/b/
+        # Note: A trailing path.sep is added to prevent false positives, such as test/a -> test/ab
+        realBasePath = fs.realpathSync(basePath) + path.sep
+        realInitialPath = fs.realpathSync(initialPath) + path.sep
+        if initialPathIsDirectory and realBasePath.startsWith(realInitialPath)
+          unless fs.isSymbolicLinkSync(initialPath)
+            atom.notifications.addWarning('Cannot paste a folder into itself')
+            continue
+
         if copiedPaths
           # append a number to the file if an item with the same name exists
           fileCounter = 0
@@ -753,7 +762,7 @@ class TreeView
               newPath = "#{filePath}#{fileCounter}#{extension}"
             fileCounter += 1
 
-          if fs.isDirectorySync(initialPath)
+          if initialPathIsDirectory
             # use fs.copy to copy directories since read/write will fail for directories
             catchAndShowFileErrors =>
               fs.copySync(initialPath, newPath)
@@ -764,9 +773,8 @@ class TreeView
               fs.writeFileSync(newPath, fs.readFileSync(initialPath))
               @emitter.emit 'entry-copied', {initialPath, newPath}
         else if cutPaths
-          # Only move the target if the cut target doesn't exist and if the newPath
-          # is not within the initial path
-          unless fs.existsSync(newPath) or newPath.startsWith(initialPath)
+          # Only move the target if the cut target doesn't exist
+          unless fs.existsSync(newPath)
             try
               @emitter.emit 'will-move-entry', {initialPath, newPath}
               fs.moveSync(initialPath, newPath)
@@ -858,6 +866,13 @@ class TreeView
     if initialPath is newDirectoryPath
       return
 
+    realNewDirectoryPath = fs.realpathSync(newDirectoryPath) + path.sep
+    realInitialPath = fs.realpathSync(initialPath) + path.sep
+    if fs.isDirectorySync(initialPath) and realNewDirectoryPath.startsWith(realInitialPath)
+      unless fs.isSymbolicLinkSync(initialPath)
+        atom.notifications.addWarning('Cannot move a folder into itself')
+        return
+
     entryName = path.basename(initialPath)
     newPath = path.join(newDirectoryPath, entryName)
 
@@ -1105,11 +1120,13 @@ class TreeView
         return if initialPaths.includes(newDirectoryPath)
 
         entry.classList.remove('drag-over', 'selected')
-        parentSelected = entry.parentNode.closest('.entry.selected')
-        return if parentSelected
 
         # iterate backwards so files in a dir are moved before the dir itself
         for initialPath in initialPaths by -1
+          # Note: this is necessary on Windows to circumvent node-pathwatcher
+          # holding a lock on expanded folders and preventing them from
+          # being moved or deleted
+          # TODO: This can be removed when tree-view is switched to @atom/watcher
           @entryForPath(initialPath)?.collapse?()
           @moveEntry(initialPath, newDirectoryPath)
       else

spec/tree-view-package-spec.coffee

@@ -1539,43 +1539,69 @@ describe "TreeView", ->
 
       beforeEach ->
         LocalStorage.clear()
+        atom.notifications.clear()
 
-      describe "when attempting to paste a directory into itself", ->
-        describe "when copied", ->
-          beforeEach ->
-            LocalStorage['tree-view:copyPath'] = JSON.stringify([dirPath])
-
-          it "makes a copy inside itself", ->
+      for operation in ['copy', 'cut']
+        describe "when attempting to #{operation} and paste a directory into itself", ->
+          it "shows a warning notification and does not paste", ->
+            # /dir-1/ -> /dir-1/
+            LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath])
             newPath = path.join(dirPath, path.basename(dirPath))
             dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1}))
             expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow()
-            expect(fs.existsSync(newPath)).toBeTruthy()
-
-          it "dispatches an event to the tree-view", ->
-            newPath = path.join(dirPath, path.basename(dirPath))
-            callback = jasmine.createSpy("onEntryCopied")
-            treeView.onEntryCopied(callback)
-
-            dirView.click()
-            atom.commands.dispatch(treeView.element, "tree-view:paste")
-            expect(callback).toHaveBeenCalledWith(initialPath: dirPath, newPath: newPath)
+            expect(fs.existsSync(newPath)).toBe false
+            expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself'
+
+        describe "when attempting to #{operation} and paste a directory into a nested child directory", ->
+          it "shows a warning notification and does not paste", ->
+            nestedPath = path.join(dirPath, 'nested')
+            fs.makeTreeSync(nestedPath)
+
+            # /dir-1/ -> /dir-1/nested/
+            LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath])
+            newPath = path.join(nestedPath, path.basename(dirPath))
+            dirView.reload()
+            nestedView = dirView.querySelector('.directory')
+            nestedView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1}))
+            expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow()
+            expect(fs.existsSync(newPath)).toBe false
+            expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself'
+
+        describe "when attempting to #{operation} and paste a directory into a sibling directory that starts with the same letter", ->
+          it "allows the paste to occur", ->
+            # /dir-1/ -> /dir-2/
+            LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath])
+            newPath = path.join(dirPath2, path.basename(dirPath))
+            dirView2.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1}))
+            expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow()
+            expect(fs.existsSync(newPath)).toBe true
+            expect(atom.notifications.getNotifications()[0]).toBeUndefined()
 
-          it 'does not keep copying recursively', ->
-            LocalStorage['tree-view:copyPath'] = JSON.stringify([dirPath])
-            dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1}))
+        describe "when attempting to #{operation} and paste a directory into a symlink of itself", ->
+          it "shows a warning notification and does not paste", ->
+            fs.symlinkSync(dirPath, path.join(rootDirPath, 'symdir'), 'junction')
 
+            # /dir-1/ -> symlink of /dir-1/
+            LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath])
             newPath = path.join(dirPath, path.basename(dirPath))
+            symlinkView = root1.querySelector('.directory')
+            symlinkView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1}))
             expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow()
-            expect(fs.existsSync(newPath)).toBeTruthy()
-            expect(fs.existsSync(path.join(newPath, path.basename(dirPath)))).toBeFalsy()
+            expect(fs.existsSync(newPath)).toBe false
+            expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself'
 
-        describe "when cut", ->
-          it "does nothing", ->
-            LocalStorage['tree-view:cutPath'] = JSON.stringify([dirPath])
-            dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1}))
+        describe "when attempting to #{operation} and paste a symlink into its target directory", ->
+          it "allows the paste to occur", ->
+            symlinkedPath = path.join(rootDirPath, 'symdir')
+            fs.symlinkSync(dirPath, symlinkedPath, 'junction')
 
-            expect(fs.existsSync(dirPath)).toBeTruthy()
-            expect(fs.existsSync(path.join(dirPath, path.basename(dirPath)))).toBeFalsy()
+            # symlink of /dir-1/ -> /dir-1/
+            LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([symlinkedPath])
+            newPath = path.join(dirPath, path.basename(symlinkedPath))
+            dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1}))
+            expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow()
+            expect(fs.existsSync(newPath)).toBe true
+            expect(atom.notifications.getNotifications()[0]).toBeUndefined()
 
       describe "when pasting entries which don't exist anymore", ->
         it "skips the entry which doesn't exist", ->
@@ -3665,7 +3691,7 @@ describe "TreeView", ->
           entries: new Map())
 
   describe "Dragging and dropping files", ->
-    [alphaDirPath, betaFilePath, etaDirPath, gammaDirPath, deltaFilePath, epsilonFilePath, thetaFilePath] = []
+    [alphaDirPath, alphaFilePath, betaFilePath, etaDirPath, gammaDirPath, deltaFilePath, epsilonFilePath, thetaFilePath] = []
 
     beforeEach ->
       rootDirPath = fs.absolute(temp.mkdirSync('tree-view'))
@@ -3683,6 +3709,10 @@ describe "TreeView", ->
       thetaDirPath = path.join(gammaDirPath, "theta")
       thetaFilePath = path.join(thetaDirPath, "theta.txt")
 
+      alpha2DirPath = path.join(rootDirPath, "alpha2")
+
+      symlinkToAlphaDirPath = path.join(rootDirPath, "symalpha")
+
       fs.writeFileSync(alphaFilePath, "doesn't matter")
       fs.writeFileSync(zetaFilePath, "doesn't matter")
 
@@ -3696,7 +3726,12 @@ describe "TreeView", ->
       fs.makeTreeSync(thetaDirPath)
       fs.writeFileSync(thetaFilePath, "doesn't matter")
 
+      fs.makeTreeSync(alpha2DirPath)
+
+      fs.symlinkSync(alphaDirPath, symlinkToAlphaDirPath, 'junction')
+
       atom.project.setPaths([rootDirPath])
+      atom.notifications.clear()
 
     describe "when dragging a FileView onto a DirectoryView's header", ->
       it "should add the selected class to the DirectoryView", ->
@@ -3884,7 +3919,7 @@ describe "TreeView", ->
     describe "when dropping a DirectoryView and FileViews onto a DirectoryView's header", ->
       it "should move the files and directory to the hovered directory", ->
         # Dragging alpha.txt and alphaDir into thetaDir
-        alphaFile = treeView.roots[0].entries.children[2]
+        alphaFile = Array.from(treeView.roots[0].entries.children).find (element) -> element.getPath() is alphaFilePath
         alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha')
         alphaDir.expand()
 
@@ -3976,6 +4011,76 @@ describe "TreeView", ->
             expect(editors[0].getPath()).toBe thetaFilePath.replace('gamma', 'alpha')
             expect(editors[1].getPath()).toBe thetaFilePath2
 
+      it "shows a warning notification and does not move the directory if it would result in recursive copying", ->
+        # Dragging alphaDir onto etaDir, which is a child of alphaDir's
+        alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha')
+        alphaDir.expand()
+
+        etaDir = alphaDir.entries.children[0]
+        etaDir.expand()
+
+        [dragStartEvent, dragEnterEvent, dropEvent] =
+          eventHelpers.buildInternalDragEvents([alphaDir], etaDir.querySelector('.header'), etaDir, treeView)
+        treeView.onDragStart(dragStartEvent)
+        treeView.onDrop(dropEvent)
+        expect(etaDir.children.length).toBe 2
+        etaDir.expand()
+        expect(etaDir.querySelector('.entries').children.length).toBe 0
+
+        expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot move a folder into itself'
+
+      it "shows a warning notification and does not move the directory if it would result in recursive copying (symlink)", ->
+        # Dragging alphaDir onto symalpha, which is a symlink to alphaDir
+        alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha')
+        alphaDir.expand()
+
+        symlinkDir = treeView.roots[0].entries.children[3]
+        symlinkDir.expand()
+
+        [dragStartEvent, dragEnterEvent, dropEvent] =
+          eventHelpers.buildInternalDragEvents([alphaDir], symlinkDir.querySelector('.header'), symlinkDir, treeView)
+        treeView.onDragStart(dragStartEvent)
+        treeView.onDrop(dropEvent)
+        expect(symlinkDir.children.length).toBe 2
+        symlinkDir.expand()
+        expect(symlinkDir.querySelector('.entries').children.length).toBe 2
+
+        expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot move a folder into itself'
+
+      it "moves successfully when dragging a directory onto a sibling directory that starts with the same letter", ->
+        # Dragging alpha onto alpha2, which is a sibling of alpha's
+        alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha')
+        alphaDir.expand()
+
+        alpha2Dir = findDirectoryContainingText(treeView.roots[0], 'alpha2')
+        [dragStartEvent, dragEnterEvent, dropEvent] =
+          eventHelpers.buildInternalDragEvents([alphaDir], alpha2Dir.querySelector('.header'), alpha2Dir, treeView)
+        treeView.onDragStart(dragStartEvent)
+        treeView.onDrop(dropEvent)
+        expect(alpha2Dir.children.length).toBe 2
+        alpha2Dir.expand()
+        expect(alpha2Dir.querySelector('.entries').children.length).toBe 1
+
+        expect(atom.notifications.getNotifications()[0]).toBeUndefined()
+
+      it "moves successfully when dragging a symlink into its target directory", ->
+        # Dragging alphaDir onto symalpha, which is a symlink to alphaDir
+        alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha')
+        alphaDir.expand()
+
+        symlinkDir = treeView.roots[0].entries.children[3]
+        symlinkDir.expand()
+
+        [dragStartEvent, dragEnterEvent, dropEvent] =
+          eventHelpers.buildInternalDragEvents([symlinkDir], alphaDir.querySelector('.header'), alphaDir, treeView)
+        treeView.onDragStart(dragStartEvent)
+        treeView.onDrop(dropEvent)
+        expect(alphaDir.children.length).toBe 2
+        alphaDir.reload()
+        expect(alphaDir.querySelector('.entries').children.length).toBe 3
+
+        expect(atom.notifications.getNotifications()[0]).toBeUndefined()
+
     describe "when dropping a DirectoryView and FileViews onto the same DirectoryView's header", ->
       it "should not move the files and directory to the hovered directory", ->
         # Dragging alpha.txt and alphaDir into alphaDir