Skip to content

Commit

Permalink
Do note remove Content-Type when render :body
Browse files Browse the repository at this point in the history
`render :body` should just not set the `Content-Type` header. By
removing the header, it breaks the compatibility with other parts.

After this commit, `render :body` will returns `text/html` content type,
sets by default from `ActionDispatch::Response`, and it will preserve
the overridden content type if you override it.

Fixes #14197, #14238

This partially reverts commit 3047376.
  • Loading branch information
sikachu committed Mar 5, 2014
1 parent 058d3c6 commit ed88a60
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 51 deletions.
4 changes: 2 additions & 2 deletions actionpack/lib/action_controller/metal/rack_delegation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ module ActionController
module RackDelegation
extend ActiveSupport::Concern

delegate :headers, :status=, :location=, :content_type=, :no_content_type=,
:status, :location, :content_type, :no_content_type, :to => "@_response"
delegate :headers, :status=, :location=, :content_type=,
:status, :location, :content_type, :to => "@_response"

def dispatch(action, request)
set_response!(request)
Expand Down
4 changes: 1 addition & 3 deletions actionpack/lib/action_controller/metal/rendering.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ def _render_in_priorities(options)
def _process_format(format, options = {})
super

if options[:body]
self.headers.delete "Content-Type"
elsif options[:plain]
if options[:plain]
self.content_type = Mime::TEXT
else
self.content_type ||= format.to_s
Expand Down
13 changes: 1 addition & 12 deletions actionpack/lib/action_dispatch/http/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ class Response
# content you're giving them, so we need to send that along.
attr_accessor :charset

attr_accessor :no_content_type # :nodoc:

CONTENT_TYPE = "Content-Type".freeze
SET_COOKIE = "Set-Cookie".freeze
LOCATION = "Location".freeze
Expand Down Expand Up @@ -305,17 +303,8 @@ def append_charset?
!@sending_file && @charset != false
end

def remove_content_type!
headers.delete CONTENT_TYPE
end

def rack_response(status, header)
if no_content_type
remove_content_type!
else
assign_default_content_type_and_charset!(header)
end

assign_default_content_type_and_charset!(header)
handle_conditional_get!

header[SET_COOKIE] = header[SET_COOKIE].join("\n") if header[SET_COOKIE].respond_to?(:join)
Expand Down
29 changes: 12 additions & 17 deletions actionpack/test/controller/new_base/render_body_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ def with_custom_layout
render body: "hello world", layout: "greetings"
end

def with_custom_content_type
response.headers['Content-Type'] = 'application/json'
render body: '["troll","face"]'
end

def with_ivar_in_layout
@ivar = "hello world"
render body: "hello world", layout: "ivar"
Expand Down Expand Up @@ -141,6 +146,13 @@ class RenderBodyTest < Rack::TestCase
assert_status 200
end

test "specified content type should not be removed" do
get "/render_body/with_layout/with_custom_content_type"

assert_equal %w{ troll face }, JSON.parse(response.body)
assert_equal 'application/json', response.headers['Content-Type']
end

test "rendering body with layout: false" do
get "/render_body/with_layout/with_layout_false"

Expand All @@ -154,22 +166,5 @@ class RenderBodyTest < Rack::TestCase
assert_body "hello world"
assert_status 200
end

test "rendering from minimal controller returns response with no content type" do
get "/render_body/minimal/index"

assert_header_no_content_type
end

test "rendering from normal controller returns response with no content type" do
get "/render_body/simple/index"

assert_header_no_content_type
end

def assert_header_no_content_type
assert_not response.headers.has_key?("Content-Type"),
%(Expect response not to have Content-Type header, got "#{response.headers["Content-Type"]}")
end
end
end
8 changes: 0 additions & 8 deletions actionpack/test/dispatch/response_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -235,14 +235,6 @@ def test_response_body_encoding
assert_equal @response.body, body.each.to_a.join
end
end

test "does not add default content-type if Content-Type is none" do
resp = ActionDispatch::Response.new.tap { |response|
response.no_content_type = true
}

assert_not resp.headers.has_key?('Content-Type')
end
end

class ResponseIntegrationTest < ActionDispatch::IntegrationTest
Expand Down
5 changes: 3 additions & 2 deletions actionview/lib/action_view/helpers/rendering_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ module RenderingHelper
# * <tt>:html</tt> - Renders the html safe string passed in out, otherwise
# performs html escape on the string first. Setting the content type as
# <tt>text/html</tt>.
# * <tt>:body</tt> - Renders the text passed in, and does not set content
# type in the response.
# * <tt>:body</tt> - Renders the text passed in, and inherits the content
# type of <tt>text/html</tt> from <tt>ActionDispatch::Response</tt>
# object.
#
# If no options hash is passed or :update specified, the default is to render a partial and use the second parameter
# as the locals hash.
Expand Down
5 changes: 0 additions & 5 deletions actionview/lib/action_view/rendering.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,6 @@ def _render_template(options) #:nodoc:
# Assign the rendered format to lookup context.
def _process_format(format, options = {}) #:nodoc:
super

if options[:body]
self.no_content_type = true
end

lookup_context.formats = [format.to_sym]
lookup_context.rendered_format = lookup_context.formats.first
end
Expand Down
7 changes: 5 additions & 2 deletions guides/source/layouts_and_rendering.md
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,13 @@ type, by using the `:body` option to `render`:
render body: "raw"
```

TIP: This option should be used only if you explicitly want the content type to
be unset. Using `:plain` or `:html` might be more appropriate in most of the
TIP: This option should be used only if you don't care about the content type of
the response. Using `:plain` or `:html` might be more appropriate in most of the
time.

NOTE: Unless overriden, your response returned from this render option will be
`text/html`, as that is the default content type of Action Dispatch response.

#### Options for `render`

Calls to the `render` method generally accept four options:
Expand Down

0 comments on commit ed88a60

Please sign in to comment.