Bug #16918: Dir.mktmpdir should yield a copy of the dir to protect cleanup

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-darwin18]
• Backport: 2.6: DONE, 2.7: DONE

[headius (Charles Nutter)]

If you modify the dir string passed into the block from Dir.mktmpdir, the logic to clean up the temporary directory may fail:

$ rvm ruby-2.6.5 do ruby -rtmpdir -e "Dir.mktmpdir('foo') {|dir| dir << 'bar'}"
Traceback (most recent call last):
	9: from -e:1:in `<main>'
	8: from /Users/headius/.rvm/rubies/ruby-2.6.5/lib/ruby/2.6.0/tmpdir.rb:101:in `mktmpdir'
	7: from /Users/headius/.rvm/rubies/ruby-2.6.5/lib/ruby/2.6.0/fileutils.rb:758:in `remove_entry'
	6: from /Users/headius/.rvm/rubies/ruby-2.6.5/lib/ruby/2.6.0/fileutils.rb:1480:in `postorder_traverse'
	5: from /Users/headius/.rvm/rubies/ruby-2.6.5/lib/ruby/2.6.0/fileutils.rb:760:in `block in remove_entry'
	4: from /Users/headius/.rvm/rubies/ruby-2.6.5/lib/ruby/2.6.0/fileutils.rb:1425:in `remove'
	3: from /Users/headius/.rvm/rubies/ruby-2.6.5/lib/ruby/2.6.0/fileutils.rb:1436:in `remove_file'
	2: from /Users/headius/.rvm/rubies/ruby-2.6.5/lib/ruby/2.6.0/fileutils.rb:1442:in `platform_support'
	1: from /Users/headius/.rvm/rubies/ruby-2.6.5/lib/ruby/2.6.0/fileutils.rb:1437:in `block in remove_file'
/Users/headius/.rvm/rubies/ruby-2.6.5/lib/ruby/2.6.0/fileutils.rb:1437:in `unlink': No such file or directory @ apply2files - /var/folders/cq/ylcgmnn556x33f5hsqd0h54h0000gn/T/foo20200528-99594-tuq6pubar (Errno::ENOENT)

I believe Dir.mktmpdir should protect its cleanup logic by yielding a copy of the dir string, rather than the exact string object it intends to use for cleanup.

[jeremyevans0 (Jeremy Evans)]

• Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN to 2.6: REQUIRED, 2.7: REQUIRED

I agree this is a bug and have added a pull request to fix it: https://github.com/ruby/ruby/pull/3159

[shevegen (Robert A. Heiler)]

Makes sense to me what headius wrote, so if there are no side effects (I have too
little experience with Dir.mktmpdir myself) +1 to the suggestion from me.

• Status changed from Open to Closed

Applied in changeset git|2ecfb88ee50510955acd3ae9fc94a5f109e7f109.

---

Correctly remove temporary directory if path yielded is mutated

Another approach would be to freeze the string, but that could
cause backwards compatibility issues.

Fixes [Bug #16918]

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.6: REQUIRED, 2.7: REQUIRED to 2.6: REQUIRED, 2.7: DONE

ruby_2_7 13d2ab0d88bbf72ed310efaec6edc46dd96fdb4d merged revision(s) 2ecfb88ee50510955acd3ae9fc94a5f109e7f109.

[usa (Usaku NAKAMURA)]

• Backport changed from 2.6: REQUIRED, 2.7: DONE to 2.6: DONE, 2.7: DONE

backported into ruby_2_6 at r67910.