Add SERVER_PROTOCOL to SPEC (#1883)

SPEC currently does not currently specify a way to get the HTTP
version in use.  However, both Chunked and CommonLogger need access
to the http version for correct functioning, and other users in
the rack ecosystem need it as well (Roda needs it, and I've just
identified a need for it in rack-test).

Unicorn, Webrick, and Puma all currently set SERVER_PROTOCOL.
However, Puma currently sets SERVER_PROTOCOL statically to
HTTP/1.1, unlike Unicorn and Webrick, which set it to the
protocol used by the client.  Unicorn and Puma set HTTP_VERSION
to the protocol used by the client.

This specifies that SERVER_PROTOCOL should match the protocol
used by the client, that it should be a valid protocol matching
HTTP/\d(\.\d)?, and that if HTTP_VERSION is provided, it must
match SERVER_PROTOCOL.  This will require minor changes to Puma
to be compliant with the new SPEC.

Set SERVER_PROTOCOL to HTTP/1.1 by default in Rack::MockRequest,
allowing it to be set by the :http_version option. Update
CommonLogger specs to include the version.

This removes a spec in Chunked for usage without SERVER_PROTOCOL.
A comment in the removed lines indicate unicorn will not set
SERVER_PROTOCOL for HTTP/0.9 requests, but that is incorrect, as
unicorn has set SERVER_PROTOCOL to HTTP/0.9 since 2009 (see unicorn
commit bd0599c4ac91d95cae1f34df3ae99c92f3225391).  The related
comment was correct when added in 2009 (rack commit
895beec0622d3cafdc5fbae20d665c6d5f6c8e7c), but has been incorrect
since the code was changed from HTTP_VERSION to SERVER_PROTOCOL in
2015 (rack commit e702d31335c1a820e99c3acdd9d3368ac25da010).

4 files changed, 49 insertions, 11 deletions

diff --git a/test/spec_chunked.rb b/test/spec_chunked.rb
index 3fcd4af2..ec3e4fb2 100644
--- a/test/spec_chunked.rb
+++ b/test/spec_chunked.rb
@@ -113,9 +113,6 @@ describe Rack::Chunked do
       body.join.must_equal 'Hello World!'
     end
 
-    @env.delete('SERVER_PROTOCOL') # unicorn will do this on pre-HTTP/1.0 requests
-    check.call
-
     @env['SERVER_PROTOCOL'] = 'HTTP/0.9' # not sure if this happens in practice
     check.call
   end
diff --git a/test/spec_common_logger.rb b/test/spec_common_logger.rb
index 3d562ae5..09995dda 100644
--- a/test/spec_common_logger.rb
+++ b/test/spec_common_logger.rb
@@ -30,14 +30,14 @@ describe Rack::CommonLogger do
     res = Rack::MockRequest.new(Rack::CommonLogger.new(app)).get("/")
 
     res.errors.wont_be :empty?
-    res.errors.must_match(/"GET \/ " 200 #{length} /)
+    res.errors.must_match(/"GET \/ HTTP\/1\.1" 200 #{length} /)
   end
 
   it "log to anything with +write+" do
     log = StringIO.new
     Rack::MockRequest.new(Rack::CommonLogger.new(app, log)).get("/")
 
-    log.string.must_match(/"GET \/ " 200 #{length} /)
+    log.string.must_match(/"GET \/ HTTP\/1\.1" 200 #{length} /)
   end
 
   it "work with standard library logger" do
@@ -45,21 +45,21 @@ describe Rack::CommonLogger do
     log = Logger.new(logdev)
     Rack::MockRequest.new(Rack::CommonLogger.new(app, log)).get("/")
 
-    logdev.string.must_match(/"GET \/ " 200 #{length} /)
+    logdev.string.must_match(/"GET \/ HTTP\/1\.1" 200 #{length} /)
   end
 
   it "log - content length if header is missing" do
     res = Rack::MockRequest.new(Rack::CommonLogger.new(app_without_length)).get("/")
 
     res.errors.wont_be :empty?
-    res.errors.must_match(/"GET \/ " 200 - /)
+    res.errors.must_match(/"GET \/ HTTP\/1\.1" 200 - /)
   end
 
   it "log - content length if header is zero" do
     res = Rack::MockRequest.new(Rack::CommonLogger.new(app_with_zero_length)).get("/")
 
     res.errors.wont_be :empty?
-    res.errors.must_match(/"GET \/ " 200 - /)
+    res.errors.must_match(/"GET \/ HTTP\/1\.1" 200 - /)
   end
 
   it "log - records host from X-Forwarded-For header" do
@@ -94,7 +94,7 @@ describe Rack::CommonLogger do
       Rack::MockRequest.new(Rack::CommonLogger.new(app, log)).get("/")
     end
 
-    md = /- - - \[([^\]]+)\] "(\w+) \/ " (\d{3}) \d+ ([\d\.]+)/.match(log.string)
+    md = /- - - \[([^\]]+)\] "(\w+) \/ HTTP\/1\.1" (\d{3}) \d+ ([\d\.]+)/.match(log.string)
     md.wont_equal nil
     time, method, status, duration = *md.captures
     time.must_equal Time.at(0).strftime("%d/%b/%Y:%H:%M:%S %z")
@@ -108,7 +108,7 @@ describe Rack::CommonLogger do
     log = Logger.new(logdev)
     Rack::MockRequest.new(Rack::CommonLogger.new(app, log)).get("/hello")
 
-    logdev.string.must_match(/"GET \/hello " 200 #{length} /)
+    logdev.string.must_match(/"GET \/hello HTTP\/1\.1" 200 #{length} /)
   end
 
   it "log path with SCRIPT_NAME" do
@@ -116,7 +116,15 @@ describe Rack::CommonLogger do
     log = Logger.new(logdev)
     Rack::MockRequest.new(Rack::CommonLogger.new(app, log)).get("/path", script_name: "/script")
 
-    logdev.string.must_match(/"GET \/script\/path " 200 #{length} /)
+    logdev.string.must_match(/"GET \/script\/path HTTP\/1\.1" 200 #{length} /)
+  end
+
+  it "log path with SERVER_PROTOCOL" do
+    logdev = StringIO.new
+    log = Logger.new(logdev)
+    Rack::MockRequest.new(Rack::CommonLogger.new(app, log)).get("/path", http_version: "HTTP/1.0")
+
+    logdev.string.must_match(/"GET \/path HTTP\/1\.0" 200 #{length} /)
   end
 
   def length
diff --git a/test/spec_lint.rb b/test/spec_lint.rb
index 06d7ffa4..bd9d5d9b 100755
--- a/test/spec_lint.rb
+++ b/test/spec_lint.rb
@@ -46,6 +46,26 @@ describe Rack::Lint do
     }.must_raise(Rack::Lint::LintError).
       message.must_match(/missing required key SERVER_NAME/)
 
+    lambda {
+      e = env
+      e.delete("SERVER_PROTOCOL")
+      Rack::Lint.new(nil).call(e)
+    }.must_raise(Rack::Lint::LintError).
+      message.must_match(/missing required key SERVER_PROTOCOL/)
+
+    lambda {
+      e = env
+      e["SERVER_PROTOCOL"] = 'Foo'
+      Rack::Lint.new(nil).call(e)
+    }.must_raise(Rack::Lint::LintError).
+      message.must_match(/env\[SERVER_PROTOCOL\] does not match HTTP/)
+
+    lambda {
+      e = env
+      e["HTTP_VERSION"] = 'HTTP/1.0'
+      Rack::Lint.new(nil).call(e)
+    }.must_raise(Rack::Lint::LintError).
+      message.must_match(/env\[HTTP_VERSION\] does not equal env\[SERVER_PROTOCOL\]/)
 
     lambda {
       Rack::Lint.new(nil).call(env("HTTP_CONTENT_TYPE" => "text/plain"))
diff --git a/test/spec_mock.rb b/test/spec_mock.rb
index 70a7c6f2..48dfde06 100644
--- a/test/spec_mock.rb
+++ b/test/spec_mock.rb
@@ -64,6 +64,7 @@ describe Rack::MockRequest do
     env["REQUEST_METHOD"].must_equal "GET"
     env["SERVER_NAME"].must_equal "example.org"
     env["SERVER_PORT"].must_equal "80"
+    env["SERVER_PROTOCOL"].must_equal "HTTP/1.1"
     env["QUERY_STRING"].must_equal ""
     env["PATH_INFO"].must_equal "/"
     env["SCRIPT_NAME"].must_equal ""
@@ -172,6 +173,18 @@ describe Rack::MockRequest do
     env["REQUEST_METHOD"].must_equal "GET"
   end
 
+  it "accept :script_name option to set SCRIPT_NAME" do
+    res = Rack::MockRequest.new(app).get("/", script_name: '/foo')
+    env = YAML.unsafe_load(res.body)
+    env["SCRIPT_NAME"].must_equal "/foo"
+  end
+
+  it "accept :http_version option to set SERVER_PROTOCOL" do
+    res = Rack::MockRequest.new(app).get("/", http_version: 'HTTP/1.0')
+    env = YAML.unsafe_load(res.body)
+    env["SERVER_PROTOCOL"].must_equal "HTTP/1.0"
+  end
+
   it "accept params and build query string for GET requests" do
     res = Rack::MockRequest.new(app).get("/foo?baz=2", params: { foo: { bar: "1" } })
     env = YAML.unsafe_load(res.body)