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

Dir.tmpdir should be more apprehensive in giving out something in the current working directory #4450

Closed
evadne opened this issue Jan 21, 2017 · 2 comments
Closed

Dir.tmpdir should be more apprehensive in giving out something in the current working directory #4450

evadne opened this issue Jan 21, 2017 · 2 comments
Milestone
Invalid or Duplicate

Comments

@evadne
Copy link

evadne commented Jan 21, 2017 •
edited

Environment

JRuby: 9.1.5.0
OS: Amazon Linux

Expected Behavior

Dir.tmpdir should prefer to provide something in /tmp over something in .

Actual Behavior

Dir.tmpdir chooses . over /tmp because it prefers an user-writable, non-world-writable directory over an user-writable, world-writable directory with sticky bit set.

Thoughts

Temporary files will sometimes accumulate in the application‘s root directory if the application was written with an assumption that things get dumped into /tmp which can be cleared by another script run periodically. In cases where apps are deployed and release-managed with Capistrano this can cause releases to swell and take up more disk space than necessary.

My setup looked like this:

[evadne@ecu-converter-nonprod-1 /]$ ls -al /srv/converter/releases | grep 20170118213914
drwxr-xr-x 14 deployer deployer 77824 Jan 21 18:51 20170118213914
[evadne@ecu-converter-nonprod-1 /]$ ls -la / | grep tmp
drwxrwxrwt  13 root     root     49152 Jan 21 16:41 tmp

At this time I have worked around the issue by setting TMPDIR to something in /var/tmp with proper permissions.

I would think that a sensible change would be to not prefer the current directory over somewhere else, as shown in this diff:

	diff --git a/lib/ruby/stdlib/tmpdir.rb b/lib/ruby/stdlib/tmpdir.rb
	index f62a082..bb4086d 100644
	--- a/lib/ruby/stdlib/tmpdir.rb
	+++ b/lib/ruby/stdlib/tmpdir.rb
	@@ -27,7 +27,7 @@ class Dir
	       # FileUtils.remove_entry_secure(dir) crashes when a dir is under
	       # a world-writable directory because it tries to open directory.
	       # Opening directory is not allowed in Java.
	-      dirs = [ENV['TMPDIR'], ENV['TMP'], ENV['TEMP'], @@systmpdir, '/tmp', '.']
	+      dirs = [ENV['TMPDIR'], ENV['TMP'], ENV['TEMP'], @@systmpdir, '/tmp']
	       for dir in dirs
	         if dir and stat = File.stat(dir) and stat.directory? and stat.writable? and !stat.world_writable?
	           return File.expand_path(dir)

See:
#405
#633
@BanzaiMan
Copy link
Member

Since tmpdir is in MRI's standard library, I do not think we should be patching it here. If you want this change, please report it on https://bugs.ruby-lang.org.

Thank you.

@kares kares added this to the Invalid or Duplicate milestone Jan 22, 2017
@evadne
Copy link
Author

evadne commented Jan 22, 2017

@BanzaiMan https://ruby-doc.org/stdlib-2.3.0/libdoc/tmpdir/rdoc/Dir.html#method-c-tmpdir — implementation only has one loop (instead of two in JRuby 9.1.5.0) so I think this is still worth a look

@evadne evadne mentioned this issue Jan 22, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Invalid or Duplicate
Development

No branches or pull requests
3 participants
@BanzaiMan @kares @evadne