walkdir: avoid symlink loops when follow_symlinks == false (#35006)

* walkdir: avoid symlink loops when `follow_symlinks == false`

Because `isdir()` attempts to dereference symlinks, attempting to
`walkdir()` trees that contain symlink loops errors out. This change
modifies `walkdir()` to treat all symlinks as files when
`follow_symlinks == false`.

* rm: When checking `filemode()`, use `lstat()` to avoid following symlinks

Author: staticfloat

base/file.jl

@@ -261,7 +261,7 @@ function rm(path::AbstractString; force::Bool=false, recursive::Bool=false)
         try
             @static if Sys.iswindows()
                 # is writable on windows actually means "is deletable"
-                if (filemode(path) & 0o222) == 0
+                if (filemode(lstat(path)) & 0o222) == 0
                     chmod(path, 0o777)
                 end
             end
@@ -853,10 +853,13 @@ function walkdir(root; topdown=true, follow_symlinks=false, onerror=throw)
     dirs = Vector{eltype(content)}()
     files = Vector{eltype(content)}()
     for name in content
-        if isdir(joinpath(root, name))
-            push!(dirs, name)
-        else
+        path = joinpath(root, name)
+
+        # If we're not following symlinks, then treat all symlinks as files
+        if (!follow_symlinks && islink(path)) || !isdir(path)
             push!(files, name)
+        else
+            push!(dirs, name)
         end
     end
 

test/file.jl

@@ -1218,8 +1218,18 @@ cd(dirwalk) do
 
         root, dirs, files = take!(chnl)
         @test root == joinpath(".", "sub_dir1")
-        @test dirs == (has_symlinks ? ["link", "subsub_dir1", "subsub_dir2"] : ["subsub_dir1", "subsub_dir2"])
-        @test files == ["file1", "file2"]
+        if has_symlinks
+            if follow_symlinks
+                @test dirs ==  ["link", "subsub_dir1", "subsub_dir2"]
+                @test files == ["file1", "file2"]
+            else
+                @test dirs ==  ["subsub_dir1", "subsub_dir2"]
+                @test files == ["file1", "file2", "link"]
+            end
+        else
+            @test dirs ==  ["subsub_dir1", "subsub_dir2"]
+            @test files == ["file1", "file2"]
+        end
 
         root, dirs, files = take!(chnl)
         if follow_symlinks
@@ -1256,8 +1266,18 @@ cd(dirwalk) do
             root, dirs, files = take!(chnl)
         end
         @test root == joinpath(".", "sub_dir1")
-        @test dirs ==  (has_symlinks ? ["link", "subsub_dir1", "subsub_dir2"] : ["subsub_dir1", "subsub_dir2"])
-        @test files == ["file1", "file2"]
+        if has_symlinks
+            if follow_symlinks
+                @test dirs ==  ["link", "subsub_dir1", "subsub_dir2"]
+                @test files == ["file1", "file2"]
+            else
+                @test dirs ==  ["subsub_dir1", "subsub_dir2"]
+                @test files == ["file1", "file2", "link"]
+            end
+        else
+            @test dirs ==  ["subsub_dir1", "subsub_dir2"]
+            @test files == ["file1", "file2"]
+        end
 
         root, dirs, files = take!(chnl)
         @test root == joinpath(".", "sub_dir2")
@@ -1289,6 +1309,18 @@ cd(dirwalk) do
     @test root == joinpath(".", "sub_dir2")
     @test dirs == []
     @test files == ["file_dir2"]
+
+    # Test that symlink loops don't cause errors
+    if has_symlinks
+        mkdir(joinpath(".", "sub_dir3"))
+        symlink("foo", joinpath(".", "sub_dir3", "foo"))
+
+        @test_throws Base.IOError walkdir(joinpath(".", "sub_dir3"); follow_symlinks=true)
+        root, dirs, files = take!(walkdir(joinpath(".", "sub_dir3"); follow_symlinks=false))
+        @test root == joinpath(".", "sub_dir3")
+        @test dirs == []
+        @test files == ["foo"]
+    end
 end
 rm(dirwalk, recursive=true)