refactor, `Http::Headers` stores headers in env notation

Also:
cleanup, use consistent syntax for `Http::Header` and test.

2 files changed, 42 insertions, 29 deletions

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