remove `header=` on the response object.

People should be free to mutate the header object, but not to set a new
header object.  That header object may be specific to the webserver, and
we need to hide it's internals.

3 files changed, 16 insertions, 13 deletions

diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb
index 747d295261..cc1cb3f0f0 100644
--- a/actionpack/lib/action_dispatch/http/cache.rb
+++ b/actionpack/lib/action_dispatch/http/cache.rb
@@ -151,11 +151,11 @@ module ActionDispatch
           control.merge! @cache_control
 
           if control.empty?
-            headers[CACHE_CONTROL] = DEFAULT_CACHE_CONTROL
+            self[CACHE_CONTROL] = DEFAULT_CACHE_CONTROL
           elsif control[:no_cache]
-            headers[CACHE_CONTROL] = NO_CACHE
+            self[CACHE_CONTROL] = NO_CACHE
             if control[:extras]
-              headers[CACHE_CONTROL] += ", #{control[:extras].join(', ')}"
+              self[CACHE_CONTROL] += ", #{control[:extras].join(', ')}"
             end
           else
             extras  = control[:extras]
@@ -167,7 +167,7 @@ module ActionDispatch
             options << MUST_REVALIDATE if control[:must_revalidate]
             options.concat(extras) if extras
 
-            headers[CACHE_CONTROL] = options.join(", ")
+            self[CACHE_CONTROL] = options.join(", ")
           end
         end
       end
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index ea60907c09..aae011fd6a 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -41,9 +41,8 @@ module ActionDispatch # :nodoc:
     attr_writer :sending_file
 
     # Get and set headers for this response.
-    attr_accessor :header
+    attr_reader :header
 
-    alias_method :headers=, :header=
     alias_method :headers,  :header
 
     delegate :[], :[]=, :to => :@header
@@ -117,8 +116,9 @@ module ActionDispatch # :nodoc:
       super()
 
       header = merge_default_headers(header, default_headers)
+      @header = header
 
-      self.body, self.header, self.status = body, header, status
+      self.body, self.status = body, status
 
       @sending_file = false
       @blank        = false
@@ -287,6 +287,7 @@ module ActionDispatch # :nodoc:
     #
     #   status, headers, body = *response
     def to_a
+      commit!
       rack_response @status, @header.to_hash
     end
     alias prepare! to_a
@@ -311,6 +312,9 @@ module ActionDispatch # :nodoc:
   private
 
     def before_committed
+      return if committed?
+      assign_default_content_type_and_charset!
+      handle_conditional_get!
     end
 
     def before_sending
@@ -328,15 +332,15 @@ module ActionDispatch # :nodoc:
       body.respond_to?(:each) ? body : [body]
     end
 
-    def assign_default_content_type_and_charset!(headers)
-      return if headers[CONTENT_TYPE].present?
+    def assign_default_content_type_and_charset!
+      return if self[CONTENT_TYPE].present?
 
       @content_type ||= Mime::HTML
 
       type = @content_type.to_s.dup
       type << "; charset=#{charset}" if append_charset?
 
-      headers[CONTENT_TYPE] = type
+      self[CONTENT_TYPE] = type
     end
 
     def append_charset?
@@ -380,9 +384,6 @@ module ActionDispatch # :nodoc:
     end
 
     def rack_response(status, header)
-      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)
 
       if NO_CONTENT_CODES.include?(@status)
diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb
index 5fbd19acdf..7aca251066 100644
--- a/actionpack/test/dispatch/response_test.rb
+++ b/actionpack/test/dispatch/response_test.rb
@@ -72,12 +72,14 @@ class ResponseTest < ActiveSupport::TestCase
 
   test "content type" do
     [204, 304].each do |c|
+      @response = ActionDispatch::Response.new
       @response.status = c.to_s
       _, headers, _ = @response.to_a
       assert !headers.has_key?("Content-Type"), "#{c} should not have Content-Type header"
     end
 
     [200, 302, 404, 500].each do |c|
+      @response = ActionDispatch::Response.new
       @response.status = c.to_s
       _, headers, _ = @response.to_a
       assert headers.has_key?("Content-Type"), "#{c} did not have Content-Type header"