From 96562a517d3373466ec306b5f821a41f4758d2a6 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Tue, 26 Jul 2022 21:23:47 +0900 Subject: [PATCH] [ruby/fileutils] Narrow the scope of ensure The ensure in postorder_traverse was added for [Bug #6756]. The intention was to try to delete the parent directory if it failed to get the children. (It may be possible to delete the directory if it is empty.) However, the ensure region rescue'ed not only "failure to get children" but also "failure to delete each child". Thus, the following raised Errno::ENOTEMPTY, but we expect it to raise Errno::EACCES. ``` $ mkdir foo $ touch foo/bar $ chmod 555 foo $ ruby -rfileutils -e 'FileUtils.rm_rf("foo")' ``` This changeset narrows the ensure region so that it rescues only "failure to get children". https://github.com/ruby/fileutils/commit/ec5d3b84ea --- lib/fileutils.rb | 12 ++++++++++-- test/fileutils/test_fileutils.rb | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/fileutils.rb b/lib/fileutils.rb index 8ae5266864..4ba7d18a5d 100644 --- a/lib/fileutils.rb +++ b/lib/fileutils.rb @@ -2328,13 +2328,21 @@ module FileUtils def postorder_traverse if directory? - entries().each do |ent| + begin + children = entries() + rescue Errno::EACCES + # Failed to get the list of children. + # Assuming there is no children, try to process the parent directory. + yield self + return + end + + children.each do |ent| ent.postorder_traverse do |e| yield e end end end - ensure yield self end diff --git a/test/fileutils/test_fileutils.rb b/test/fileutils/test_fileutils.rb index 8c49eb39bb..05ba8d184a 100644 --- a/test/fileutils/test_fileutils.rb +++ b/test/fileutils/test_fileutils.rb @@ -750,6 +750,24 @@ class TestFileUtils < Test::Unit::TestCase assert_file_not_exist 'tmp/tmpdir3' end + def test_rm_r_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_r 'tmpdatadir' + end + ensure + chmod "+x", 'tmpdatadir' + end + end + def test_remove_entry_cjk_path dir = "tmpdir\u3042" my_rm_rf dir