diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index f15c476998d..3c8e344e0c0 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,25 @@ +* When a host is not specified for an `ActionController::Renderer`'s env, + the host and related options will now be derived from the routes' + `default_url_options` and `ActionDispatch::Http::URL.secure_protocol`. + + This means that for an application with a configuration like: + + ```ruby + Rails.application.default_url_options = { host: "rubyonrails.org" } + Rails.application.config.force_ssl = true + ``` + + rendering a URL like: + + ```ruby + ApplicationController.renderer.render inline: "<%= blog_url %>" + ``` + + will now return `"https://rubyonrails.org/blog"` instead of + `"http://example.org/blog"`. + + *Jonathan Hefner* + * Add details of cookie name and size to `CookieOverflow` exception. *Andy Waite* diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index e75d93cee43..64a17e82df3 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -23,10 +23,7 @@ module ActionController attr_reader :controller DEFAULTS = { - http_host: "example.org", - https: false, method: "get", - script_name: "", input: "" }.freeze @@ -46,7 +43,14 @@ module ActionController new_env[key] = value end - new_env["rack.url_scheme"] = new_env["HTTPS"] == "on" ? "https" : "http" + if new_env["HTTP_HOST"] + new_env["HTTPS"] ||= "off" + new_env["SCRIPT_NAME"] ||= "" + end + + if new_env["HTTPS"] + new_env["rack.url_scheme"] = new_env["HTTPS"] == "on" ? "https" : "http" + end new_env end @@ -90,6 +94,13 @@ module ActionController # * +defaults+ - Default values for the Rack env. Entries are specified in # the same format as +env+. +env+ will be merged on top of these values. # +defaults+ will be retained when calling #new on a renderer instance. + # + # If no +http_host+ is specified, the env HTTP host will be derived from the + # routes' +default_url_options+. In this case, the +https+ boolean and the + # +script_name+ will also be derived from +default_url_options+ if they were + # not specified. Additionally, the +https+ boolean will fall back to + # +Rails.application.config.force_ssl+ if +default_url_options+ does not + # specify a +protocol+. def initialize(controller, env, defaults) @controller = controller @defaults = defaults @@ -128,9 +139,7 @@ module ActionController # # Otherwise, a partial is rendered using the second parameter as the locals hash. def render(*args) - raise "missing controller" unless controller - - request = ActionDispatch::Request.new(@env.dup) + request = ActionDispatch::Request.new(env_for_request) request.routes = controller._routes instance = controller.new @@ -152,5 +161,13 @@ module ActionController DEFAULT_ENV = normalize_env(DEFAULTS).freeze # :nodoc: delegate :normalize_env, to: :class + + def env_for_request + if @env.key?("HTTP_HOST") || controller._routes.nil? + @env.dup + else + controller._routes.default_env.merge(@env) + end + end end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 7d3d14c45c2..7cb1bef06ac 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -375,6 +375,7 @@ module ActionDispatch @disable_clear_and_finalize = false @finalized = false @env_key = "ROUTES_#{object_id}_SCRIPT_NAME" + @default_env = nil @set = Journey::Routes.new @router = Journey::Router.new @set @@ -405,6 +406,25 @@ module ActionDispatch end private :make_request + def default_env + if default_url_options != @default_env&.[]("action_dispatch.routes.default_url_options") + url_options = default_url_options.dup.freeze + uri = URI(ActionDispatch::Http::URL.full_url_for(host: "example.org", **url_options)) + + @default_env = { + "action_dispatch.routes" => self, + "action_dispatch.routes.default_url_options" => url_options, + "HTTPS" => uri.scheme == "https" ? "on" : "off", + "rack.url_scheme" => uri.scheme, + "HTTP_HOST" => uri.port == uri.default_port ? uri.host : "#{uri.host}:#{uri.port}", + "SCRIPT_NAME" => uri.path.chomp("/"), + "rack.input" => "", + }.freeze + end + + @default_env + end + def draw(&block) clear! unless @disable_clear_and_finalize eval_block(block) diff --git a/actionpack/test/controller/renderer_test.rb b/actionpack/test/controller/renderer_test.rb index e64dbd6a642..c7c63051384 100644 --- a/actionpack/test/controller/renderer_test.rb +++ b/actionpack/test/controller/renderer_test.rb @@ -108,18 +108,17 @@ class RendererTest < ActiveSupport::TestCase html = "Hello world!" xml = "

Hello world!

\n" - assert_equal html, render["respond_to/using_defaults"] - assert_equal xml, render["respond_to/using_defaults", formats: :xml] + assert_equal html, render("respond_to/using_defaults") + assert_equal xml, render("respond_to/using_defaults", formats: :xml) end test "rendering with helpers" do - assert_equal "

1\n
2

", render[inline: '<%= simple_format "1\n2" %>'] + assert_equal "

1\n
2

", render(inline: '<%= simple_format "1\n2" %>') end test "rendering with user specified defaults" do - ApplicationController.renderer.defaults.merge!(hello: "hello", https: true) - renderer = ApplicationController.renderer.new - content = renderer.render inline: "<%= request.ssl? %>" + renderer.defaults.merge!(hello: "hello", https: true) + content = renderer.new.render inline: "<%= request.ssl? %>" assert_equal "true", content end @@ -138,8 +137,81 @@ class RendererTest < ActiveSupport::TestCase assert_equal "https://example.org/asset.jpg", content end + test "uses default_url_options from the controller's routes when env[:http_host] not specified" do + with_default_url_options( + protocol: "https", + host: "foo.example.com", + port: 9001, + script_name: "/bar", + ) do + assert_equal "https://foo.example.com:9001/bar/posts", render_url_for(controller: :posts) + end + end + + test "uses config.force_ssl when env[:http_host] not specified" do + with_default_url_options(host: "foo.example.com") do + with_force_ssl do + assert_equal "https://foo.example.com/posts", render_url_for(controller: :posts) + end + end + end + + test "can specify env[:https] when using default_url_options" do + with_default_url_options(host: "foo.example.com") do + @renderer = renderer.new(https: true) + assert_equal "https://foo.example.com/posts", render_url_for(controller: :posts) + end + end + + test "env[:https] overrides default_url_options[:protocol]" do + with_default_url_options(host: "foo.example.com", protocol: "https") do + @renderer = renderer.new(https: false) + assert_equal "http://foo.example.com/posts", render_url_for(controller: :posts) + end + end + + test "can specify env[:script_name] when using default_url_options" do + with_default_url_options(host: "foo.example.com") do + @renderer = renderer.new(script_name: "/bar") + assert_equal "http://foo.example.com/bar/posts", render_url_for(controller: :posts) + end + end + + test "env[:script_name] overrides default_url_options[:script_name]" do + with_default_url_options(host: "foo.example.com", script_name: "/bar") do + @renderer = renderer.new(script_name: "") + assert_equal "http://foo.example.com/posts", render_url_for(controller: :posts) + end + end + private - def render - @render ||= ApplicationController.renderer.method(:render) + def renderer + @renderer ||= ApplicationController.renderer.new + end + + def render(...) + renderer.render(...) + end + + def render_url_for(*args) + render inline: "<%= full_url_for(*#{args.inspect}) %>" + end + + def with_default_url_options(default_url_options) + original_default_url_options = renderer.controller._routes.default_url_options + renderer.controller._routes.default_url_options = default_url_options + yield + ensure + renderer.controller._routes.default_url_options = original_default_url_options + renderer.controller._routes.default_env # refresh + end + + def with_force_ssl(force_ssl = true) + # In a real app, an initializer will set `URL.secure_protocol = app.config.force_ssl`. + original_secure_protocol = ActionDispatch::Http::URL.secure_protocol + ActionDispatch::Http::URL.secure_protocol = force_ssl + yield + ensure + ActionDispatch::Http::URL.secure_protocol = original_secure_protocol end end diff --git a/actiontext/CHANGELOG.md b/actiontext/CHANGELOG.md index 8e6c921b1a9..05f266251a5 100644 --- a/actiontext/CHANGELOG.md +++ b/actiontext/CHANGELOG.md @@ -1,3 +1,9 @@ +* Action Text attachment URLs rendered in a background job (a la Turbo + Streams) now use `Rails.application.default_url_options` and + `Rails.application.config.force_ssl` instead of `http://example.org`. + + *Jonathan Hefner* + * Focus rich-text editor after calling `fill_in_rich_text_area` *Sean Doyle* diff --git a/actiontext/test/dummy/app/jobs/broadcast_job.rb b/actiontext/test/dummy/app/jobs/broadcast_job.rb new file mode 100644 index 00000000000..109b34ef502 --- /dev/null +++ b/actiontext/test/dummy/app/jobs/broadcast_job.rb @@ -0,0 +1,9 @@ +class BroadcastJob < ApplicationJob + def perform(file, message) + File.write(file, <<~HTML) + + + + HTML + end +end diff --git a/actiontext/test/integration/job_render_test.rb b/actiontext/test/integration/job_render_test.rb new file mode 100644 index 00000000000..86955b3794b --- /dev/null +++ b/actiontext/test/integration/job_render_test.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "test_helper" + +class ActionText::JobRenderTest < ActiveJob::TestCase + include Rails::Dom::Testing::Assertions::SelectorAssertions + + test "uses app default_url_options" do + blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") + message = Message.create!(content: ActionText::Content.new.append_attachables(blob)) + + Dir.mktmpdir do |dir| + file = File.join(dir, "broadcast.html") + + BroadcastJob.perform_later(file, message) + + with_default_url_options(host: "foo.example.com", port: 9001) do + perform_enqueued_jobs + end + + rendered = Nokogiri::HTML::DocumentFragment.parse(File.read(file)) + assert_select rendered, "img:match('src', ?)", %r"//foo.example.com:9001/.+/racecar" + end + end + + private + def with_default_url_options(default_url_options) + original_default_url_options = Dummy::Application.default_url_options + Dummy::Application.default_url_options = default_url_options + yield + ensure + Dummy::Application.default_url_options = original_default_url_options + end +end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 5447be1bb0a..65441eb3288 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1437,6 +1437,27 @@ module ApplicationTests assert_deprecated(Rails.deprecator) { Rails.application.config.enable_dependency_loading = true } end + test "ActionController::Base::renderer uses Rails.application.default_url_options and config.force_ssl" do + add_to_config <<~RUBY + config.force_ssl = true + + Rails.application.default_url_options = { + host: "foo.example.com", + port: 9001, + script_name: "/bar", + } + + routes.prepend do + resources :posts + end + RUBY + + app "development" + + posts_url = ApplicationController.renderer.render(inline: "<%= posts_url %>") + assert_equal "https://foo.example.com:9001/bar/posts", posts_url + end + test "ActionController::Base.raise_on_open_redirects is true by default for new apps" do app "development"