From e16afe61abd78c55f80752ca020b90d59ae1940f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 23 Sep 2015 14:39:45 -0700 Subject: [PATCH] stop applying default headers in ActionDispatch::Response I'm making this change so that I can construct response objects that *don't* have the default headers applied. For example, I would like to construct a response object from the return value of a controller. If you need to construct a response object with the default headers, then please use the alternate constructor: `ActionDispatch::Response.create` --- actionpack/CHANGELOG.md | 5 +++++ actionpack/lib/action_controller/metal.rb | 2 +- actionpack/lib/action_controller/metal/live.rb | 8 ++++---- actionpack/lib/action_controller/test_case.rb | 2 +- actionpack/lib/action_dispatch/http/response.rb | 16 ++++++++++------ .../lib/action_dispatch/testing/test_response.rb | 2 +- actionpack/test/dispatch/response_test.rb | 6 +++--- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 4a47c0fdf8..bb15edee63 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,8 @@ +* ActionDispatch::Response#new no longer applies default headers. If you want + default headers applied to the response object, then call + `ActionDispatch::Response.create`. This change only impacts people who are + directly constructing an `ActionDispatch::Response` object. + * Accessing mime types via constants like `Mime::HTML` is deprecated. Please change code like this: diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 0384740fef..3d72755f1d 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -135,7 +135,7 @@ module ActionController end def self.make_response!(request) - ActionDispatch::Response.new.tap do |res| + ActionDispatch::Response.create.tap do |res| res.request = request end end diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 667c7f87ca..c874165816 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -236,6 +236,10 @@ module ActionController end end + def initialize(status = 200, header = {}, body = []) + super(status, Header.new(self, header), body) + end + private def before_committed @@ -257,10 +261,6 @@ module ActionController buf end - def merge_default_headers(original, default) - Header.new self, super - end - def handle_conditional_get! super unless committed? end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 47b6b5d4b3..cf78688126 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -585,7 +585,7 @@ module ActionController end def build_response(klass) - klass.new + klass.create end included do diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index b1b70f7d61..cbeea9e267 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -103,13 +103,21 @@ module ActionDispatch # :nodoc: end end + def self.create(status = 200, header = {}, body = [], default_headers: self.default_headers) + header = merge_default_headers(header, default_headers) + new status, header, body + end + + def self.merge_default_headers(original, default) + default.respond_to?(:merge) ? default.merge(original) : original + end + # The underlying body, as a streamable object. attr_reader :stream - def initialize(status = 200, header = {}, body = [], default_headers: self.class.default_headers) + def initialize(status = 200, header = {}, body = []) super() - header = merge_default_headers(header, default_headers) @header = header self.body, self.status = body, status @@ -345,10 +353,6 @@ module ActionDispatch # :nodoc: def before_sending end - def merge_default_headers(original, default) - default.respond_to?(:merge) ? default.merge(original) : original - end - def build_buffer(response, body) Buffer.new response, body end diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index 6a31d6243f..4b79a90242 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -7,7 +7,7 @@ module ActionDispatch # See Response for more information on controller response objects. class TestResponse < Response def self.from_response(response) - new response.status, response.headers, response.body, default_headers: nil + new response.status, response.headers, response.body end # Was the response successful? diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 795d036e20..85cdcda655 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -4,7 +4,7 @@ require 'rack/content_length' class ResponseTest < ActiveSupport::TestCase def setup - @response = ActionDispatch::Response.new + @response = ActionDispatch::Response.create end def test_can_wait_until_commit @@ -217,7 +217,7 @@ class ResponseTest < ActiveSupport::TestCase 'X-Content-Type-Options' => 'nosniff', 'X-XSS-Protection' => '1;' } - resp = ActionDispatch::Response.new.tap { |response| + resp = ActionDispatch::Response.create.tap { |response| response.body = 'Hello' } resp.to_a @@ -236,7 +236,7 @@ class ResponseTest < ActiveSupport::TestCase ActionDispatch::Response.default_headers = { 'X-XX-XXXX' => 'Here is my phone number' } - resp = ActionDispatch::Response.new.tap { |response| + resp = ActionDispatch::Response.create.tap { |response| response.body = 'Hello' } resp.to_a