diff options
| author | Matt Robenolt <matt@ydekproductions.com> | 2015-11-06 09:13:04 -0800 |
|---|---|---|
| committer | Samuel Williams <samuel.williams@oriontransfer.co.nz> | 2019-11-21 01:17:10 +0900 |
| commit | e702d31335c1a820e99c3acdd9d3368ac25da010 (patch) | |
| tree | 60b1d2c3703afeebbc3cd749dfc46f2acc01e0c1 | |
| parent | d55193de9be3be6cbfa4cc2c632fce31abd23cdf (diff) | |
| download | rack-e702d31335c1a820e99c3acdd9d3368ac25da010.tar.gz | |
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.
| -rw-r--r-- | lib/rack/chunked.rb | 4 | ||||
| -rw-r--r-- | lib/rack/common_logger.rb | 2 | ||||
| -rw-r--r-- | 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 |