Fix history saving when directory writability is unreliable

Author: st0012

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

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")