From 58a324cb6a7bc98f3f193631cb0371509850c472 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 17 Feb 2017 10:23:15 -0800 Subject: [PATCH] Fix segv in `File.join` when SEPARATOR and Separator are set Setting `File::SEPARATOR` and `File::Separator` allows the existing separator string to be garbage collected. Since a global variable in C was pointing at the string, when the string was GC'd the reference would go bad, and `File.join` would use the bad reference and cause the program to crash. This commit changes `File.join` to look up the `File::SEPARATOR` constant when it is called. This keeps the program from crashing even if the original separator string is GC'd, and also makes the function behave like the documentation says it should. --- file.c | 6 ++---- test/ruby/test_file_exhaustive.rb | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/file.c b/file.c index bc928f3dec..5352c9fa24 100644 --- a/file.c +++ b/file.c @@ -4359,8 +4359,6 @@ rb_file_s_split(VALUE klass, VALUE path) return rb_assoc_new(rb_file_dirname(path), rb_file_s_basename(1,&path)); } -static VALUE separator; - static VALUE rb_file_join(VALUE ary, VALUE sep); static VALUE @@ -4461,7 +4459,7 @@ rb_file_join(VALUE ary, VALUE sep) static VALUE rb_file_s_join(VALUE klass, VALUE args) { - return rb_file_join(args, separator); + return rb_file_join(args, rb_const_get(rb_cFile, rb_intern("SEPARATOR"))); } #if defined(HAVE_TRUNCATE) || defined(HAVE_CHSIZE) @@ -6023,7 +6021,7 @@ Init_File(void) rb_define_singleton_method(rb_cFile, "extname", rb_file_s_extname, 1); rb_define_singleton_method(rb_cFile, "path", rb_file_s_path, 1); - separator = rb_obj_freeze(rb_usascii_str_new2("/")); + VALUE separator = rb_obj_freeze(rb_usascii_str_new_lit("/")); /* separates directory parts in path */ rb_define_const(rb_cFile, "Separator", separator); rb_define_const(rb_cFile, "SEPARATOR", separator); diff --git a/test/ruby/test_file_exhaustive.rb b/test/ruby/test_file_exhaustive.rb index 49e695fc75..ea65d54a68 100644 --- a/test/ruby/test_file_exhaustive.rb +++ b/test/ruby/test_file_exhaustive.rb @@ -3,6 +3,7 @@ require "fileutils" require "tmpdir" require "socket" +require "envutil" class TestFileExhaustive < Test::Unit::TestCase DRIVE = Dir.pwd[%r'\A(?:[a-z]:|//[^/]+/[^/]+)'i] @@ -199,6 +200,30 @@ def assert_integer_or_nil(n) end end + def test_file_join_works_after_const_set + EnvUtil.suppress_warning do + File.const_set :SEPARATOR, "/" + File.const_set :Separator, "/" + end + + GC.start + assert_equal "a/b", File.join("a", "b") + end + + def test_separator_impacts_join + before = File::SEPARATOR + EnvUtil.suppress_warning do + File.const_set :SEPARATOR, "$" + end + + assert_equal "a$b", File.join("a", "b") + + ensure + EnvUtil.suppress_warning do + File.const_set :SEPARATOR, before + end + end + def test_stat fn1 = regular_file hardlinkfile -- 2.11.0