Skip to content

Commit 86754a8

Browse files
committed
Use accept header instead of mangling request path.
Instead of appending a format to the request, it's much better to just pass a more appropriate accept header. Rails will figure out the format from that instead. This allows devs to use `:as` on routes that don't have a format. Introduce an `IdentityEncoder` to avoid `if request_encoder`, essentially a better version of the purpose of the `WWWFormEncoder`. One that makes conceptual sense on GET requests too. Fixes rails#27144.
1 parent 49aa974 commit 86754a8

File tree

3 files changed

+44
-24
lines changed

3 files changed

+44
-24
lines changed

actionpack/lib/action_dispatch/testing/integration.rb

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: n
211211
end
212212

213213
if path =~ %r{://}
214-
path = build_expanded_path(path, request_encoder) do |location|
214+
path = build_expanded_path(path) do |location|
215215
https! URI::HTTPS === location if location.scheme
216216

217217
if url_host = location.host
@@ -220,8 +220,6 @@ def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: n
220220
host! url_host
221221
end
222222
end
223-
elsif as
224-
path = build_expanded_path(path, request_encoder)
225223
end
226224

227225
hostname, port = host.split(":")
@@ -239,7 +237,7 @@ def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: n
239237
"HTTP_HOST" => host,
240238
"REMOTE_ADDR" => remote_addr,
241239
"CONTENT_TYPE" => request_encoder.content_type,
242-
"HTTP_ACCEPT" => accept
240+
"HTTP_ACCEPT" => request_encoder.accept_header || accept
243241
}
244242

245243
wrapped_headers = Http::Headers.from_hash({})
@@ -291,10 +289,10 @@ def build_full_uri(path, env)
291289
"#{env['rack.url_scheme']}://#{env['SERVER_NAME']}:#{env['SERVER_PORT']}#{path}"
292290
end
293291

294-
def build_expanded_path(path, request_encoder)
292+
def build_expanded_path(path)
295293
location = URI.parse(path)
296294
yield location if block_given?
297-
path = request_encoder.append_format_to location.path
295+
path = location.path
298296
location.query ? "#{path}?#{location.query}" : path
299297
end
300298
end
Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,33 @@
11
module ActionDispatch
22
class RequestEncoder # :nodoc:
3-
@encoders = {}
3+
class IdentityEncoder
4+
def content_type; end
5+
def accept_header; end
6+
def encode_params(params); params; end
7+
def response_parser; -> body { body }; end
8+
end
9+
10+
@encoders = { identity: IdentityEncoder.new }
411

512
attr_reader :response_parser
613

7-
def initialize(mime_name, param_encoder, response_parser, url_encoded_form = false)
14+
def initialize(mime_name, param_encoder, response_parser = nil)
815
@mime = Mime[mime_name]
916

1017
unless @mime
1118
raise ArgumentError, "Can't register a request encoder for " \
1219
"unregistered MIME Type: #{mime_name}. See `Mime::Type.register`."
1320
end
1421

15-
@url_encoded_form = url_encoded_form
16-
@path_format = ".#{@mime.symbol}" unless @url_encoded_form
17-
@response_parser = response_parser || -> body { body }
18-
@param_encoder = param_encoder || :"to_#{@mime.symbol}".to_proc
22+
@response_parser = response_parser || -> body { body }
23+
@param_encoder = param_encoder || :"to_#{@mime.symbol}".to_proc
1924
end
2025

21-
def append_format_to(path)
22-
if @url_encoded_form
23-
path
24-
else
25-
path + @path_format
26-
end
26+
def content_type
27+
@mime.to_s
2728
end
2829

29-
def content_type
30+
def accept_header
3031
@mime.to_s
3132
end
3233

@@ -40,15 +41,13 @@ def self.parser(content_type)
4041
end
4142

4243
def self.encoder(name)
43-
@encoders[name] || WWWFormEncoder
44+
@encoders[name] || @encoders[:identity]
4445
end
4546

4647
def self.register_encoder(mime_name, param_encoder: nil, response_parser: nil)
4748
@encoders[mime_name] = new(mime_name, param_encoder, response_parser)
4849
end
4950

5051
register_encoder :json, response_parser: -> body { JSON.parse(body) }
51-
52-
WWWFormEncoder = new(:url_encoded_form, -> params { params }, nil, true)
5352
end
5453
end

actionpack/test/controller/integration_test.rb

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,10 @@ def test_cookies_set_in_setup_are_persisted_through_the_session
930930

931931
class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest
932932
class FooController < ActionController::Base
933+
def foos
934+
render plain: "ok"
935+
end
936+
933937
def foos_json
934938
render json: params.permit(:foo)
935939
end
@@ -958,13 +962,30 @@ def test_standard_json_encoding_works
958962
def test_encoding_as_json
959963
post_to_foos as: :json do
960964
assert_response :success
961-
assert_match "foos_json.json", request.path
962965
assert_equal "application/json", request.content_type
966+
assert_equal "application/json", request.accepts.first.to_s
967+
assert_equal :json, request.format.ref
963968
assert_equal({ "foo" => "fighters" }, request.request_parameters)
964969
assert_equal({ "foo" => "fighters" }, response.parsed_body)
965970
end
966971
end
967972

973+
def test_doesnt_mangle_request_path
974+
with_routing do |routes|
975+
routes.draw do
976+
ActiveSupport::Deprecation.silence do
977+
post ":action" => FooController
978+
end
979+
end
980+
981+
post "/foos"
982+
assert_equal "/foos", request.path
983+
984+
post "/foos_json", as: :json
985+
assert_equal "/foos_json", request.path
986+
end
987+
end
988+
968989
def test_encoding_as_without_mime_registration
969990
assert_raise ArgumentError do
970991
ActionDispatch::IntegrationTest.register_encoder :wibble
@@ -979,8 +1000,10 @@ def test_registering_custom_encoder
9791000

9801001
post_to_foos as: :wibble do
9811002
assert_response :success
982-
assert_match "foos_wibble.wibble", request.path
1003+
assert_equal "/foos_wibble", request.path
9831004
assert_equal "text/wibble", request.content_type
1005+
assert_equal "text/wibble", request.accepts.first.to_s
1006+
assert_equal :wibble, request.format.ref
9841007
assert_equal Hash.new, request.request_parameters # Unregistered MIME Type can't be parsed.
9851008
assert_equal "ok", response.parsed_body
9861009
end

0 commit comments

Comments
 (0)