mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
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 #27144.
This commit is contained in:
parent
49aa974ec8
commit
86754a8f7b
3 changed files with 45 additions and 25 deletions
|
@ -211,7 +211,7 @@ module ActionDispatch
|
||||||
end
|
end
|
||||||
|
|
||||||
if path =~ %r{://}
|
if path =~ %r{://}
|
||||||
path = build_expanded_path(path, request_encoder) do |location|
|
path = build_expanded_path(path) do |location|
|
||||||
https! URI::HTTPS === location if location.scheme
|
https! URI::HTTPS === location if location.scheme
|
||||||
|
|
||||||
if url_host = location.host
|
if url_host = location.host
|
||||||
|
@ -220,8 +220,6 @@ module ActionDispatch
|
||||||
host! url_host
|
host! url_host
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
elsif as
|
|
||||||
path = build_expanded_path(path, request_encoder)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
hostname, port = host.split(":")
|
hostname, port = host.split(":")
|
||||||
|
@ -239,7 +237,7 @@ module ActionDispatch
|
||||||
"HTTP_HOST" => host,
|
"HTTP_HOST" => host,
|
||||||
"REMOTE_ADDR" => remote_addr,
|
"REMOTE_ADDR" => remote_addr,
|
||||||
"CONTENT_TYPE" => request_encoder.content_type,
|
"CONTENT_TYPE" => request_encoder.content_type,
|
||||||
"HTTP_ACCEPT" => accept
|
"HTTP_ACCEPT" => request_encoder.accept_header || accept
|
||||||
}
|
}
|
||||||
|
|
||||||
wrapped_headers = Http::Headers.from_hash({})
|
wrapped_headers = Http::Headers.from_hash({})
|
||||||
|
@ -291,10 +289,10 @@ module ActionDispatch
|
||||||
"#{env['rack.url_scheme']}://#{env['SERVER_NAME']}:#{env['SERVER_PORT']}#{path}"
|
"#{env['rack.url_scheme']}://#{env['SERVER_NAME']}:#{env['SERVER_PORT']}#{path}"
|
||||||
end
|
end
|
||||||
|
|
||||||
def build_expanded_path(path, request_encoder)
|
def build_expanded_path(path)
|
||||||
location = URI.parse(path)
|
location = URI.parse(path)
|
||||||
yield location if block_given?
|
yield location if block_given?
|
||||||
path = request_encoder.append_format_to location.path
|
path = location.path
|
||||||
location.query ? "#{path}?#{location.query}" : path
|
location.query ? "#{path}?#{location.query}" : path
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,10 +1,17 @@
|
||||||
module ActionDispatch
|
module ActionDispatch
|
||||||
class RequestEncoder # :nodoc:
|
class RequestEncoder # :nodoc:
|
||||||
@encoders = {}
|
class IdentityEncoder
|
||||||
|
def content_type; end
|
||||||
|
def accept_header; end
|
||||||
|
def encode_params(params); params; end
|
||||||
|
def response_parser; -> body { body }; end
|
||||||
|
end
|
||||||
|
|
||||||
|
@encoders = { identity: IdentityEncoder.new }
|
||||||
|
|
||||||
attr_reader :response_parser
|
attr_reader :response_parser
|
||||||
|
|
||||||
def initialize(mime_name, param_encoder, response_parser, url_encoded_form = false)
|
def initialize(mime_name, param_encoder, response_parser = nil)
|
||||||
@mime = Mime[mime_name]
|
@mime = Mime[mime_name]
|
||||||
|
|
||||||
unless @mime
|
unless @mime
|
||||||
|
@ -12,21 +19,15 @@ module ActionDispatch
|
||||||
"unregistered MIME Type: #{mime_name}. See `Mime::Type.register`."
|
"unregistered MIME Type: #{mime_name}. See `Mime::Type.register`."
|
||||||
end
|
end
|
||||||
|
|
||||||
@url_encoded_form = url_encoded_form
|
|
||||||
@path_format = ".#{@mime.symbol}" unless @url_encoded_form
|
|
||||||
@response_parser = response_parser || -> body { body }
|
@response_parser = response_parser || -> body { body }
|
||||||
@param_encoder = param_encoder || :"to_#{@mime.symbol}".to_proc
|
@param_encoder = param_encoder || :"to_#{@mime.symbol}".to_proc
|
||||||
end
|
end
|
||||||
|
|
||||||
def append_format_to(path)
|
def content_type
|
||||||
if @url_encoded_form
|
@mime.to_s
|
||||||
path
|
|
||||||
else
|
|
||||||
path + @path_format
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def content_type
|
def accept_header
|
||||||
@mime.to_s
|
@mime.to_s
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -40,7 +41,7 @@ module ActionDispatch
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.encoder(name)
|
def self.encoder(name)
|
||||||
@encoders[name] || WWWFormEncoder
|
@encoders[name] || @encoders[:identity]
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.register_encoder(mime_name, param_encoder: nil, response_parser: nil)
|
def self.register_encoder(mime_name, param_encoder: nil, response_parser: nil)
|
||||||
|
@ -48,7 +49,5 @@ module ActionDispatch
|
||||||
end
|
end
|
||||||
|
|
||||||
register_encoder :json, response_parser: -> body { JSON.parse(body) }
|
register_encoder :json, response_parser: -> body { JSON.parse(body) }
|
||||||
|
|
||||||
WWWFormEncoder = new(:url_encoded_form, -> params { params }, nil, true)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -930,6 +930,10 @@ end
|
||||||
|
|
||||||
class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest
|
class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest
|
||||||
class FooController < ActionController::Base
|
class FooController < ActionController::Base
|
||||||
|
def foos
|
||||||
|
render plain: "ok"
|
||||||
|
end
|
||||||
|
|
||||||
def foos_json
|
def foos_json
|
||||||
render json: params.permit(:foo)
|
render json: params.permit(:foo)
|
||||||
end
|
end
|
||||||
|
@ -958,13 +962,30 @@ class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest
|
||||||
def test_encoding_as_json
|
def test_encoding_as_json
|
||||||
post_to_foos as: :json do
|
post_to_foos as: :json do
|
||||||
assert_response :success
|
assert_response :success
|
||||||
assert_match "foos_json.json", request.path
|
|
||||||
assert_equal "application/json", request.content_type
|
assert_equal "application/json", request.content_type
|
||||||
|
assert_equal "application/json", request.accepts.first.to_s
|
||||||
|
assert_equal :json, request.format.ref
|
||||||
assert_equal({ "foo" => "fighters" }, request.request_parameters)
|
assert_equal({ "foo" => "fighters" }, request.request_parameters)
|
||||||
assert_equal({ "foo" => "fighters" }, response.parsed_body)
|
assert_equal({ "foo" => "fighters" }, response.parsed_body)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_doesnt_mangle_request_path
|
||||||
|
with_routing do |routes|
|
||||||
|
routes.draw do
|
||||||
|
ActiveSupport::Deprecation.silence do
|
||||||
|
post ":action" => FooController
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
post "/foos"
|
||||||
|
assert_equal "/foos", request.path
|
||||||
|
|
||||||
|
post "/foos_json", as: :json
|
||||||
|
assert_equal "/foos_json", request.path
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def test_encoding_as_without_mime_registration
|
def test_encoding_as_without_mime_registration
|
||||||
assert_raise ArgumentError do
|
assert_raise ArgumentError do
|
||||||
ActionDispatch::IntegrationTest.register_encoder :wibble
|
ActionDispatch::IntegrationTest.register_encoder :wibble
|
||||||
|
@ -979,8 +1000,10 @@ class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest
|
||||||
|
|
||||||
post_to_foos as: :wibble do
|
post_to_foos as: :wibble do
|
||||||
assert_response :success
|
assert_response :success
|
||||||
assert_match "foos_wibble.wibble", request.path
|
assert_equal "/foos_wibble", request.path
|
||||||
assert_equal "text/wibble", request.content_type
|
assert_equal "text/wibble", request.content_type
|
||||||
|
assert_equal "text/wibble", request.accepts.first.to_s
|
||||||
|
assert_equal :wibble, request.format.ref
|
||||||
assert_equal Hash.new, request.request_parameters # Unregistered MIME Type can't be parsed.
|
assert_equal Hash.new, request.request_parameters # Unregistered MIME Type can't be parsed.
|
||||||
assert_equal "ok", response.parsed_body
|
assert_equal "ok", response.parsed_body
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue