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/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 new file mode 100644 index 000000000..6bf8e8772 --- /dev/null +++ b/test/irb/test_history_windows.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: false +require_relative "history_test_case" + +module TestIRB + class HistoryWindowsTest < HistoryTestCase + class TestInputMethodWithHistory < TestInputMethod + HISTORY = [] + + include IRB::HistorySavingAbility + + def gets + super&.tap do |line| + HISTORY << line unless line.empty? + end + end + end + + def test_history_is_saved_in_readonly_attributed_directory + omit "Windows-only" unless windows? + + 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 + + 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