Skip to content

Commit 12c0c0a

Browse files
Merge pull request rails#46649 from jonathanhefner/action_controller-renderer-default_url_options
Use `routes.default_url_options` in `AC::Renderer` env
2 parents 532294b + 93038ba commit 12c0c0a

File tree

8 files changed

+216
-15
lines changed

8 files changed

+216
-15
lines changed

actionpack/CHANGELOG.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,25 @@
1+
* When a host is not specified for an `ActionController::Renderer`'s env,
2+
the host and related options will now be derived from the routes'
3+
`default_url_options` and `ActionDispatch::Http::URL.secure_protocol`.
4+
5+
This means that for an application with a configuration like:
6+
7+
```ruby
8+
Rails.application.default_url_options = { host: "rubyonrails.org" }
9+
Rails.application.config.force_ssl = true
10+
```
11+
12+
rendering a URL like:
13+
14+
```ruby
15+
ApplicationController.renderer.render inline: "<%= blog_url %>"
16+
```
17+
18+
will now return `"https://rubyonrails.org/blog"` instead of
19+
`"http://example.org/blog"`.
20+
21+
*Jonathan Hefner*
22+
123
* Add details of cookie name and size to `CookieOverflow` exception.
224

325
*Andy Waite*

actionpack/lib/action_controller/renderer.rb

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,7 @@ class Renderer
2323
attr_reader :controller
2424

2525
DEFAULTS = {
26-
http_host: "example.org",
27-
https: false,
2826
method: "get",
29-
script_name: "",
3027
input: ""
3128
}.freeze
3229

@@ -46,7 +43,14 @@ def self.normalize_env(env) # :nodoc:
4643
new_env[key] = value
4744
end
4845

49-
new_env["rack.url_scheme"] = new_env["HTTPS"] == "on" ? "https" : "http"
46+
if new_env["HTTP_HOST"]
47+
new_env["HTTPS"] ||= "off"
48+
new_env["SCRIPT_NAME"] ||= ""
49+
end
50+
51+
if new_env["HTTPS"]
52+
new_env["rack.url_scheme"] = new_env["HTTPS"] == "on" ? "https" : "http"
53+
end
5054

5155
new_env
5256
end
@@ -90,6 +94,13 @@ def with_defaults(defaults)
9094
# * +defaults+ - Default values for the Rack env. Entries are specified in
9195
# the same format as +env+. +env+ will be merged on top of these values.
9296
# +defaults+ will be retained when calling #new on a renderer instance.
97+
#
98+
# If no +http_host+ is specified, the env HTTP host will be derived from the
99+
# routes' +default_url_options+. In this case, the +https+ boolean and the
100+
# +script_name+ will also be derived from +default_url_options+ if they were
101+
# not specified. Additionally, the +https+ boolean will fall back to
102+
# +Rails.application.config.force_ssl+ if +default_url_options+ does not
103+
# specify a +protocol+.
93104
def initialize(controller, env, defaults)
94105
@controller = controller
95106
@defaults = defaults
@@ -128,9 +139,7 @@ def defaults
128139
#
129140
# Otherwise, a partial is rendered using the second parameter as the locals hash.
130141
def render(*args)
131-
raise "missing controller" unless controller
132-
133-
request = ActionDispatch::Request.new(@env.dup)
142+
request = ActionDispatch::Request.new(env_for_request)
134143
request.routes = controller._routes
135144

136145
instance = controller.new
@@ -152,5 +161,13 @@ def render(*args)
152161
DEFAULT_ENV = normalize_env(DEFAULTS).freeze # :nodoc:
153162

154163
delegate :normalize_env, to: :class
164+
165+
def env_for_request
166+
if @env.key?("HTTP_HOST") || controller._routes.nil?
167+
@env.dup
168+
else
169+
controller._routes.default_env.merge(@env)
170+
end
171+
end
155172
end
156173
end

actionpack/lib/action_dispatch/routing/route_set.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ def initialize(config = DEFAULT_CONFIG)
375375
@disable_clear_and_finalize = false
376376
@finalized = false
377377
@env_key = "ROUTES_#{object_id}_SCRIPT_NAME"
378+
@default_env = nil
378379

379380
@set = Journey::Routes.new
380381
@router = Journey::Router.new @set
@@ -405,6 +406,25 @@ def make_request(env)
405406
end
406407
private :make_request
407408

409+
def default_env
410+
if default_url_options != @default_env&.[]("action_dispatch.routes.default_url_options")
411+
url_options = default_url_options.dup.freeze
412+
uri = URI(ActionDispatch::Http::URL.full_url_for(host: "example.org", **url_options))
413+
414+
@default_env = {
415+
"action_dispatch.routes" => self,
416+
"action_dispatch.routes.default_url_options" => url_options,
417+
"HTTPS" => uri.scheme == "https" ? "on" : "off",
418+
"rack.url_scheme" => uri.scheme,
419+
"HTTP_HOST" => uri.port == uri.default_port ? uri.host : "#{uri.host}:#{uri.port}",
420+
"SCRIPT_NAME" => uri.path.chomp("/"),
421+
"rack.input" => "",
422+
}.freeze
423+
end
424+
425+
@default_env
426+
end
427+
408428
def draw(&block)
409429
clear! unless @disable_clear_and_finalize
410430
eval_block(block)

actionpack/test/controller/renderer_test.rb

Lines changed: 80 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,18 +108,17 @@ class RendererTest < ActiveSupport::TestCase
108108
html = "Hello world!"
109109
xml = "<p>Hello world!</p>\n"
110110

111-
assert_equal html, render["respond_to/using_defaults"]
112-
assert_equal xml, render["respond_to/using_defaults", formats: :xml]
111+
assert_equal html, render("respond_to/using_defaults")
112+
assert_equal xml, render("respond_to/using_defaults", formats: :xml)
113113
end
114114

115115
test "rendering with helpers" do
116-
assert_equal "<p>1\n<br />2</p>", render[inline: '<%= simple_format "1\n2" %>']
116+
assert_equal "<p>1\n<br />2</p>", render(inline: '<%= simple_format "1\n2" %>')
117117
end
118118

119119
test "rendering with user specified defaults" do
120-
ApplicationController.renderer.defaults.merge!(hello: "hello", https: true)
121-
renderer = ApplicationController.renderer.new
122-
content = renderer.render inline: "<%= request.ssl? %>"
120+
renderer.defaults.merge!(hello: "hello", https: true)
121+
content = renderer.new.render inline: "<%= request.ssl? %>"
123122

124123
assert_equal "true", content
125124
end
@@ -138,8 +137,81 @@ class RendererTest < ActiveSupport::TestCase
138137
assert_equal "https://example.org/asset.jpg", content
139138
end
140139

140+
test "uses default_url_options from the controller's routes when env[:http_host] not specified" do
141+
with_default_url_options(
142+
protocol: "https",
143+
host: "foo.example.com",
144+
port: 9001,
145+
script_name: "/bar",
146+
) do
147+
assert_equal "https://foo.example.com:9001/bar/posts", render_url_for(controller: :posts)
148+
end
149+
end
150+
151+
test "uses config.force_ssl when env[:http_host] not specified" do
152+
with_default_url_options(host: "foo.example.com") do
153+
with_force_ssl do
154+
assert_equal "https://foo.example.com/posts", render_url_for(controller: :posts)
155+
end
156+
end
157+
end
158+
159+
test "can specify env[:https] when using default_url_options" do
160+
with_default_url_options(host: "foo.example.com") do
161+
@renderer = renderer.new(https: true)
162+
assert_equal "https://foo.example.com/posts", render_url_for(controller: :posts)
163+
end
164+
end
165+
166+
test "env[:https] overrides default_url_options[:protocol]" do
167+
with_default_url_options(host: "foo.example.com", protocol: "https") do
168+
@renderer = renderer.new(https: false)
169+
assert_equal "http://foo.example.com/posts", render_url_for(controller: :posts)
170+
end
171+
end
172+
173+
test "can specify env[:script_name] when using default_url_options" do
174+
with_default_url_options(host: "foo.example.com") do
175+
@renderer = renderer.new(script_name: "/bar")
176+
assert_equal "http://foo.example.com/bar/posts", render_url_for(controller: :posts)
177+
end
178+
end
179+
180+
test "env[:script_name] overrides default_url_options[:script_name]" do
181+
with_default_url_options(host: "foo.example.com", script_name: "/bar") do
182+
@renderer = renderer.new(script_name: "")
183+
assert_equal "http://foo.example.com/posts", render_url_for(controller: :posts)
184+
end
185+
end
186+
141187
private
142-
def render
143-
@render ||= ApplicationController.renderer.method(:render)
188+
def renderer
189+
@renderer ||= ApplicationController.renderer.new
190+
end
191+
192+
def render(...)
193+
renderer.render(...)
194+
end
195+
196+
def render_url_for(*args)
197+
render inline: "<%= full_url_for(*#{args.inspect}) %>"
198+
end
199+
200+
def with_default_url_options(default_url_options)
201+
original_default_url_options = renderer.controller._routes.default_url_options
202+
renderer.controller._routes.default_url_options = default_url_options
203+
yield
204+
ensure
205+
renderer.controller._routes.default_url_options = original_default_url_options
206+
renderer.controller._routes.default_env # refresh
207+
end
208+
209+
def with_force_ssl(force_ssl = true)
210+
# In a real app, an initializer will set `URL.secure_protocol = app.config.force_ssl`.
211+
original_secure_protocol = ActionDispatch::Http::URL.secure_protocol
212+
ActionDispatch::Http::URL.secure_protocol = force_ssl
213+
yield
214+
ensure
215+
ActionDispatch::Http::URL.secure_protocol = original_secure_protocol
144216
end
145217
end

actiontext/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
* Action Text attachment URLs rendered in a background job (a la Turbo
2+
Streams) now use `Rails.application.default_url_options` and
3+
`Rails.application.config.force_ssl` instead of `http://example.org`.
4+
5+
*Jonathan Hefner*
6+
17
* Focus rich-text editor after calling `fill_in_rich_text_area`
28

39
*Sean Doyle*
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
class BroadcastJob < ApplicationJob
2+
def perform(file, message)
3+
File.write(file, <<~HTML)
4+
<turbo-stream action="replace" target="message_#{message.id}">
5+
<template>#{message.content}</template>
6+
</turbo-stream>
7+
HTML
8+
end
9+
end
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# frozen_string_literal: true
2+
3+
require "test_helper"
4+
5+
class ActionText::JobRenderTest < ActiveJob::TestCase
6+
include Rails::Dom::Testing::Assertions::SelectorAssertions
7+
8+
test "uses app default_url_options" do
9+
blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg")
10+
message = Message.create!(content: ActionText::Content.new.append_attachables(blob))
11+
12+
Dir.mktmpdir do |dir|
13+
file = File.join(dir, "broadcast.html")
14+
15+
BroadcastJob.perform_later(file, message)
16+
17+
with_default_url_options(host: "foo.example.com", port: 9001) do
18+
perform_enqueued_jobs
19+
end
20+
21+
rendered = Nokogiri::HTML::DocumentFragment.parse(File.read(file))
22+
assert_select rendered, "img:match('src', ?)", %r"//foo.example.com:9001/.+/racecar"
23+
end
24+
end
25+
26+
private
27+
def with_default_url_options(default_url_options)
28+
original_default_url_options = Dummy::Application.default_url_options
29+
Dummy::Application.default_url_options = default_url_options
30+
yield
31+
ensure
32+
Dummy::Application.default_url_options = original_default_url_options
33+
end
34+
end

railties/test/application/configuration_test.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,6 +1437,27 @@ def index
14371437
assert_deprecated(Rails.deprecator) { Rails.application.config.enable_dependency_loading = true }
14381438
end
14391439

1440+
test "ActionController::Base::renderer uses Rails.application.default_url_options and config.force_ssl" do
1441+
add_to_config <<~RUBY
1442+
config.force_ssl = true
1443+
1444+
Rails.application.default_url_options = {
1445+
host: "foo.example.com",
1446+
port: 9001,
1447+
script_name: "/bar",
1448+
}
1449+
1450+
routes.prepend do
1451+
resources :posts
1452+
end
1453+
RUBY
1454+
1455+
app "development"
1456+
1457+
posts_url = ApplicationController.renderer.render(inline: "<%= posts_url %>")
1458+
assert_equal "https://foo.example.com:9001/bar/posts", posts_url
1459+
end
1460+
14401461
test "ActionController::Base.raise_on_open_redirects is true by default for new apps" do
14411462
app "development"
14421463

0 commit comments

Comments
 (0)