From 23ce9e9b99771080355dccfee6a8453e103682e4 Mon Sep 17 00:00:00 2001 From: David Chen Date: Sun, 31 Jul 2016 15:05:08 +0800 Subject: [PATCH] Fix Accept header overridden when "xhr: true" in integration test In integration test when specify the "Accept" header with "xhr: true" option, the Accept header is overridden with a default xhr Accept header. The issue only affects HTTP header "Accept" but not CGI variable "HTTP_ACCEPT". For example: get '/page', headers: { 'Accept' => 'application/json' }, xhr: true # This is WRONG! And the response.content_type is also affected. # It should be "application/json" assert_equal "text/javascript, text/html, ...", request.accept assert_equal 'text/html', response.content_type The issue is in `ActionDispatch::Integration::RequestHelpers`. When setting "xhr: true" the helper sets a default HTTP_ACCEPT if blank. But the code doesn't consider supporting both HTTP header style and CGI variable style. For detail see this GitHub issue: https://github.com/rails/rails/issues/25859 --- actionpack/CHANGELOG.md | 6 ++++++ .../lib/action_dispatch/testing/integration.rb | 11 +++++++---- actionpack/test/controller/integration_test.rb | 13 +++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 7bb9b62468..ca55a063e7 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,9 @@ +* Don't override the `Accept` header in integration tests when called with `xhr: true`. + + Fixes #25859. + + *David Chen* + * Fix 'defaults' option for root route. A regression from some refactoring for the 5.0 release, this change diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 3008d10992..9be3759556 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -366,14 +366,17 @@ def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: n "HTTP_ACCEPT" => accept } + wrapped_headers = Http::Headers.from_hash({}) + wrapped_headers.merge!(headers) if headers + if xhr - headers["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" - headers["HTTP_ACCEPT"] ||= [Mime[:js], Mime[:html], Mime[:xml], "text/xml", "*/*"].join(", ") + wrapped_headers["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" + wrapped_headers["HTTP_ACCEPT"] ||= [Mime[:js], Mime[:html], Mime[:xml], "text/xml", "*/*"].join(", ") end # this modifies the passed request_env directly - if headers.present? - Http::Headers.from_hash(request_env).merge!(headers) + if wrapped_headers.present? + Http::Headers.from_hash(request_env).merge!(wrapped_headers) end if env.present? Http::Headers.from_hash(request_env).merge!(env) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 0d6a441789..210757fb76 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -403,6 +403,7 @@ def get respond_to do |format| format.html { render plain: "OK", status: 200 } format.js { render plain: "JS OK", status: 200 } + format.json { render json: "JSON OK", status: 200 } format.xml { render xml: "", status: 200 } format.rss { render xml: "", status: 200 } format.atom { render xml: "", status: 200 } @@ -727,6 +728,18 @@ def test_respect_removal_of_default_headers_by_a_controller_action assert_includes @response.headers, "c" end + def test_accept_not_overriden_when_xhr_true + with_test_route_set do + get "/get", headers: { "Accept" => "application/json" }, xhr: true + assert_equal "application/json", request.accept + assert_equal "application/json", response.content_type + + get "/get", headers: { "HTTP_ACCEPT" => "application/json" }, xhr: true + assert_equal "application/json", request.accept + assert_equal "application/json", response.content_type + end + end + private def with_default_headers(headers) original = ActionDispatch::Response.default_headers -- GitLab