Skip to content
Browse files

Add `#no_content_type` attribute to `AD::Response`

Setting this attribute to `true` will remove the content type header
from the request. This is use in `render :body` feature.
  • Loading branch information...
1 parent 9fe506e commit 3047376870d4a7adc7ff15c3cb4852e073c8f1da @sikachu sikachu committed Feb 14, 2014
View
4 actionpack/lib/action_controller/metal/rack_delegation.rb
@@ -5,8 +5,8 @@ module ActionController
module RackDelegation
extend ActiveSupport::Concern
- delegate :headers, :status=, :location=, :content_type=,
- :status, :location, :content_type, :to => "@_response"
+ delegate :headers, :status=, :location=, :content_type=, :no_content_type=,
+ :status, :location, :content_type, :no_content_type, :to => "@_response"
def dispatch(action, request)
set_response!(request)
View
10 actionpack/lib/action_controller/metal/rendering.rb
@@ -44,15 +44,13 @@ def _render_in_priorities(options)
def _process_format(format, options = {})
super
- self.content_type ||= format.to_s
- if options[:body].present?
- self.content_type = "none"
+ if options[:body]
self.headers.delete "Content-Type"
- end
-
- if options[:plain].present?
+ elsif options[:plain]
self.content_type = Mime::TEXT
+ else
+ self.content_type ||= format.to_s
end
end
View
15 actionpack/lib/action_dispatch/http/response.rb
@@ -63,6 +63,8 @@ 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
@@ -288,7 +290,7 @@ def munge_body_object(body)
end
def assign_default_content_type_and_charset!(headers)
- return if headers[CONTENT_TYPE].present? || @content_type == "none"
+ return if headers[CONTENT_TYPE].present?
@content_type ||= Mime::HTML
@charset ||= self.class.default_charset unless @charset == false
@@ -303,8 +305,17 @@ def append_charset?
!@sending_file && @charset != false
end
+ def remove_content_type!
+ headers.delete CONTENT_TYPE
@tenderlove
Ruby on Rails member
tenderlove added a note Feb 28, 2014

Why are we mutating the header hash here? It didn't used to do that.

If you're using a streaming response, this will break because the header hash is frozen (since it has already been sent to the client).

Also, what if someone specifically set a content type in their response? Do we actually want to delete it? For example:

class MainController < ApplicationController
  def index
    response.headers['Content-Type'] = 'application/json'
    render body: JSON.dump([1,2,3,4])
  end
end

I know we have a render json thing, but this is just an example. Are we sure unconditionally rm'ing the content type header is The Right Thing™?

/cc @jeremy

@jeremy
Ruby on Rails member
jeremy added a note Mar 1, 2014

:+1: we want to not set the default content type, but we don't want to erase the content type you provided—that's precisely why people would be using render body: ... :grin:

@sikachu
Ruby on Rails member
sikachu added a note Mar 1, 2014

Yep, this patch is going too far. Going to revert this then :sweat_smile:.

See #14238 for more detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
+
def rack_response(status, header)
- assign_default_content_type_and_charset!(header)
+ if no_content_type
+ remove_content_type!
+ else
+ assign_default_content_type_and_charset!(header)
+ end
+
handle_conditional_get!
header[SET_COOKIE] = header[SET_COOKIE].join("\n") if header[SET_COOKIE].respond_to?(:join)
View
2 actionpack/test/dispatch/response_test.rb
@@ -238,7 +238,7 @@ def test_response_body_encoding
test "does not add default content-type if Content-Type is none" do
resp = ActionDispatch::Response.new.tap { |response|
- response.content_type = 'none'
+ response.no_content_type = true
}
assert_not resp.headers.has_key?('Content-Type')
View
5 actionview/lib/action_view/rendering.rb
@@ -102,6 +102,11 @@ 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

0 comments on commit 3047376

Please sign in to comment.
Something went wrong with that request. Please try again.