From df339f3bb36b8bb5bcb70ee5cf82fbedc3a753a1 Mon Sep 17 00:00:00 2001 From: Arthur Chiu Date: Fri, 27 Mar 2026 21:36:45 -0700 Subject: [PATCH 1/5] fix: correct 'Allows' header to 'Allow' in 405 responses per RFC 7231 --- padrino-core/lib/padrino-core/application/routing.rb | 2 +- padrino-core/test/test_routing.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/padrino-core/lib/padrino-core/application/routing.rb b/padrino-core/lib/padrino-core/application/routing.rb index aaa1ba3e3..3eb14c3c0 100644 --- a/padrino-core/lib/padrino-core/application/routing.rb +++ b/padrino-core/lib/padrino-core/application/routing.rb @@ -965,7 +965,7 @@ def route!(base = settings, pass_block = nil) verb = request.request_method candidacies, allows = routes.partition { |route| route.verb == verb } if candidacies.empty? - response['Allows'] = allows.map(&:verb).join(', ') + response['Allow'] = allows.map(&:verb).join(', ') halt 405 end end diff --git a/padrino-core/test/test_routing.rb b/padrino-core/test/test_routing.rb index 490b25370..833eafeb6 100644 --- a/padrino-core/test/test_routing.rb +++ b/padrino-core/test/test_routing.rb @@ -749,6 +749,7 @@ class Test < Padrino::Application end get '/bar' assert_equal 405, status + assert_equal 'POST', response['Allow'] end it 'should respond to' do From 90294cc926462ebba0925dbf98e54aedf6cbb0ea Mon Sep 17 00:00:00 2001 From: Arthur Chiu Date: Fri, 27 Mar 2026 21:38:09 -0700 Subject: [PATCH 2/5] fix: correct Mail::Message.set guard to check name instead of :erb --- padrino-mailer/lib/padrino-mailer/ext.rb | 2 +- padrino-mailer/test/test_message.rb | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/padrino-mailer/lib/padrino-mailer/ext.rb b/padrino-mailer/lib/padrino-mailer/ext.rb index 0a7d3a14a..cf6ba0766 100644 --- a/padrino-mailer/lib/padrino-mailer/ext.rb +++ b/padrino-mailer/lib/padrino-mailer/ext.rb @@ -137,7 +137,7 @@ def settings # Sinatra almost compatibility. # def self.set(name, value) - self.class.instance_eval { define_method(name) { value } unless method_defined?(:erb) } + self.class.instance_eval { define_method(name) { value } unless method_defined?(name) } end ## diff --git a/padrino-mailer/test/test_message.rb b/padrino-mailer/test/test_message.rb index 7597d4e92..9acf4f69e 100644 --- a/padrino-mailer/test/test_message.rb +++ b/padrino-mailer/test/test_message.rb @@ -166,4 +166,23 @@ assert_equal "Object 1
\nObject 2
\nObject <evil>
\nObject
", message.body.to_s.chomp end end + + describe 'Mail::Message.set' do + it 'should define accessor methods for settings' do + Mail::Message.set(:custom_setting, 'custom_value') + message = Mail::Message.new + assert_equal 'custom_value', message.settings.custom_setting + ensure + Class.remove_method(:custom_setting) if Class.method_defined?(:custom_setting) + end + + it 'should not redefine an already defined method' do + Mail::Message.set(:another_setting, 'first') + Mail::Message.set(:another_setting, 'second') + message = Mail::Message.new + assert_equal 'first', message.settings.another_setting + ensure + Class.remove_method(:another_setting) if Class.method_defined?(:another_setting) + end + end end From 62f5d1b8ff4a63b7ebb96eb920cbe6d83a57f17c Mon Sep 17 00:00:00 2001 From: Arthur Chiu <24772+achiurizo@users.noreply.github.com> Date: Fri, 27 Mar 2026 23:54:19 -0700 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- padrino-core/lib/padrino-core/application/routing.rb | 2 +- padrino-mailer/lib/padrino-mailer/ext.rb | 4 +++- padrino-mailer/test/test_message.rb | 6 ++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/padrino-core/lib/padrino-core/application/routing.rb b/padrino-core/lib/padrino-core/application/routing.rb index 3eb14c3c0..33e9fb470 100644 --- a/padrino-core/lib/padrino-core/application/routing.rb +++ b/padrino-core/lib/padrino-core/application/routing.rb @@ -965,7 +965,7 @@ def route!(base = settings, pass_block = nil) verb = request.request_method candidacies, allows = routes.partition { |route| route.verb == verb } if candidacies.empty? - response['Allow'] = allows.map(&:verb).join(', ') + response['Allow'] = allows.map(&:verb).uniq.join(', ') halt 405 end end diff --git a/padrino-mailer/lib/padrino-mailer/ext.rb b/padrino-mailer/lib/padrino-mailer/ext.rb index cf6ba0766..3e4a6a888 100644 --- a/padrino-mailer/lib/padrino-mailer/ext.rb +++ b/padrino-mailer/lib/padrino-mailer/ext.rb @@ -137,7 +137,9 @@ def settings # Sinatra almost compatibility. # def self.set(name, value) - self.class.instance_eval { define_method(name) { value } unless method_defined?(name) } + singleton_class.class_eval do + define_method(name) { value } unless method_defined?(name) + end end ## diff --git a/padrino-mailer/test/test_message.rb b/padrino-mailer/test/test_message.rb index 9acf4f69e..22bd10002 100644 --- a/padrino-mailer/test/test_message.rb +++ b/padrino-mailer/test/test_message.rb @@ -169,20 +169,22 @@ describe 'Mail::Message.set' do it 'should define accessor methods for settings' do + custom_setting_predefined = Class.method_defined?(:custom_setting) Mail::Message.set(:custom_setting, 'custom_value') message = Mail::Message.new assert_equal 'custom_value', message.settings.custom_setting ensure - Class.remove_method(:custom_setting) if Class.method_defined?(:custom_setting) + Class.remove_method(:custom_setting) if !custom_setting_predefined && Class.method_defined?(:custom_setting) end it 'should not redefine an already defined method' do + another_setting_predefined = Class.method_defined?(:another_setting) Mail::Message.set(:another_setting, 'first') Mail::Message.set(:another_setting, 'second') message = Mail::Message.new assert_equal 'first', message.settings.another_setting ensure - Class.remove_method(:another_setting) if Class.method_defined?(:another_setting) + Class.remove_method(:another_setting) if !another_setting_predefined && Class.method_defined?(:another_setting) end end end From 21a49319c1611d6ad625a341a705853e5f66e8ea Mon Sep 17 00:00:00 2001 From: Arthur Chiu Date: Sat, 28 Mar 2026 09:56:39 -0700 Subject: [PATCH 4/5] fix: address PR review feedback for Allow header and set tests --- padrino-mailer/test/test_message.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/padrino-mailer/test/test_message.rb b/padrino-mailer/test/test_message.rb index 22bd10002..9fc80e9af 100644 --- a/padrino-mailer/test/test_message.rb +++ b/padrino-mailer/test/test_message.rb @@ -169,22 +169,20 @@ describe 'Mail::Message.set' do it 'should define accessor methods for settings' do - custom_setting_predefined = Class.method_defined?(:custom_setting) - Mail::Message.set(:custom_setting, 'custom_value') + Mail::Message.set(:_test_custom_setting, 'custom_value') message = Mail::Message.new - assert_equal 'custom_value', message.settings.custom_setting + assert_equal 'custom_value', message.settings._test_custom_setting ensure - Class.remove_method(:custom_setting) if !custom_setting_predefined && Class.method_defined?(:custom_setting) + Class.send(:remove_method, :_test_custom_setting) if Class.method_defined?(:_test_custom_setting) end it 'should not redefine an already defined method' do - another_setting_predefined = Class.method_defined?(:another_setting) - Mail::Message.set(:another_setting, 'first') - Mail::Message.set(:another_setting, 'second') + Mail::Message.set(:_test_another_setting, 'first') + Mail::Message.set(:_test_another_setting, 'second') message = Mail::Message.new - assert_equal 'first', message.settings.another_setting + assert_equal 'first', message.settings._test_another_setting ensure - Class.remove_method(:another_setting) if !another_setting_predefined && Class.method_defined?(:another_setting) + Class.send(:remove_method, :_test_another_setting) if Class.method_defined?(:_test_another_setting) end end end From a00c998a1f781e0adb1df466c879563f17a6810b Mon Sep 17 00:00:00 2001 From: Nathan Esquenazi Date: Sat, 28 Mar 2026 10:11:47 -0700 Subject: [PATCH 5/5] Fix test cleanup to target Mail::Message.singleton_class The ensure blocks were cleaning up from Class instead of Mail::Message.singleton_class, making them no-ops since set() defines methods on the singleton class. --- padrino-mailer/test/test_message.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/padrino-mailer/test/test_message.rb b/padrino-mailer/test/test_message.rb index 9fc80e9af..26e6a9576 100644 --- a/padrino-mailer/test/test_message.rb +++ b/padrino-mailer/test/test_message.rb @@ -173,7 +173,9 @@ message = Mail::Message.new assert_equal 'custom_value', message.settings._test_custom_setting ensure - Class.send(:remove_method, :_test_custom_setting) if Class.method_defined?(:_test_custom_setting) + if Mail::Message.singleton_class.method_defined?(:_test_custom_setting) + Mail::Message.singleton_class.send(:remove_method, :_test_custom_setting) + end end it 'should not redefine an already defined method' do @@ -182,7 +184,9 @@ message = Mail::Message.new assert_equal 'first', message.settings._test_another_setting ensure - Class.send(:remove_method, :_test_another_setting) if Class.method_defined?(:_test_another_setting) + if Mail::Message.singleton_class.method_defined?(:_test_another_setting) + Mail::Message.singleton_class.send(:remove_method, :_test_another_setting) + end end end end