Skip to content

Commit

Permalink
FileUtils.rm* methods swallows only Errno::ENOENT when force is true
Browse files Browse the repository at this point in the history
... instead of any StandardError.

To behave like the standard `rm` command, it should only ignore
exceptions about not existing files, not every exception. This should
make debugging some errors easier, because the expectation is that `rm
-rf` will succeed if and only if, all given files (previously existent
or not) are removed. However, due to this exception swallowing, this is
not always the case.

From the `rm` man page

> COMPATIBILITY
>
> The rm utility differs from historical implementations in that the -f
> option only masks attempts to remove non-existent files instead of
> masking a large variety of errors.

Co-Authored-By: David Rodríguez <deivid.rodriguez@riseup.net>
  • Loading branch information
mame and deivid-rodriguez committed Jul 26, 2022
1 parent ec5d3b8 commit fa65d67
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 6 deletions.
17 changes: 11 additions & 6 deletions lib/fileutils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@ def mv(src, dest, force: nil, noop: nil, verbose: nil, secure: nil)
#
# Keyword arguments:
#
# - <tt>force: true</tt> - ignores raised exceptions of StandardError
# - <tt>force: true</tt> - ignores raised exceptions of Errno::ENOENT
# and its descendants.
# - <tt>noop: true</tt> - does not remove files; returns +nil+.
# - <tt>verbose: true</tt> - prints an equivalent command:
Expand Down Expand Up @@ -1248,7 +1248,7 @@ def rm_f(list, noop: nil, verbose: nil)
#
# Keyword arguments:
#
# - <tt>force: true</tt> - ignores raised exceptions of StandardError
# - <tt>force: true</tt> - ignores raised exceptions of Errno::ENOENT
# and its descendants.
# - <tt>noop: true</tt> - does not remove entries; returns +nil+.
# - <tt>secure: true</tt> - removes +src+ securely;
Expand Down Expand Up @@ -1315,7 +1315,7 @@ def rm_rf(list, noop: nil, verbose: nil, secure: nil)
# see {Avoiding the TOCTTOU Vulnerability}[rdoc-ref:FileUtils@Avoiding+the+TOCTTOU+Vulnerability].
#
# Optional argument +force+ specifies whether to ignore
# raised exceptions of StandardError and its descendants.
# raised exceptions of Errno::ENOENT and its descendants.
#
# Related: {methods for deleting}[rdoc-ref:FileUtils@Deleting].
#
Expand Down Expand Up @@ -1384,10 +1384,12 @@ def remove_entry_secure(path, force = false)
ent.remove
rescue
raise unless force
raise unless Errno::ENOENT === $!
end
end
rescue
raise unless force
raise unless Errno::ENOENT === $!
end
module_function :remove_entry_secure

Expand All @@ -1413,7 +1415,7 @@ def fu_stat_identical_entry?(a, b) #:nodoc:
# should be {interpretable as a path}[rdoc-ref:FileUtils@Path+Arguments].
#
# Optional argument +force+ specifies whether to ignore
# raised exceptions of StandardError and its descendants.
# raised exceptions of Errno::ENOENT and its descendants.
#
# Related: FileUtils.remove_entry_secure.
#
Expand All @@ -1423,10 +1425,12 @@ def remove_entry(path, force = false)
ent.remove
rescue
raise unless force
raise unless Errno::ENOENT === $!
end
end
rescue
raise unless force
raise unless Errno::ENOENT === $!
end
module_function :remove_entry

Expand All @@ -1437,14 +1441,15 @@ def remove_entry(path, force = false)
# should be {interpretable as a path}[rdoc-ref:FileUtils@Path+Arguments].
#
# Optional argument +force+ specifies whether to ignore
# raised exceptions of StandardError and its descendants.
# raised exceptions of Errno::ENOENT and its descendants.
#
# Related: {methods for deleting}[rdoc-ref:FileUtils@Deleting].
#
def remove_file(path, force = false)
Entry_.new(path).remove_file
rescue
raise unless force
raise unless Errno::ENOENT === $!
end
module_function :remove_file

Expand All @@ -1456,7 +1461,7 @@ def remove_file(path, force = false)
# should be {interpretable as a path}[rdoc-ref:FileUtils@Path+Arguments].
#
# Optional argument +force+ specifies whether to ignore
# raised exceptions of StandardError and its descendants.
# raised exceptions of Errno::ENOENT and its descendants.
#
# Related: {methods for deleting}[rdoc-ref:FileUtils@Deleting].
#
Expand Down
20 changes: 20 additions & 0 deletions test/fileutils/test_fileutils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1814,6 +1814,26 @@ def test_rm_rf
assert_file_not_exist 'tmpdatadir'
end

def test_rm_rf_no_permissions
check_singleton :rm_rf

return if /mswin|mingw/ =~ RUBY_PLATFORM

mkdir 'tmpdatadir'
touch 'tmpdatadir/tmpdata'
chmod "-x", 'tmpdatadir'

begin
assert_raise Errno::EACCES do
rm_rf 'tmpdatadir'
end

assert_file_exist 'tmpdatadir'
ensure
chmod "+x", 'tmpdatadir'
end
end

def test_rmdir
check_singleton :rmdir

Expand Down

0 comments on commit fa65d67

Please sign in to comment.