From ed1c2a1adb18ef79004db03e00cc8c6394301e42 Mon Sep 17 00:00:00 2001
From: Louis-Michel Couture <louis-michel.couture@gsoft.com>
Date: Fri, 5 May 2023 10:20:13 -0400
Subject: [PATCH] Make sure Mailer defaults :from and :reply_to are handled
 correctly

Rails allow procs and lambda with either zero or more argument. Devise
however always tried to call instance_eval on those values, which does
always pass one argument: self.

There was a PR to fix this specific problem in Devise https://github.com/heartcombo/devise/pull/4627,
before the arity check was fixed in rails itself: https://github.com/rails/rails/pull/30391.

But even if the problem was fixed in Rails, Devise was still calling
the proc/lambas with instance_eval. That meant the fix added to Rails
did not apply to Devise.

The fix is to let Rails handle the :from and :reply_to defaults. We do
that by unsetting the headers instead of trying to replicate Rails handling
in Devise. This lets Rails handle it when setting up the mailer.
---
 lib/devise/mailers/helpers.rb | 24 +++++++++---------------
 test/mailers/mailer_test.rb   | 26 ++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/lib/devise/mailers/helpers.rb b/lib/devise/mailers/helpers.rb
index f6997462..29a49197 100644
--- a/lib/devise/mailers/helpers.rb
+++ b/lib/devise/mailers/helpers.rb
@@ -33,28 +33,22 @@ module Devise
           subject: subject_for(action),
           to: resource.email,
           from: mailer_sender(devise_mapping),
-          reply_to: mailer_reply_to(devise_mapping),
+          reply_to: mailer_sender(devise_mapping),
           template_path: template_paths,
           template_name: action
-        }.merge(opts)
+        }
+        # Give priority to the mailer's default if they exists.
+        headers.delete(:from) if default_params[:from]
+        headers.delete(:reply_to) if default_params[:reply_to]
+
+        headers.merge!(opts)
 
         @email = headers[:to]
         headers
       end
 
-      def mailer_reply_to(mapping)
-        mailer_sender(mapping, :reply_to)
-      end
-
-      def mailer_from(mapping)
-        mailer_sender(mapping, :from)
-      end
-
-      def mailer_sender(mapping, sender = :from)
-        default_sender = default_params[sender]
-        if default_sender.present?
-          default_sender.respond_to?(:to_proc) ? instance_eval(&default_sender) : default_sender
-        elsif Devise.mailer_sender.is_a?(Proc)
+      def mailer_sender(mapping)
+        if Devise.mailer_sender.is_a?(Proc)
           Devise.mailer_sender.call(mapping.name)
         else
           Devise.mailer_sender
diff --git a/test/mailers/mailer_test.rb b/test/mailers/mailer_test.rb
index f8369052..6f9f568e 100644
--- a/test/mailers/mailer_test.rb
+++ b/test/mailers/mailer_test.rb
@@ -17,4 +17,30 @@ class MailerTest < ActionMailer::TestCase
 
     assert mail.content_transfer_encoding, "7bit"
   end
+
+  test "default values defined as proc with different arity are handled correctly" do
+    class TestMailerWithDefault < Devise::Mailer
+      default from: -> { computed_from }
+      default reply_to: ->(_) { computed_reply_to }
+
+      def confirmation_instructions(record, token, opts = {})
+        @token = token
+        devise_mail(record, :confirmation_instructions, opts)
+      end
+
+      private
+
+      def computed_from
+        "from@example.com"
+      end
+
+      def computed_reply_to
+        "reply_to@example.com"
+      end
+    end
+
+    mail = TestMailerWithDefault.confirmation_instructions(create_user, "confirmation-token")
+    assert mail.from, "from@example.com"
+    assert mail.reply_to, "reply_to@example.com"
+  end
 end