From 8945be464feb8c9ec8c4e7be52e5195f17a1ef5e Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Wed, 13 Mar 2013 11:14:49 +0100 Subject: Http::Headers respects headers that are not prefixed with HTTP_ --- actionpack/CHANGELOG.md | 4 ++++ actionpack/lib/action_dispatch/http/headers.rb | 14 +++++++++++++- actionpack/test/dispatch/header_test.rb | 19 +++++++++++++------ 3 files changed, 30 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 157a038b7c..2e31358f87 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,9 @@ ## Rails 4.0.0 (unreleased) ## +* Http::Headers respects headers that are not prefixed with HTTP_ + + *Yves Senn* + * Fix incorrectly appended square brackets to a multiple select box if an explicit name has been given and it already ends with "[]" diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index dc04d4577b..187e83cbd9 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -1,6 +1,16 @@ module ActionDispatch module Http class Headers + NON_PREFIX_VARIABLES = %w( + CONTENT_TYPE CONTENT_LENGTH + HTTPS AUTH_TYPE GATEWAY_INTERFACE + PATH_INFO PATH_TRANSLATED QUERY_STRING + REMOTE_ADDR REMOTE_HOST REMOTE_IDENT REMOTE_USER + REQUEST_METHOD SCRIPT_NAME + SERVER_NAME SERVER_PORT SERVER_PROTOCOL SERVER_SOFTWARE + ) + HEADER_REGEXP = /\A[A-Za-z-]+\z/ + include Enumerable def initialize(env = {}) @@ -32,7 +42,9 @@ module ActionDispatch end def cgi_name(k) - "HTTP_#{k.upcase.gsub(/-/, '_')}" + k = k.upcase.gsub(/-/, '_') + k = "HTTP_#{k.upcase.gsub(/-/, '_')}" unless NON_PREFIX_VARIABLES.include?(k) + k end end end diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index 42432510c3..d3fc83ca12 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -3,14 +3,16 @@ require 'abstract_unit' class HeaderTest < ActiveSupport::TestCase def setup @headers = ActionDispatch::Http::Headers.new( - "HTTP_CONTENT_TYPE" => "text/plain" + "CONTENT_TYPE" => "text/plain", + "HTTP_REFERER" => "/some/page" ) end def test_each headers = [] @headers.each { |pair| headers << pair } - assert_equal [["HTTP_CONTENT_TYPE", "text/plain"]], headers + assert_equal [["CONTENT_TYPE", "text/plain"], + ["HTTP_REFERER", "/some/page"]], headers end def test_setter @@ -19,19 +21,24 @@ class HeaderTest < ActiveSupport::TestCase end def test_key? - assert @headers.key?('HTTP_CONTENT_TYPE') - assert @headers.include?('HTTP_CONTENT_TYPE') + assert @headers.key?('CONTENT_TYPE') + assert @headers.include?('CONTENT_TYPE') end def test_fetch_with_block assert_equal 'omg', @headers.fetch('notthere') { 'omg' } end - test "content type" do + test "accessing http header" do + assert_equal "/some/page", @headers["Referer"] + assert_equal "/some/page", @headers["referer"] + assert_equal "/some/page", @headers["HTTP_REFERER"] + end + + test "accessing special header" do assert_equal "text/plain", @headers["Content-Type"] assert_equal "text/plain", @headers["content-type"] assert_equal "text/plain", @headers["CONTENT_TYPE"] - assert_equal "text/plain", @headers["HTTP_CONTENT_TYPE"] end test "fetch" do -- cgit v1.2.3 From e2a5de2bb200773943e605cdddb9b18bbfa77e13 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Wed, 13 Mar 2013 11:30:45 +0100 Subject: refactor, `Http::Headers` stores headers in env notation Also: cleanup, use consistent syntax for `Http::Header` and test. --- actionpack/lib/action_dispatch/http/headers.rb | 38 ++++++++++++++------------ actionpack/test/dispatch/header_test.rb | 33 ++++++++++++++-------- 2 files changed, 42 insertions(+), 29 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 187e83cbd9..ecf050a869 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -14,37 +14,41 @@ module ActionDispatch include Enumerable def initialize(env = {}) - @headers = env + @env = env end - def [](header_name) - @headers[env_name(header_name)] + def [](key) + @env[env_name(key)] end - def []=(k,v); @headers[k] = v; end - def key?(k); @headers.key? k; end + def []=(key, value) + @env[env_name(key)] = value + end + + def key?(key); @env.key? key; end alias :include? :key? - def fetch(header_name, *args, &block) - @headers.fetch env_name(header_name), *args, &block + def fetch(key, *args, &block) + @env.fetch env_name(key), *args, &block end def each(&block) - @headers.each(&block) + @env.each(&block) end private - - # Converts a HTTP header name to an environment variable name if it is - # not contained within the headers hash. - def env_name(header_name) - @headers.include?(header_name) ? header_name : cgi_name(header_name) + def env_name(key) + if key =~ HEADER_REGEXP + cgi_name(key) + else + key + end end - def cgi_name(k) - k = k.upcase.gsub(/-/, '_') - k = "HTTP_#{k.upcase.gsub(/-/, '_')}" unless NON_PREFIX_VARIABLES.include?(k) - k + def cgi_name(key) + key = key.upcase.gsub(/-/, '_') + key = "HTTP_#{key}" unless NON_PREFIX_VARIABLES.include?(key) + key end end end diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index d3fc83ca12..9f92c3791e 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -1,32 +1,41 @@ -require 'abstract_unit' +require "abstract_unit" class HeaderTest < ActiveSupport::TestCase - def setup + setup do @headers = ActionDispatch::Http::Headers.new( "CONTENT_TYPE" => "text/plain", "HTTP_REFERER" => "/some/page" ) end - def test_each + test "each" do headers = [] @headers.each { |pair| headers << pair } assert_equal [["CONTENT_TYPE", "text/plain"], ["HTTP_REFERER", "/some/page"]], headers end - def test_setter - @headers['foo'] = "bar" - assert_equal "bar", @headers['foo'] + test "set new headers" do + @headers["Host"] = "127.0.0.1" + + assert_equal "127.0.0.1", @headers["Host"] + assert_equal "127.0.0.1", @headers["HTTP_HOST"] + end + + test "set new env variables" do + @headers["HTTP_HOST"] = "127.0.0.1" + + assert_equal "127.0.0.1", @headers["Host"] + assert_equal "127.0.0.1", @headers["HTTP_HOST"] end - def test_key? - assert @headers.key?('CONTENT_TYPE') - assert @headers.include?('CONTENT_TYPE') + test "key?" do + assert @headers.key?("CONTENT_TYPE") + assert @headers.include?("CONTENT_TYPE") end - def test_fetch_with_block - assert_equal 'omg', @headers.fetch('notthere') { 'omg' } + test "fetch with block" do + assert_equal "omg", @headers.fetch("notthere") { "omg" } end test "accessing http header" do @@ -43,6 +52,6 @@ class HeaderTest < ActiveSupport::TestCase test "fetch" do assert_equal "text/plain", @headers.fetch("content-type", nil) - assert_equal "not found", @headers.fetch('not-found', 'not found') + assert_equal "not found", @headers.fetch("not-found", "not found") end end -- cgit v1.2.3 From 9af59b2468e4ad6c3c2ca89f90968fdcaa417aba Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Wed, 13 Mar 2013 12:25:32 +0100 Subject: allow headers and env to be passed in `IntegrationTest`. Closes #6513. --- actionpack/CHANGELOG.md | 11 ++++ actionpack/lib/action_dispatch/http/headers.rb | 16 ++++- .../lib/action_dispatch/testing/integration.rb | 68 +++++++++++----------- actionpack/test/controller/integration_test.rb | 15 +++++ actionpack/test/dispatch/header_test.rb | 45 +++++++++++++- 5 files changed, 119 insertions(+), 36 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 2e31358f87..3eb40a586e 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,16 @@ ## Rails 4.0.0 (unreleased) ## +* `ActionDispatch::IntegrationTest` allows headers and rack env + variables to be passed when performing requests. + Fixes #6513. + + Example: + + get "/success", {}, "HTTP_REFERER" => "http://test.com/", + "Host" => "http://test.com" + + *Yves Senn* + * Http::Headers respects headers that are not prefixed with HTTP_ *Yves Senn* diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index ecf050a869..3f3f71b280 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -12,9 +12,11 @@ module ActionDispatch HEADER_REGEXP = /\A[A-Za-z-]+\z/ include Enumerable + attr_reader :env def initialize(env = {}) - @env = env + @env = {} + merge!(env) end def [](key) @@ -36,6 +38,18 @@ module ActionDispatch @env.each(&block) end + def merge(headers_or_env) + headers = Http::Headers.new(env.dup) + headers.merge!(headers_or_env) + headers + end + + def merge!(headers_or_env) + headers_or_env.each do |key, value| + self[env_name(key)] = value + end + end + private def env_name(key) if key =~ HEADER_REGEXP diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index ed4e88aab6..ae1b0b5dea 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -17,7 +17,7 @@ module ActionDispatch # a Hash, or a String that is appropriately encoded # (application/x-www-form-urlencoded or # multipart/form-data). - # - +headers+: Additional headers to pass, as a Hash. The headers will be + # - +headers_or_env+: Additional headers to pass, as a Hash. The headers will be # merged into the Rack env hash. # # This method returns a Response object, which one can use to @@ -28,44 +28,44 @@ module ActionDispatch # # You can also perform POST, PATCH, PUT, DELETE, and HEAD requests with # +#post+, +#patch+, +#put+, +#delete+, and +#head+. - def get(path, parameters = nil, headers = nil) - process :get, path, parameters, headers + def get(path, parameters = nil, headers_or_env = nil) + process :get, path, parameters, headers_or_env end # Performs a POST request with the given parameters. See +#get+ for more # details. - def post(path, parameters = nil, headers = nil) - process :post, path, parameters, headers + def post(path, parameters = nil, headers_or_env = nil) + process :post, path, parameters, headers_or_env end # Performs a PATCH request with the given parameters. See +#get+ for more # details. - def patch(path, parameters = nil, headers = nil) - process :patch, path, parameters, headers + def patch(path, parameters = nil, headers_or_env = nil) + process :patch, path, parameters, headers_or_env end # Performs a PUT request with the given parameters. See +#get+ for more # details. - def put(path, parameters = nil, headers = nil) - process :put, path, parameters, headers + def put(path, parameters = nil, headers_or_env = nil) + process :put, path, parameters, headers_or_env end # Performs a DELETE request with the given parameters. See +#get+ for # more details. - def delete(path, parameters = nil, headers = nil) - process :delete, path, parameters, headers + def delete(path, parameters = nil, headers_or_env = nil) + process :delete, path, parameters, headers_or_env end # Performs a HEAD request with the given parameters. See +#get+ for more # details. - def head(path, parameters = nil, headers = nil) - process :head, path, parameters, headers + def head(path, parameters = nil, headers_or_env = nil) + process :head, path, parameters, headers_or_env end # Performs a OPTIONS request with the given parameters. See +#get+ for # more details. - def options(path, parameters = nil, headers = nil) - process :options, path, parameters, headers + def options(path, parameters = nil, headers_or_env = nil) + process :options, path, parameters, headers_or_env end # Performs an XMLHttpRequest request with the given parameters, mirroring @@ -74,11 +74,11 @@ module ActionDispatch # The request_method is +:get+, +:post+, +:patch+, +:put+, +:delete+ or # +:head+; the parameters are +nil+, a hash, or a url-encoded or multipart # string; the headers are a hash. - def xml_http_request(request_method, path, parameters = nil, headers = nil) - headers ||= {} - headers['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' - headers['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') - process(request_method, path, parameters, headers) + def xml_http_request(request_method, path, parameters = nil, headers_or_env = nil) + headers_or_env ||= {} + headers_or_env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' + headers_or_env['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') + process(request_method, path, parameters, headers_or_env) end alias xhr :xml_http_request @@ -95,40 +95,40 @@ module ActionDispatch # redirect. Note that the redirects are followed until the response is # not a redirect--this means you may run into an infinite loop if your # redirect loops back to itself. - def request_via_redirect(http_method, path, parameters = nil, headers = nil) - process(http_method, path, parameters, headers) + def request_via_redirect(http_method, path, parameters = nil, headers_or_env = nil) + process(http_method, path, parameters, headers_or_env) follow_redirect! while redirect? status end # Performs a GET request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def get_via_redirect(path, parameters = nil, headers = nil) - request_via_redirect(:get, path, parameters, headers) + def get_via_redirect(path, parameters = nil, headers_or_env = nil) + request_via_redirect(:get, path, parameters, headers_or_env) end # Performs a POST request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def post_via_redirect(path, parameters = nil, headers = nil) - request_via_redirect(:post, path, parameters, headers) + def post_via_redirect(path, parameters = nil, headers_or_env = nil) + request_via_redirect(:post, path, parameters, headers_or_env) end # Performs a PATCH request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def patch_via_redirect(path, parameters = nil, headers = nil) - request_via_redirect(:patch, path, parameters, headers) + def patch_via_redirect(path, parameters = nil, headers_or_env = nil) + request_via_redirect(:patch, path, parameters, headers_or_env) end # Performs a PUT request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def put_via_redirect(path, parameters = nil, headers = nil) - request_via_redirect(:put, path, parameters, headers) + def put_via_redirect(path, parameters = nil, headers_or_env = nil) + request_via_redirect(:put, path, parameters, headers_or_env) end # Performs a DELETE request, following any subsequent redirect. # See +request_via_redirect+ for more information. - def delete_via_redirect(path, parameters = nil, headers = nil) - request_via_redirect(:delete, path, parameters, headers) + def delete_via_redirect(path, parameters = nil, headers_or_env = nil) + request_via_redirect(:delete, path, parameters, headers_or_env) end end @@ -268,8 +268,8 @@ module ActionDispatch end # Performs the actual request. - def process(method, path, parameters = nil, rack_env = nil) - rack_env ||= {} + def process(method, path, parameters = nil, headers_or_env = nil) + rack_env = Http::Headers.new(headers_or_env || {}).env if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 72b882539c..c3bdf74d93 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -573,6 +573,21 @@ class MetalIntegrationTest < ActionDispatch::IntegrationTest def test_generate_url_without_controller assert_equal 'http://www.example.com/foo', url_for(:controller => "foo") end + + def test_pass_headers + get "/success", {}, "Referer" => "http://www.example.com/foo", "Host" => "http://nohost.com" + + assert_equal "http://nohost.com", @request.env["HTTP_HOST"] + assert_equal "http://www.example.com/foo", @request.env["HTTP_REFERER"] + end + + def test_pass_env + get "/success", {}, "HTTP_REFERER" => "http://test.com/", "HTTP_HOST" => "http://test.com" + + assert_equal "http://test.com", @request.env["HTTP_HOST"] + assert_equal "http://test.com/", @request.env["HTTP_REFERER"] + end + end class ApplicationIntegrationTest < ActionDispatch::IntegrationTest diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index 9f92c3791e..0bfc18b4ed 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -8,7 +8,23 @@ class HeaderTest < ActiveSupport::TestCase ) end - test "each" do + test "#new with mixed headers and env" do + headers = ActionDispatch::Http::Headers.new( + "Content-Type" => "application/json", + "HTTP_REFERER" => "/some/page", + "Host" => "http://test.com") + + assert_equal({"CONTENT_TYPE" => "application/json", + "HTTP_REFERER" => "/some/page", + "HTTP_HOST" => "http://test.com"}, headers.env) + end + + test "#env returns the headers as env variables" do + assert_equal({"CONTENT_TYPE" => "text/plain", + "HTTP_REFERER" => "/some/page"}, @headers.env) + end + + test "#each iterates through the env variables" do headers = [] @headers.each { |pair| headers << pair } assert_equal [["CONTENT_TYPE", "text/plain"], @@ -54,4 +70,31 @@ class HeaderTest < ActiveSupport::TestCase assert_equal "text/plain", @headers.fetch("content-type", nil) assert_equal "not found", @headers.fetch("not-found", "not found") end + + test "#merge! headers with mutation" do + @headers.merge!("Host" => "http://example.test", + "Content-Type" => "text/html") + assert_equal({"HTTP_HOST" => "http://example.test", + "CONTENT_TYPE" => "text/html", + "HTTP_REFERER" => "/some/page"}, @headers.env) + end + + test "#merge! env with mutation" do + @headers.merge!("HTTP_HOST" => "http://first.com", + "CONTENT_TYPE" => "text/html") + assert_equal({"HTTP_HOST" => "http://first.com", + "CONTENT_TYPE" => "text/html", + "HTTP_REFERER" => "/some/page"}, @headers.env) + end + + test "merge without mutation" do + combined = @headers.merge("HTTP_HOST" => "http://example.com", + "CONTENT_TYPE" => "text/html") + assert_equal({"HTTP_HOST" => "http://example.com", + "CONTENT_TYPE" => "text/html", + "HTTP_REFERER" => "/some/page"}, combined.env) + + assert_equal({"CONTENT_TYPE" => "text/plain", + "HTTP_REFERER" => "/some/page"}, @headers.env) + end end -- cgit v1.2.3 From a709246d1739c44e04b00412e7a4ec09c1500fc3 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Wed, 13 Mar 2013 16:25:28 +0100 Subject: `Http::Headers` respects dotted env vars, symbols, headers with numbers. --- actionpack/lib/action_dispatch/http/headers.rb | 17 ++++++---------- actionpack/test/dispatch/header_test.rb | 27 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 11 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 3f3f71b280..1574518a16 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -1,7 +1,7 @@ module ActionDispatch module Http class Headers - NON_PREFIX_VARIABLES = %w( + CGI_VARIABLES = %w( CONTENT_TYPE CONTENT_LENGTH HTTPS AUTH_TYPE GATEWAY_INTERFACE PATH_INFO PATH_TRANSLATED QUERY_STRING @@ -9,7 +9,7 @@ module ActionDispatch REQUEST_METHOD SCRIPT_NAME SERVER_NAME SERVER_PORT SERVER_PROTOCOL SERVER_SOFTWARE ) - HEADER_REGEXP = /\A[A-Za-z-]+\z/ + HTTP_HEADER = /\A[A-Za-z0-9-]+\z/ include Enumerable attr_reader :env @@ -52,16 +52,11 @@ module ActionDispatch private def env_name(key) - if key =~ HEADER_REGEXP - cgi_name(key) - else - key + key = key.to_s + if key =~ HTTP_HEADER + key = key.upcase.tr('-', '_') + key = "HTTP_" + key unless CGI_VARIABLES.include?(key) end - end - - def cgi_name(key) - key = key.upcase.gsub(/-/, '_') - key = "HTTP_#{key}" unless NON_PREFIX_VARIABLES.include?(key) key end end diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index 0bfc18b4ed..3bb3b3db23 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -38,6 +38,13 @@ class HeaderTest < ActiveSupport::TestCase assert_equal "127.0.0.1", @headers["HTTP_HOST"] end + test "headers can contain numbers" do + @headers["Content-MD5"] = "Q2hlY2sgSW50ZWdyaXR5IQ==" + + assert_equal "Q2hlY2sgSW50ZWdyaXR5IQ==", @headers["Content-MD5"] + assert_equal "Q2hlY2sgSW50ZWdyaXR5IQ==", @headers["HTTP_CONTENT_MD5"] + end + test "set new env variables" do @headers["HTTP_HOST"] = "127.0.0.1" @@ -97,4 +104,24 @@ class HeaderTest < ActiveSupport::TestCase assert_equal({"CONTENT_TYPE" => "text/plain", "HTTP_REFERER" => "/some/page"}, @headers.env) end + + test "env variables with . are not modified" do + headers = ActionDispatch::Http::Headers.new + headers.merge! "rack.input" => "", + "rack.request.cookie_hash" => "", + "action_dispatch.logger" => "" + + assert_equal(["action_dispatch.logger", + "rack.input", + "rack.request.cookie_hash"], headers.env.keys.sort) + end + + test "symbols are treated as strings" do + headers = ActionDispatch::Http::Headers.new(:SERVER_NAME => "example.com", + "HTTP_REFERER" => "/", + :Host => "test.com") + assert_equal "example.com", headers["SERVER_NAME"] + assert_equal "/", headers[:HTTP_REFERER] + assert_equal "test.com", headers["HTTP_HOST"] + end end -- cgit v1.2.3