Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

filesystem: fix error handling #36856

Merged
merged 2 commits into from
Aug 7, 2020
Merged

filesystem: fix error handling #36856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e530576
filesystem: fix error handling
vtjnash Jul 30, 2020
0640d25
Update file.jl
vtjnash Aug 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 33 additions & 37 deletions base/file.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ function readdir(dir::AbstractString; join::Bool=false, sort::Bool=true)
# defined in sys.c, to call uv_fs_readdir, which sets errno on error.
err = ccall(:uv_fs_scandir, Int32, (Ptr{Cvoid}, Ptr{UInt8}, Cstring, Cint, Ptr{Cvoid}),
C_NULL, uv_readdir_req, dir, 0, C_NULL)
err < 0 && throw(SystemError("unable to read directory $dir", -err))
err < 0 && throw(_UVError("readdir", err, "with ", repr(dir)))

# iterate the listing into entries
entries = String[]
Expand All @@ -804,10 +804,9 @@ readdir(; join::Bool=false, sort::Bool=true) =
Return an iterator that walks the directory tree of a directory.
The iterator returns a tuple containing `(rootpath, dirs, files)`.
The directory tree can be traversed top-down or bottom-up.
If `walkdir` encounters a [`SystemError`](@ref)
it will rethrow the error by default.
If `walkdir` or `stat` encounters a `IOError` it will rethrow the error by default.
A custom error handling function can be provided through `onerror` keyword argument.
`onerror` is called with a `SystemError` as argument.
`onerror` is called with a `IOError` as argument.

# Examples
```julia
Expand Down Expand Up @@ -839,48 +838,45 @@ julia> (root, dirs, files) = first(itr)
```
"""
function walkdir(root; topdown=true, follow_symlinks=false, onerror=throw)
content = nothing
try
content = readdir(root)
catch err
isa(err, SystemError) || throw(err)
onerror(err)
# Need to return an empty closed channel to skip the current root folder
chnl = Channel(0)
close(chnl)
return chnl
end
dirs = Vector{eltype(content)}()
files = Vector{eltype(content)}()
for name in content
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)
function _walkdir(chnl, root)
tryf(f, p) = try
f(p)
catch err
isa(err, IOError) || rethrow()
try
onerror(err)
catch err2
close(chnl, err2)
end
return
end
content = tryf(readdir, root)
content === nothing && return
dirs = Vector{eltype(content)}()
files = Vector{eltype(content)}()
for name in content
path = joinpath(root, name)

# If we're not following symlinks, then treat all symlinks as files
if (!follow_symlinks && something(tryf(islink, path), true)) || !something(tryf(isdir, path), false)
push!(files, name)
else
push!(dirs, name)
end
end
end

function _it(chnl)
if topdown
put!(chnl, (root, dirs, files))
push!(chnl, (root, dirs, files))
end
for dir in dirs
path = joinpath(root,dir)
if follow_symlinks || !islink(path)
for (root_l, dirs_l, files_l) in walkdir(path, topdown=topdown, follow_symlinks=follow_symlinks, onerror=onerror)
put!(chnl, (root_l, dirs_l, files_l))
end
end
_walkdir(chnl, joinpath(root, dir))
end
if !topdown
put!(chnl, (root, dirs, files))
push!(chnl, (root, dirs, files))
end
nothing
end

return Channel(_it)
return Channel(chnl -> _walkdir(chnl, root))
end

function unlink(p::AbstractString)
Expand Down
8 changes: 4 additions & 4 deletions base/filesystem.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,26 +219,26 @@ const SEEK_END = Int32(2)

function seek(f::File, n::Integer)
ret = ccall(:jl_lseek, Int64, (OS_HANDLE, Int64, Int32), f.handle, n, SEEK_SET)
systemerror("seek", ret == -1)
ret == -1 && (@static Sys.iswindows() ? windowserror : systemerror)("seek")
return f
end

function seekend(f::File)
ret = ccall(:jl_lseek, Int64, (OS_HANDLE, Int64, Int32), f.handle, 0, SEEK_END)
systemerror("seekend", ret == -1)
ret == -1 && (@static Sys.iswindows() ? windowserror : systemerror)("seekend")
return f
end

function skip(f::File, n::Integer)
ret = ccall(:jl_lseek, Int64, (OS_HANDLE, Int64, Int32), f.handle, n, SEEK_CUR)
systemerror("skip", ret == -1)
ret == -1 && (@static Sys.iswindows() ? windowserror : systemerror)("skip")
return f
end

function position(f::File)
check_open(f)
ret = ccall(:jl_lseek, Int64, (OS_HANDLE, Int64, Int32), f.handle, 0, SEEK_CUR)
systemerror("lseek", ret == -1)
ret == -1 && (@static Sys.iswindows() ? windowserror : systemerror)("lseek")
return ret
end

Expand Down
4 changes: 2 additions & 2 deletions stdlib/REPL/src/REPLCompletions.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,10 @@ function complete_path(path::AbstractString, pos; use_envpath=false, shell_escap
filesinpath = readdir(pathdir)
catch e
# Bash allows dirs in PATH that can't be read, so we should as well.
if isa(e, SystemError)
if isa(e, Base.IOError)
continue
else
# We only handle SystemErrors here
# We only handle IOError here
rethrow()
end
end
Expand Down
60 changes: 33 additions & 27 deletions test/file.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ subdir = joinpath(dir, "adir")
mkdir(subdir)
subdir2 = joinpath(dir, "adir2")
mkdir(subdir2)
@test_throws Base.IOError mkdir(file)
@test_throws Base._UVError("mkdir", Base.UV_EEXIST) mkdir(file)
let err = nothing
try
mkdir(file)
Expand Down Expand Up @@ -445,9 +445,9 @@ close(f)

rm(c_tmpdir, recursive=true)
@test !isdir(c_tmpdir)
@test_throws Base.IOError rm(c_tmpdir)
@test_throws Base._UVError(Sys.iswindows() ? "chmod" : "unlink", Base.UV_ENOENT) rm(c_tmpdir)
@test rm(c_tmpdir, force=true) === nothing
@test_throws Base.IOError rm(c_tmpdir, recursive=true)
@test_throws Base._UVError(Sys.iswindows() ? "chmod" : "unlink", Base.UV_ENOENT) rm(c_tmpdir, recursive=true)
@test rm(c_tmpdir, force=true, recursive=true) === nothing

if !Sys.iswindows()
Expand All @@ -462,8 +462,8 @@ if !Sys.iswindows()
@test stat(file).gid == 0
@test stat(file).uid == 0
else
@test_throws Base.IOError chown(file, -2, -1) # Non-root user cannot change ownership to another user
@test_throws Base.IOError chown(file, -1, -2) # Non-root user cannot change group to a group they are not a member of (eg: nogroup)
@test_throws Base._UVError("chown", Base.UV_EPERM) chown(file, -2, -1) # Non-root user cannot change ownership to another user
@test_throws Base._UVError("chown", Base.UV_EPERM) chown(file, -1, -2) # Non-root user cannot change group to a group they are not a member of (eg: nogroup)
end
else
# test that chown doesn't cause any errors for Windows
Expand Down Expand Up @@ -904,29 +904,31 @@ if !Sys.iswindows() || Sys.windows_version() >= Sys.WINDOWS_VISTA_VER
for src in dirs
for dst in dirs
# cptree
@test_throws ArgumentError Base.cptree(src,dst; force=true, follow_symlinks=false)
@test_throws ArgumentError Base.cptree(src,dst; force=true, follow_symlinks=true)
@test_throws ArgumentError Base.cptree(src, dst; force=true, follow_symlinks=false)
@test_throws ArgumentError Base.cptree(src, dst; force=true, follow_symlinks=true)
# cp
@test_throws ArgumentError cp(src,dst; force=true, follow_symlinks=false)
@test_throws ArgumentError cp(src,dst; force=true, follow_symlinks=true)
@test_throws ArgumentError cp(src, dst; force=true, follow_symlinks=false)
@test_throws ArgumentError cp(src, dst; force=true, follow_symlinks=true)
# mv
@test_throws ArgumentError mv(src,dst; force=true)
@test_throws ArgumentError mv(src, dst; force=true)
end
end
end
# None existing src
mktempdir() do tmpdir
none_existing_src = joinpath(tmpdir, "none_existing_src")
nonexisting_src = joinpath(tmpdir, "nonexisting_src")
dst = joinpath(tmpdir, "dst")
@test !ispath(none_existing_src)
@test !ispath(nonexisting_src)
# cptree
@test_throws ArgumentError Base.cptree(none_existing_src,dst; force=true, follow_symlinks=false)
@test_throws ArgumentError Base.cptree(none_existing_src,dst; force=true, follow_symlinks=true)
@test_throws(ArgumentError("'$nonexisting_src' is not a directory. Use `cp(src, dst)`"),
Base.cptree(nonexisting_src, dst; force=true, follow_symlinks=false))
@test_throws(ArgumentError("'$nonexisting_src' is not a directory. Use `cp(src, dst)`"),
Base.cptree(nonexisting_src, dst; force=true, follow_symlinks=true))
# cp
@test_throws Base.IOError cp(none_existing_src,dst; force=true, follow_symlinks=false)
@test_throws Base.IOError cp(none_existing_src,dst; force=true, follow_symlinks=true)
@test_throws Base._UVError("open", Base.UV_ENOENT) cp(nonexisting_src, dst; force=true, follow_symlinks=false)
@test_throws Base._UVError("open", Base.UV_ENOENT) cp(nonexisting_src, dst; force=true, follow_symlinks=true)
# mv
@test_throws Base.IOError mv(none_existing_src,dst; force=true)
@test_throws Base._UVError("open", Base.UV_ENOENT) mv(nonexisting_src, dst; force=true)
end
end

Expand Down Expand Up @@ -1130,13 +1132,13 @@ if !Sys.iswindows()
for src in files
for dst in files
# cptree
@test_throws ArgumentError Base.cptree(src,dst; force=true, follow_symlinks=false)
@test_throws ArgumentError Base.cptree(src,dst; force=true, follow_symlinks=true)
@test_throws ArgumentError Base.cptree(src, dst; force=true, follow_symlinks=false)
@test_throws ArgumentError Base.cptree(src, dst; force=true, follow_symlinks=true)
# cp
@test_throws ArgumentError cp(src,dst; force=true, follow_symlinks=false)
@test_throws ArgumentError cp(src,dst; force=true, follow_symlinks=true)
@test_throws ArgumentError cp(src, dst; force=true, follow_symlinks=false)
@test_throws ArgumentError cp(src, dst; force=true, follow_symlinks=true)
# mv
@test_throws ArgumentError mv(src,dst; force=true)
@test_throws ArgumentError mv(src, dst; force=true)
end
end
end
Expand Down Expand Up @@ -1298,7 +1300,8 @@ cd(dirwalk) do
@test files == ["file1", "file2"]

rm(joinpath("sub_dir1"), recursive=true)
@test_throws TaskFailedException take!(chnl_error) # throws an error because sub_dir1 do not exist
@test_throws(Base._UVError("readdir", Base.UV_ENOENT, "with ", repr(joinpath(".", "sub_dir1"))),
take!(chnl_error)) # throws an error because sub_dir1 do not exist

root, dirs, files = take!(chnl_noerror)
@test root == "."
Expand All @@ -1313,9 +1316,12 @@ cd(dirwalk) do
# Test that symlink loops don't cause errors
if has_symlinks
mkdir(joinpath(".", "sub_dir3"))
symlink("foo", joinpath(".", "sub_dir3", "foo"))
foo = joinpath(".", "sub_dir3", "foo")
symlink("foo", foo)

@test_throws Base.IOError walkdir(joinpath(".", "sub_dir3"); follow_symlinks=true)
let files = walkdir(joinpath(".", "sub_dir3"); follow_symlinks=true)
@test_throws Base._UVError("stat", Base.UV_ELOOP, "for file ", repr(foo)) take!(files)
end
root, dirs, files = take!(walkdir(joinpath(".", "sub_dir3"); follow_symlinks=false))
@test root == joinpath(".", "sub_dir3")
@test dirs == []
Expand Down Expand Up @@ -1504,8 +1510,8 @@ end
rm(d, recursive=true)
@test !ispath(d)
@test isempty(readdir())
@test_throws SystemError readdir(d)
@test_throws Base.IOError readdir(join=true)
@test_throws Base._UVError("readdir", Base.UV_ENOENT, "with ", repr(d)) readdir(d)
@test_throws Base._UVError("cwd", Base.UV_ENOENT) readdir(join=true)
end
end
end
Expand Down