Skip to content

Commit

Permalink
walkdir: avoid symlink loops when follow_symlinks == false (#35006)
Browse files Browse the repository at this point in the history
* 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

(cherry picked from commit 178ac97)
  • Loading branch information
staticfloat authored and KristofferC committed Mar 23, 2020
1 parent 4e11ce9 commit 5cf6725
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 8 deletions.
11 changes: 7 additions & 4 deletions base/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
40 changes: 36 additions & 4 deletions test/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1215,8 +1215,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
Expand Down Expand Up @@ -1253,8 +1263,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")
Expand Down Expand Up @@ -1286,6 +1306,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)

Expand Down

0 comments on commit 5cf6725

Please sign in to comment.