From 555f6e7e0d3d36da3073ed82af7bcc57dc434d6f Mon Sep 17 00:00:00 2001 From: Gert Goet Date: Tue, 8 Oct 2024 11:27:17 +0200 Subject: [PATCH 1/3] Extract logic save_history in separate helper --- lib/irb.rb | 5 +++-- lib/irb/history.rb | 39 ++++++++++++++++++++++++++++++++------- test/irb/test_history.rb | 15 +++++++++++++++ 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/lib/irb.rb b/lib/irb.rb index 213e23117..f833c3967 100644 --- a/lib/irb.rb +++ b/lib/irb.rb @@ -14,6 +14,7 @@ require_relative "irb/ruby-lex" require_relative "irb/statement" +require_relative "irb/history" require_relative "irb/input-method" require_relative "irb/locale" require_relative "irb/color" @@ -972,7 +973,7 @@ def debug_readline(binding) # debugger. input = nil forced_exit = catch(:IRB_EXIT) do - if IRB.conf[:SAVE_HISTORY] && context.io.support_history_saving? + if History.save_history? && context.io.support_history_saving? # Previous IRB session's history has been saved when `Irb#run` is exited We need # to make sure the saved history is not saved again by resetting the counter context.io.reset_history_counter @@ -1003,7 +1004,7 @@ def run(conf = IRB.conf) prev_context = conf[:MAIN_CONTEXT] conf[:MAIN_CONTEXT] = context - save_history = !in_nested_session && conf[:SAVE_HISTORY] && context.io.support_history_saving? + save_history = !in_nested_session && History.save_history? && context.io.support_history_saving? if save_history context.io.load_history diff --git a/lib/irb/history.rb b/lib/irb/history.rb index 685354b2d..ef25a6dbe 100644 --- a/lib/irb/history.rb +++ b/lib/irb/history.rb @@ -1,6 +1,33 @@ require "pathname" module IRB + module History + class << self + # Integer representation of IRB.conf[:HISTORY_FILE]. + def save_history + num = IRB.conf[:SAVE_HISTORY].to_i + # Bignums cause RangeErrors when slicing arrays. + # Treat such values as 'infinite'. + (num > save_history_max) ? -1 : num + end + + def save_history? + !save_history.zero? + end + + def infinite? + save_history.negative? + end + + private + + def save_history_max + # Max fixnum (32-bit) that can be used without getting RangeError. + 2**30 - 1 + end + end + end + module HistorySavingAbility # :nodoc: def support_history_saving? true @@ -37,7 +64,7 @@ def load_history def save_history history = self.class::HISTORY.to_a - if num = IRB.conf[:SAVE_HISTORY] and (num = num.to_i) != 0 + if History.save_history? if history_file = IRB.conf[:HISTORY_FILE] history_file = File.expand_path(history_file) end @@ -72,13 +99,11 @@ def save_history 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 - begin - hist = hist.last(num) if hist.size > num and num > 0 - rescue RangeError # bignum too big to convert into `long' - # Do nothing because the bignum should be treated as infinity - end + + unless append_history || History.infinite? + hist = hist.last(History.save_history) end + f.puts(hist) end end diff --git a/test/irb/test_history.rb b/test/irb/test_history.rb index 7a17491d8..8355a7a3d 100644 --- a/test/irb/test_history.rb +++ b/test/irb/test_history.rb @@ -39,6 +39,21 @@ class TestInputMethodWithReadlineHistory < TestInputMethod include IRB::HistorySavingAbility end + def test_history_dont_save + omit "Skip Editline" if /EditLine/n.match(Readline::VERSION) + IRB.conf[:SAVE_HISTORY] = nil + assert_history(<<~EXPECTED_HISTORY, <<~INITIAL_HISTORY, <<~INPUT) + 1 + 2 + EXPECTED_HISTORY + 1 + 2 + INITIAL_HISTORY + 3 + exit + INPUT + end + def test_history_save_1 omit "Skip Editline" if /EditLine/n.match(Readline::VERSION) IRB.conf[:SAVE_HISTORY] = 1 From d1d541438a98a345fe5423132502fd4ada983de9 Mon Sep 17 00:00:00 2001 From: Gert Goet Date: Tue, 8 Oct 2024 11:36:52 +0200 Subject: [PATCH 2/3] Extract logic history_file in helper --- lib/irb/history.rb | 111 +++++++++++++++++++++------------------ test/irb/test_history.rb | 2 +- 2 files changed, 61 insertions(+), 52 deletions(-) diff --git a/lib/irb/history.rb b/lib/irb/history.rb index ef25a6dbe..a428003d2 100644 --- a/lib/irb/history.rb +++ b/lib/irb/history.rb @@ -19,6 +19,15 @@ def infinite? save_history.negative? end + # Might be nil when HOME and XDG_CONFIG_HOME are not available. + def history_file + if (history_file = IRB.conf[:HISTORY_FILE]) + File.expand_path(history_file) + else + IRB.rc_file("_history") + end + end + private def save_history_max @@ -38,74 +47,74 @@ def reset_history_counter end def load_history + history_file = History.history_file + return unless File.exist?(history_file.to_s) + history = self.class::HISTORY - if history_file = IRB.conf[:HISTORY_FILE] - history_file = File.expand_path(history_file) - end - history_file = IRB.rc_file("_history") unless history_file - if history_file && File.exist?(history_file) - File.open(history_file, "r:#{IRB.conf[:LC_MESSAGES].encoding}") do |f| - f.each { |l| - l = l.chomp - if self.class == RelineInputMethod and history.last&.end_with?("\\") - history.last.delete_suffix!("\\") - history.last << "\n" << l - else - history << l - end - } - end - @loaded_history_lines = history.size - @loaded_history_mtime = File.mtime(history_file) + File.open(history_file, "r:#{IRB.conf[:LC_MESSAGES].encoding}") do |f| + f.each { |l| + l = l.chomp + if self.class == RelineInputMethod and history.last&.end_with?("\\") + history.last.delete_suffix!("\\") + history.last << "\n" << l + else + history << l + end + } end + @loaded_history_lines = history.size + @loaded_history_mtime = File.mtime(history_file) end 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 - if History.save_history? - if history_file = IRB.conf[:HISTORY_FILE] - history_file = File.expand_path(history_file) - end - history_file = IRB.rc_file("_history") unless history_file + if File.exist?(history_file) && + File.mtime(history_file) != @loaded_history_mtime + history = history[@loaded_history_lines..-1] if @loaded_history_lines + append_history = true + end - # When HOME and XDG_CONFIG_HOME are not available, history_file might be nil - return unless history_file + 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") } - # Change the permission of a file that already exists[BUG #7694] - begin - if File.stat(history_file).mode & 066 != 0 - File.chmod(0600, history_file) - end - rescue Errno::ENOENT - rescue Errno::EPERM - return - rescue - raise + unless append_history || History.infinite? + hist = hist.last(History.save_history) end - if File.exist?(history_file) && - File.mtime(history_file) != @loaded_history_mtime - history = history[@loaded_history_lines..-1] if @loaded_history_lines - append_history = true - end + f.puts(hist) + end + end - pathname = Pathname.new(history_file) - unless Dir.exist?(pathname.dirname) - warn "Warning: The directory to save IRB's history file does not exist. Please double check `IRB.conf[:HISTORY_FILE]`'s value." - return - end + private - 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") } + # 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) + history_file = Pathname.new(history_file) - unless append_history || History.infinite? - hist = hist.last(History.save_history) - end + return false unless history_file.dirname.writable? + return true unless history_file.exist? - f.puts(hist) + begin + if history_file.stat.mode & 0o66 != 0 + history_file.chmod 0o600 end + true + rescue Errno::EPERM # no permissions + false end end end diff --git a/test/irb/test_history.rb b/test/irb/test_history.rb index 8355a7a3d..791eef1ac 100644 --- a/test/irb/test_history.rb +++ b/test/irb/test_history.rb @@ -181,7 +181,7 @@ def test_history_does_not_raise_when_history_file_directory_does_not_exist IRB.conf[:HISTORY_FILE] = "fake/fake/fake/history_file" io = TestInputMethodWithRelineHistory.new - assert_warn(/history file does not exist/) do + assert_warn(/ensure the folder exists/i) do io.save_history end From 35864fcb1fedef91c2f3f27cfc7ca6ea88fdc709 Mon Sep 17 00:00:00 2001 From: Gert Goet Date: Thu, 10 Oct 2024 16:05:59 +0200 Subject: [PATCH 3/3] Allow for readonly history --- lib/irb.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/irb.rb b/lib/irb.rb index f833c3967..7e91b0f64 100644 --- a/lib/irb.rb +++ b/lib/irb.rb @@ -1004,9 +1004,10 @@ def run(conf = IRB.conf) prev_context = conf[:MAIN_CONTEXT] conf[:MAIN_CONTEXT] = context - save_history = !in_nested_session && History.save_history? && context.io.support_history_saving? + load_history = !in_nested_session && context.io.support_history_saving? + save_history = load_history && History.save_history? - if save_history + if load_history context.io.load_history end