From a32c2ff04693887fd2fda7b8ba5b6cf08c3cb6ee Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 31 May 2026 14:24:29 +0100 Subject: [PATCH 1/3] Fix history saving when directory writability is unreliable --- lib/irb/history.rb | 50 ++++++++++++++++++++-------------------- test/irb/test_history.rb | 27 ++++++++++++++++++++++ 2 files changed, 52 insertions(+), 25 deletions(-) diff --git a/lib/irb/history.rb b/lib/irb/history.rb index 0beff1553..1b0f2bc59 100644 --- a/lib/irb/history.rb +++ b/lib/irb/history.rb @@ -64,13 +64,6 @@ def load_history def save_history return unless History.save_history? return unless (history_file = History.history_file) - unless ensure_history_file_writable(history_file) - warn <<~WARN - Can't write history to #{History.history_file.inspect} due to insufficient permissions. - Please verify the value of `IRB.conf[:HISTORY_FILE]`. Ensure the folder exists and that both the folder and file (if it exists) are writable. - WARN - return - end history = self.class::HISTORY.to_a @@ -80,36 +73,43 @@ def save_history append_history = true end - File.open(history_file, (append_history ? "a" : "w"), 0o600, encoding: IRB.conf[:LC_MESSAGES]&.encoding) do |f| - hist = history.map { |l| l.scrub.split("\n").join("\\\n") } + begin + ensure_history_file_permissions(history_file) - unless append_history || History.infinite? - # Check size before slicing because array.last(huge_number) raises RangeError. - hist = hist.last(History.save_history) if hist.size > History.save_history - end + File.open(history_file, (append_history ? "a" : "w"), 0o600, encoding: IRB.conf[:LC_MESSAGES]&.encoding) do |f| + hist = history.map { |l| l.scrub.split("\n").join("\\\n") } + + unless append_history || History.infinite? + # Check size before slicing because array.last(huge_number) raises RangeError. + hist = hist.last(History.save_history) if hist.size > History.save_history + end - f.puts(hist) + f.puts(hist) + end + rescue Errno::ENOENT + warn <<~WARN + Can't write history to #{History.history_file.inspect} because the folder does not exist. + Please verify the value of `IRB.conf[:HISTORY_FILE]`. Ensure the folder exists. + WARN + rescue Errno::EACCES, Errno::EPERM + warn <<~WARN + Can't write history to #{History.history_file.inspect} due to insufficient permissions. + Please verify the value of `IRB.conf[:HISTORY_FILE]`. Ensure the folder exists and that both the folder and file (if it exists) are writable. + WARN end end private - # Returns boolean whether writing to +history_file+ will be possible. # Permissions of already existing +history_file+ are changed to # owner-only-readable if necessary [BUG #7694]. - def ensure_history_file_writable(history_file) + def ensure_history_file_permissions(history_file) history_file = Pathname.new(history_file) - return false unless history_file.dirname.writable? - return true unless history_file.exist? + return unless history_file.exist? - begin - if history_file.stat.mode & 0o66 != 0 - history_file.chmod 0o600 - end - true - rescue Errno::EPERM # no permissions - false + if history_file.stat.mode & 0o66 != 0 + history_file.chmod 0o600 end end end diff --git a/test/irb/test_history.rb b/test/irb/test_history.rb index c6e931e80..bf6363ce2 100644 --- a/test/irb/test_history.rb +++ b/test/irb/test_history.rb @@ -169,6 +169,33 @@ def test_history_concurrent_use_not_present assert_equal(%w"line0 line1 line2", File.read(history_file).split) end + def test_history_save_uses_actual_write_result_not_directory_writable_predicate + IRB.conf[:SAVE_HISTORY] = 1 + io = TestInputMethodWithRelineHistory.new + io.class::HISTORY.clear + io.class::HISTORY << "line1" + + history_file = IRB.rc_file("_history") + FileUtils.rm_f(history_file) + + original_writable = Pathname.instance_method(:writable?) + Pathname.class_eval do + define_method(:writable?) { false } + end + + assert_warn("") do + io.save_history + end + + assert_equal("line1\n", File.read(history_file)) + ensure + if original_writable + Pathname.class_eval do + define_method(:writable?, original_writable) + end + end + end + def test_history_different_encodings IRB.conf[:SAVE_HISTORY] = 2 IRB.conf[:LC_MESSAGES] = IRB::Locale.new("en_US.ASCII") From 30036fc12cbe8a3cee9f3456ef0693a701cd37b2 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 31 May 2026 20:50:49 +0100 Subject: [PATCH 2/3] Use Windows history regression test --- test/irb/test_history.rb | 27 ---------- test/irb/test_history_windows.rb | 92 ++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 27 deletions(-) create mode 100644 test/irb/test_history_windows.rb diff --git a/test/irb/test_history.rb b/test/irb/test_history.rb index bf6363ce2..c6e931e80 100644 --- a/test/irb/test_history.rb +++ b/test/irb/test_history.rb @@ -169,33 +169,6 @@ def test_history_concurrent_use_not_present assert_equal(%w"line0 line1 line2", File.read(history_file).split) end - def test_history_save_uses_actual_write_result_not_directory_writable_predicate - IRB.conf[:SAVE_HISTORY] = 1 - io = TestInputMethodWithRelineHistory.new - io.class::HISTORY.clear - io.class::HISTORY << "line1" - - history_file = IRB.rc_file("_history") - FileUtils.rm_f(history_file) - - original_writable = Pathname.instance_method(:writable?) - Pathname.class_eval do - define_method(:writable?) { false } - end - - assert_warn("") do - io.save_history - end - - assert_equal("line1\n", File.read(history_file)) - ensure - if original_writable - Pathname.class_eval do - define_method(:writable?, original_writable) - end - end - end - def test_history_different_encodings IRB.conf[:SAVE_HISTORY] = 2 IRB.conf[:LC_MESSAGES] = IRB::Locale.new("en_US.ASCII") diff --git a/test/irb/test_history_windows.rb b/test/irb/test_history_windows.rb new file mode 100644 index 000000000..b4a13a6ec --- /dev/null +++ b/test/irb/test_history_windows.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: false +require "irb" +require "fileutils" +require "tmpdir" + +require_relative "helper" + +module TestIRB + class HistoryWindowsTest < TestCase + class TestInputMethodWithHistory < TestInputMethod + HISTORY = [] + + include IRB::HistorySavingAbility + + def gets + super&.tap do |line| + HISTORY << line unless line.empty? + end + end + end + + def setup + @conf_backup = IRB.conf.dup + @original_verbose, $VERBOSE = $VERBOSE, nil + IRB.instance_variable_set(:@existing_rc_name_generators, nil) + end + + def teardown + IRB.conf.replace(@conf_backup) + IRB.instance_variable_set(:@existing_rc_name_generators, nil) + TestInputMethodWithHistory::HISTORY.clear + $VERBOSE = @original_verbose + end + + def test_history_is_saved_in_readonly_attributed_directory + omit "Windows-only" unless windows? + + Dir.mktmpdir("test_irb_history_windows_") do |tmpdir| + history_dir = File.join(tmpdir, "history") + history_file = File.join(history_dir, "irb_history") + FileUtils.mkdir_p(history_dir) + + with_readonly_attribute(history_dir) do + assert_nothing_raised do + File.write(File.join(history_dir, "manual_write"), "ok") + end + + _output, warning = capture_output do + run_irb_with_history(history_file) + end + + assert_equal("", warning) + assert_equal(<<~HISTORY, File.read(history_file)) + puts 'history_entry' + exit + HISTORY + end + end + end + + private + + def with_readonly_attribute(path) + assert(system("attrib", "+R", windows_path(path)), "failed to set read-only attribute on #{path}") + yield + ensure + system("attrib", "-R", windows_path(path)) if path && File.directory?(path) + end + + def windows_path(path) + return path unless File::ALT_SEPARATOR + + path.tr(File::SEPARATOR, File::ALT_SEPARATOR) + end + + def run_irb_with_history(history_file) + IRB.init_config(nil) + IRB.init_error + IRB.conf[:PROMPT_MODE] = :SIMPLE + IRB.conf[:SAVE_HISTORY] = 100 + IRB.conf[:HISTORY_FILE] = history_file + IRB.conf[:USE_PAGER] = false + + input = TestInputMethodWithHistory.new(["puts 'history_entry'", "exit"]) + irb = IRB::Irb.new(IRB::WorkSpace.new(Object.new), input) + irb.context.return_format = "=> %s\n" + irb.run(IRB.conf) + ensure + TestInputMethodWithHistory::HISTORY.clear + end + end +end From d52a03298c623f1c7daa7c280351e893d8a77944 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 31 May 2026 21:00:39 +0100 Subject: [PATCH 3/3] Share history test setup --- test/irb/history_test_case.rb | 29 +++++++++++++++++ test/irb/test_history.rb | 24 ++------------- test/irb/test_history_windows.rb | 53 ++++++++++---------------------- 3 files changed, 48 insertions(+), 58 deletions(-) create mode 100644 test/irb/history_test_case.rb diff --git a/test/irb/history_test_case.rb b/test/irb/history_test_case.rb new file mode 100644 index 000000000..8408d1fce --- /dev/null +++ b/test/irb/history_test_case.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: false +require "irb" +require "fileutils" +require "tmpdir" + +require_relative "helper" + +module TestIRB + class HistoryTestCase < TestCase + def setup + @conf_backup = IRB.conf.dup + @original_verbose, $VERBOSE = $VERBOSE, nil + @tmpdir = Dir.mktmpdir("test_irb_history_") + setup_envs(home: @tmpdir) + IRB.conf[:LC_MESSAGES] = IRB::Locale.new + save_encodings + IRB.instance_variable_set(:@existing_rc_name_generators, nil) + end + + def teardown + IRB.conf.replace(@conf_backup) + IRB.instance_variable_set(:@existing_rc_name_generators, nil) + teardown_envs + restore_encodings + $VERBOSE = @original_verbose + FileUtils.rm_rf(@tmpdir) + end + end +end diff --git a/test/irb/test_history.rb b/test/irb/test_history.rb index c6e931e80..e600c74cb 100644 --- a/test/irb/test_history.rb +++ b/test/irb/test_history.rb @@ -1,8 +1,7 @@ # frozen_string_literal: false -require 'irb' require "tempfile" -require_relative "helper" +require_relative "history_test_case" return if RUBY_PLATFORM.match?(/solaris|mswin|mingw/i) @@ -14,26 +13,7 @@ module TestIRB Readline = ::Reline end - class HistoryTest < TestCase - def setup - @conf_backup = IRB.conf.dup - @original_verbose, $VERBOSE = $VERBOSE, nil - @tmpdir = Dir.mktmpdir("test_irb_history_") - setup_envs(home: @tmpdir) - IRB.conf[:LC_MESSAGES] = IRB::Locale.new - save_encodings - IRB.instance_variable_set(:@existing_rc_name_generators, nil) - end - - def teardown - IRB.conf.replace(@conf_backup) - IRB.instance_variable_set(:@existing_rc_name_generators, nil) - teardown_envs - restore_encodings - $VERBOSE = @original_verbose - FileUtils.rm_rf(@tmpdir) - end - + class HistoryTest < HistoryTestCase class TestInputMethodWithRelineHistory < TestInputMethod # When IRB.conf[:USE_MULTILINE] is true, IRB::RelineInputMethod uses Reline::History HISTORY = Reline::History.new(Reline.core.config) diff --git a/test/irb/test_history_windows.rb b/test/irb/test_history_windows.rb index b4a13a6ec..6bf8e8772 100644 --- a/test/irb/test_history_windows.rb +++ b/test/irb/test_history_windows.rb @@ -1,12 +1,8 @@ # frozen_string_literal: false -require "irb" -require "fileutils" -require "tmpdir" - -require_relative "helper" +require_relative "history_test_case" module TestIRB - class HistoryWindowsTest < TestCase + class HistoryWindowsTest < HistoryTestCase class TestInputMethodWithHistory < TestInputMethod HISTORY = [] @@ -19,42 +15,27 @@ def gets end end - def setup - @conf_backup = IRB.conf.dup - @original_verbose, $VERBOSE = $VERBOSE, nil - IRB.instance_variable_set(:@existing_rc_name_generators, nil) - end - - def teardown - IRB.conf.replace(@conf_backup) - IRB.instance_variable_set(:@existing_rc_name_generators, nil) - TestInputMethodWithHistory::HISTORY.clear - $VERBOSE = @original_verbose - end - def test_history_is_saved_in_readonly_attributed_directory omit "Windows-only" unless windows? - Dir.mktmpdir("test_irb_history_windows_") do |tmpdir| - history_dir = File.join(tmpdir, "history") - history_file = File.join(history_dir, "irb_history") - FileUtils.mkdir_p(history_dir) - - with_readonly_attribute(history_dir) do - assert_nothing_raised do - File.write(File.join(history_dir, "manual_write"), "ok") - end + history_dir = File.join(@tmpdir, "history") + history_file = File.join(history_dir, "irb_history") + FileUtils.mkdir_p(history_dir) - _output, warning = capture_output do - run_irb_with_history(history_file) - end + with_readonly_attribute(history_dir) do + assert_nothing_raised do + File.write(File.join(history_dir, "manual_write"), "ok") + end - assert_equal("", warning) - assert_equal(<<~HISTORY, File.read(history_file)) - puts 'history_entry' - exit - HISTORY + _output, warning = capture_output do + run_irb_with_history(history_file) end + + assert_equal("", warning) + assert_equal(<<~HISTORY, File.read(history_file)) + puts 'history_entry' + exit + HISTORY end end