1
0
Fork 0
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:
Kasper Timm Hansen 2016-11-22 22:22:34 +01:00
parent 49aa974ec8
commit 86754a8f7b
3 changed files with 45 additions and 25 deletions

View file

@ -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

View file

@ -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

View file

@ -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