From e702d31335c1a820e99c3acdd9d3368ac25da010 Mon Sep 17 00:00:00 2001
From: Matt Robenolt <matt@ydekproductions.com>
Date: Fri, 6 Nov 2015 09:13:04 -0800
Subject: Do not reference HTTP_VERSION internally

HTTP_VERSION is supposed to be a client supplied header. This usage
inside Rack is conflating it with SERVER_PROTOCOL, which imo is instead
also conflating it with the client's HTTP version from the request line.

In any of these cases, HTTP_VERSION is set when an existing Version
header doesn't already exist. So it's possible to send a Version header
to conflict with the expected behaviors.

According to the CGI spec
(https://tools.ietf.org/html/draft-robinson-www-interface-00)

>  Environment variables with names beginning with "HTTP_" contain
   header data read from the client, if the protocol used was HTTP.

This is an anscillary issue with Rack, but will leave that open for
discussion since this behavior already exists.
---
 lib/rack/chunked.rb       | 4 ++--
 lib/rack/common_logger.rb | 2 +-
 test/spec_chunked.rb      | 8 ++++----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/rack/chunked.rb b/lib/rack/chunked.rb
index 0c8b4ae8..e7e7d8d1 100644
--- a/lib/rack/chunked.rb
+++ b/lib/rack/chunked.rb
@@ -62,7 +62,7 @@ module Rack
     # a version (nor response headers)
     def chunkable_version?(ver)
       case ver
-      when "HTTP/1.0", nil, "HTTP/0.9"
+      when 'HTTP/1.0', nil, 'HTTP/0.9'
         false
       else
         true
@@ -73,7 +73,7 @@ module Rack
       status, headers, body = @app.call(env)
       headers = HeaderHash.new(headers)
 
-      if ! chunkable_version?(env[HTTP_VERSION]) ||
+      if ! chunkable_version?(env[SERVER_PROTOCOL]) ||
          STATUS_WITH_NO_ENTITY_BODY.key?(status.to_i) ||
          headers[CONTENT_LENGTH] ||
          headers[TRANSFER_ENCODING]
diff --git a/lib/rack/common_logger.rb b/lib/rack/common_logger.rb
index 692b2070..a513ff6e 100644
--- a/lib/rack/common_logger.rb
+++ b/lib/rack/common_logger.rb
@@ -50,7 +50,7 @@ module Rack
         env[REQUEST_METHOD],
         env[PATH_INFO],
         env[QUERY_STRING].empty? ? "" : "?#{env[QUERY_STRING]}",
-        env[HTTP_VERSION],
+        env[SERVER_PROTOCOL],
         status.to_s[0..3],
         length,
         Utils.clock_time - began_at ]
diff --git a/test/spec_chunked.rb b/test/spec_chunked.rb
index daa36cad..23f640a5 100644
--- a/test/spec_chunked.rb
+++ b/test/spec_chunked.rb
@@ -18,7 +18,7 @@ describe Rack::Chunked do
 
   before do
     @env = Rack::MockRequest.
-      env_for('/', 'HTTP_VERSION' => '1.1', 'REQUEST_METHOD' => 'GET')
+      env_for('/', 'SERVER_PROTOCOL' => 'HTTP/1.1', 'REQUEST_METHOD' => 'GET')
   end
 
   class TrailerBody
@@ -81,7 +81,7 @@ describe Rack::Chunked do
 
   it 'not modify response when client is HTTP/1.0' do
     app = lambda { |env| [200, { "Content-Type" => "text/plain" }, ['Hello', ' ', 'World!']] }
-    @env['HTTP_VERSION'] = 'HTTP/1.0'
+    @env['SERVER_PROTOCOL'] = 'HTTP/1.0'
     status, headers, body = chunked(app).call(@env)
     status.must_equal 200
     headers.wont_include 'Transfer-Encoding'
@@ -97,10 +97,10 @@ describe Rack::Chunked do
       body.join.must_equal 'Hello World!'
     end
 
-    @env.delete('HTTP_VERSION') # unicorn will do this on pre-HTTP/1.0 requests
+    @env.delete('SERVER_PROTOCOL') # unicorn will do this on pre-HTTP/1.0 requests
     check.call
 
-    @env['HTTP_VERSION'] = 'HTTP/0.9' # not sure if this happens in practice
+    @env['SERVER_PROTOCOL'] = 'HTTP/0.9' # not sure if this happens in practice
     check.call
   end
 
-- 
cgit v1.2.3-24-ge0c7