diff options
Diffstat (limited to 'actionpack')
9 files changed, 58 insertions, 7 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 96f91b90ff..38132371cc 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,8 +1,12 @@ -* Silence Puma start-up messages running system tests. +* Fix malformed URLS when using `ApplicationController.renderer` - Fixes #28109. + The Rack environment variable `rack.url_scheme` was not being set so `scheme` was + returning `nil`. This caused URLs to be malformed with the default settings. + Fix this by setting `rack.url_scheme` when the environment is normalized. - *Yuji Yaginuma* (#28283) + Fixes #28151. + + *George Vrettos* * Commit flash changes when using a redirect route. diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index a349841082..1836a07d4e 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -50,6 +50,9 @@ module ActionController # redirect_to post_url(@post), status: 301, flash: { updated_post_id: @post.id } # redirect_to({ action: 'atom' }, alert: "Something serious happened") # + # Statements after +redirect_to+ in our controller get executed, so +redirect_to+ doesn't stop the execution of the function. + # To terminate the execution of the function immediately after the +redirect_to+, use return. + # redirect_to post_url(@post) and return def redirect_to(options = {}, response_status = {}) raise ActionControllerError.new("Cannot redirect to nil!") unless options raise AbstractController::DoubleRenderError if response_body diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index d304dcf468..3b293baa73 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -862,7 +862,7 @@ module ActionController # end # # # This will pass with flying colors as long as there's a person key in the - # # parameters, otherwise it'll raise an ActionController::MissingParameter + # # parameters, otherwise it'll raise an ActionController::ParameterMissing # # exception, which will get caught by ActionController::Base and turned # # into a 400 Bad Request reply. # def update diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index acb400cd15..e1441bd343 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -85,6 +85,7 @@ module ActionController def normalize_keys(env) new_env = {} env.each_pair { |k, v| new_env[rack_key_for(k)] = rack_value_for(k, v) } + new_env["rack.url_scheme"] = new_env["HTTPS"] == "on" ? "https" : "http" new_env end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 6e06c70dc2..dea6c4482e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -2054,7 +2054,7 @@ module ActionDispatch # your url helper definition, e.g: # # direct :browse, page: 1, size: 10 do |options| - # [ :products, options.merge(params.permit(:page, :size)) ] + # [ :products, options.merge(params.permit(:page, :size).to_h.symbolize_keys) ] # end # # In this instance the `params` object comes from the context in which the the diff --git a/actionpack/lib/action_dispatch/system_testing/server.rb b/actionpack/lib/action_dispatch/system_testing/server.rb index ee4b7efce0..4a214ef713 100644 --- a/actionpack/lib/action_dispatch/system_testing/server.rb +++ b/actionpack/lib/action_dispatch/system_testing/server.rb @@ -11,7 +11,7 @@ module ActionDispatch private def register Capybara.register_server :rails_puma do |app, port, host| - Rack::Handler::Puma.run(app, Port: port, Threads: "0:1", Silent: true) + Rack::Handler::Puma.run(app, Port: port, Threads: "0:1") end end diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb index ddc961cf84..e37f6d02aa 100644 --- a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb @@ -22,7 +22,7 @@ module ActionDispatch # fails add +take_failed_screenshot+ to the teardown block before clearing # sessions. def take_failed_screenshot - take_screenshot if failed? + take_screenshot if failed? && supports_screenshot? end private @@ -55,6 +55,10 @@ module ActionDispatch def failed? !passed? && !skipped? end + + def supports_screenshot? + page.driver.public_methods(false).include?(:save_screenshot) + end end end end diff --git a/actionpack/test/controller/renderer_test.rb b/actionpack/test/controller/renderer_test.rb index 866600b935..81b32a67b3 100644 --- a/actionpack/test/controller/renderer_test.rb +++ b/actionpack/test/controller/renderer_test.rb @@ -103,6 +103,20 @@ class RendererTest < ActiveSupport::TestCase assert_equal "true", content end + test "return valid asset url with defaults" do + renderer = ApplicationController.renderer + content = renderer.render inline: "<%= asset_url 'asset.jpg' %>" + + assert_equal "http://example.org/asset.jpg", content + end + + test "return valid asset url when https is true" do + renderer = ApplicationController.renderer.new https: true + content = renderer.render inline: "<%= asset_url 'asset.jpg' %>" + + assert_equal "https://example.org/asset.jpg", content + end + private def render @render ||= ApplicationController.renderer.method(:render) diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb index 3b4ea96c4f..d6b501b3ac 100644 --- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -1,5 +1,6 @@ require "abstract_unit" require "action_dispatch/system_testing/test_helpers/screenshot_helper" +require "capybara/dsl" class ScreenshotHelperTest < ActiveSupport::TestCase test "image path is saved in tmp directory" do @@ -25,4 +26,28 @@ class ScreenshotHelperTest < ActiveSupport::TestCase end end end + + test "rack_test driver does not support screenshot" do + begin + original_driver = Capybara.current_driver + Capybara.current_driver = :rack_test + + new_test = ActionDispatch::SystemTestCase.new("x") + assert_not new_test.send(:supports_screenshot?) + ensure + Capybara.current_driver = original_driver + end + end + + test "selenium driver supports screenshot" do + begin + original_driver = Capybara.current_driver + Capybara.current_driver = :selenium + + new_test = ActionDispatch::SystemTestCase.new("x") + assert new_test.send(:supports_screenshot?) + ensure + Capybara.current_driver = original_driver + end + end end |