[bug] Fixup SERVER_PROTOCOL & HTTP_VERSION headers (#2871)

* Fixup SERVER_PROTOCOL & HTTP_VERSION headers

HTTP_VERSION header can be defined by a client, but it's also used by Rack, Rails, Sinatra, etc.  Change c/ragel code to set SERVER_PROTOCOL to the HTTP protocol defined by the request 'first line', and for now, set HTTP_VERSION to the same.

Note that previously SERVER_PROTOCOL was set to http/1.1, which has been since the start of Puma.

* Tests - replace HTTP_VERSION with SERVER_PROTOCOL

ext/puma_http11/http11_parser.c

@@ -297,7 +297,7 @@ case 13:
 tr18:
 #line 65 "ext/puma_http11/http11_parser.rl"
 	{
-    parser->http_version(parser, PTR_TO(mark), LEN(mark, p));
+    parser->server_protocol(parser, PTR_TO(mark), LEN(mark, p));
   }
 	goto st14;
 tr26:

ext/puma_http11/http11_parser.h

@@ -46,7 +46,7 @@ typedef struct puma_parser {
   element_cb fragment;
   element_cb request_path;
   element_cb query_string;
-  element_cb http_version;
+  element_cb server_protocol;
   element_cb header_done;
 
   char buf[BUFFER_LEN];

ext/puma_http11/http11_parser.java.rl

@@ -39,8 +39,8 @@ public class Http11Parser {
     Http11.query_string(runtime, parser.data, parser.buffer, parser.query_start, fpc-parser.query_start);
   }
 
-  action http_version {
-    Http11.http_version(runtime, parser.data, parser.buffer, parser.mark, fpc-parser.mark);
+  action server_protocol {
+    Http11.server_protocol(runtime, parser.data, parser.buffer, parser.mark, fpc-parser.mark);
   }
 
   action request_path {

ext/puma_http11/http11_parser.rl

@@ -62,8 +62,8 @@ static void snake_upcase_char(char *c)
     parser->query_string(parser, PTR_TO(query_start), LEN(query_start, fpc));
   }
 
-  action http_version {
-    parser->http_version(parser, PTR_TO(mark), LEN(mark, fpc));
+  action server_protocol {
+    parser->server_protocol(parser, PTR_TO(mark), LEN(mark, fpc));
   }
 
   action request_path {

ext/puma_http11/http11_parser_common.rl

@@ -38,8 +38,8 @@
   Method = ( upper | digit | safe ){1,20} >mark %request_method;
 
   http_number = ( digit+ "." digit+ ) ;
-  HTTP_Version = ( "HTTP/" http_number ) >mark %http_version ;
-  Request_Line = ( Method " " Request_URI ("#" Fragment){0,1} " " HTTP_Version CRLF ) ;
+  Server_Protocol = ( "HTTP/" http_number ) >mark %server_protocol ;
+  Request_Line = ( Method " " Request_URI ("#" Fragment){0,1} " " Server_Protocol CRLF ) ;
 
   field_name = ( token -- ":" )+ >start_field $snake_upcase_field %write_field;
 

ext/puma_http11/org/jruby/puma/Http11.java

@@ -46,7 +46,7 @@ public class Http11 extends RubyObject {
     public static final ByteList FRAGMENT_BYTELIST = new ByteList(ByteList.plain("FRAGMENT"));
     public static final ByteList REQUEST_PATH_BYTELIST = new ByteList(ByteList.plain("REQUEST_PATH"));
     public static final ByteList QUERY_STRING_BYTELIST = new ByteList(ByteList.plain("QUERY_STRING"));
-    public static final ByteList HTTP_VERSION_BYTELIST = new ByteList(ByteList.plain("HTTP_VERSION"));
+    public static final ByteList SERVER_PROTOCOL_BYTELIST = new ByteList(ByteList.plain("SERVER_PROTOCOL"));
 
     private static ObjectAllocator ALLOCATOR = new ObjectAllocator() {
         public IRubyObject allocate(Ruby runtime, RubyClass klass) {
@@ -153,9 +153,9 @@ public class Http11 extends RubyObject {
         req.fastASet(RubyString.newStringShared(runtime, QUERY_STRING_BYTELIST),val);
     }
 
-    public static void http_version(Ruby runtime, RubyHash req, ByteList buffer, int at, int length) {
+    public static void server_protocol(Ruby runtime, RubyHash req, ByteList buffer, int at, int length) {
         RubyString val = RubyString.newString(runtime,new ByteList(buffer,at,length));
-        req.fastASet(RubyString.newStringShared(runtime, HTTP_VERSION_BYTELIST),val);
+        req.fastASet(RubyString.newStringShared(runtime, SERVER_PROTOCOL_BYTELIST),val);
     }
 
     public void header_done(Ruby runtime, RubyHash req, ByteList buffer, int at, int length) {

ext/puma_http11/org/jruby/puma/Http11Parser.java

@@ -383,7 +383,7 @@ case 1:
 	case 11:
 // line 42 "ext/puma_http11/http11_parser.java.rl"
 	{
-    Http11.http_version(runtime, parser.data, parser.buffer, parser.mark, p-parser.mark);
+    Http11.server_protocol(runtime, parser.data, parser.buffer, parser.mark, p-parser.mark);
   }
 	break;
 	case 12:

ext/puma_http11/puma_http11.c

@@ -36,7 +36,7 @@ static VALUE global_request_method;
 static VALUE global_request_uri;
 static VALUE global_fragment;
 static VALUE global_query_string;
-static VALUE global_http_version;
+static VALUE global_server_protocol;
 static VALUE global_request_path;
 
 /** Defines common length and error messages for input length validation. */
@@ -244,10 +244,10 @@ void query_string(puma_parser* hp, const char *at, size_t length)
   rb_hash_aset(hp->request, global_query_string, val);
 }
 
-void http_version(puma_parser* hp, const char *at, size_t length)
+void server_protocol(puma_parser* hp, const char *at, size_t length)
 {
   VALUE val = rb_str_new(at, length);
-  rb_hash_aset(hp->request, global_http_version, val);
+  rb_hash_aset(hp->request, global_server_protocol, val);
 }
 
 /** Finalizes the request header to have a bunch of stuff that's
@@ -289,7 +289,7 @@ VALUE HttpParser_alloc(VALUE klass)
   hp->fragment = fragment;
   hp->request_path = request_path;
   hp->query_string = query_string;
-  hp->http_version = http_version;
+  hp->server_protocol = server_protocol;
   hp->header_done = header_done;
   hp->request = Qnil;
 
@@ -469,7 +469,7 @@ void Init_puma_http11(void)
   DEF_GLOBAL(request_uri, "REQUEST_URI");
   DEF_GLOBAL(fragment, "FRAGMENT");
   DEF_GLOBAL(query_string, "QUERY_STRING");
-  DEF_GLOBAL(http_version, "HTTP_VERSION");
+  DEF_GLOBAL(server_protocol, "SERVER_PROTOCOL");
   DEF_GLOBAL(request_path, "REQUEST_PATH");
 
   eHttpParserError = rb_define_class_under(mPuma, "HttpParserError", rb_eIOError);

lib/puma/binder.rb

@@ -51,7 +51,6 @@ module Puma
         # infer properly.
 
         "QUERY_STRING".freeze => "",
-        SERVER_PROTOCOL => HTTP_11,
         SERVER_SOFTWARE => PUMA_SERVER_STRING,
         GATEWAY_INTERFACE => CGI_VER
       }

lib/puma/request.rb

@@ -313,6 +313,10 @@ module Puma
 
         env[REMOTE_ADDR] = addr
       end
+
+      # The legacy HTTP_VERSION header can be sent as a client header.
+      # Rack v4 may remove using HTTP_VERSION.  If so, remove this line.
+      env[HTTP_VERSION] = env[SERVER_PROTOCOL]
     end
     # private :normalize_env
 

test/rackup/env-dump.ru

@@ -0,0 +1,6 @@
+run lambda { |env|
+  body = "#{'─' * 70} Headers\n".dup
+  env.sort.each { |k,v| body << "#{k.ljust 30} #{v}\n" }
+  body << "#{'─' * 78}\n"
+  [200, {"Content-Type" => "text/plain"}, [body]]
+}

test/test_http10.rb

@@ -15,7 +15,7 @@ class Http10ParserTest < Minitest::Test
     assert nread == parser.nread, "Number read returned from execute does not match"
 
     assert_equal '/', req['REQUEST_PATH']
-    assert_equal 'HTTP/1.0', req['HTTP_VERSION']
+    assert_equal 'HTTP/1.0', req['SERVER_PROTOCOL']
     assert_equal '/', req['REQUEST_URI']
     assert_equal 'GET', req['REQUEST_METHOD']
     assert_nil req['FRAGMENT']

test/test_http11.rb

@@ -22,7 +22,7 @@ class Http11ParserTest < Minitest::Test
     assert nread == parser.nread, "Number read returned from execute does not match"
 
     assert_equal '/', req['REQUEST_PATH']
-    assert_equal 'HTTP/1.1', req['HTTP_VERSION']
+    assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL']
     assert_equal '/?a=1', req['REQUEST_URI']
     assert_equal 'GET', req['REQUEST_METHOD']
     assert_nil req['FRAGMENT']
@@ -63,7 +63,7 @@ class Http11ParserTest < Minitest::Test
 
     assert_equal "GET", req['REQUEST_METHOD']
     assert_equal 'http://192.168.1.96:3000/api/v1/matches/test?1=1', req['REQUEST_URI']
-    assert_equal 'HTTP/1.1', req['HTTP_VERSION']
+    assert_equal 'HTTP/1.1', req['SERVER_PROTOCOL']
 
     assert_nil req['REQUEST_PATH']
     assert_nil req['FRAGMENT']