From 2b38d5614e876d313fe981e87c4e35b91556d226 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@jeremyevans.net>
Date: Fri, 5 Jul 2024 11:03:21 -0700
Subject: [PATCH] Treat missing CRLF separator after headers as an EOFError

Fix tests that did not have correctly formatted headers.

This changes one test, with a request line that ends in
bare LF instead of CRLF, from raising BadRequest to raising
EOFError, but that seems reasonable.

Fixes #140
---
 lib/webrick/httprequest.rb       | 10 +++++++++-
 test/webrick/test_httprequest.rb | 23 ++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb
index 62ea54c..f6d0b67 100644
--- a/lib/webrick/httprequest.rb
+++ b/lib/webrick/httprequest.rb
@@ -470,8 +470,13 @@ def read_request_line(socket)
 
     def read_header(socket)
       if socket
+        end_of_headers = false
+
         while line = read_line(socket)
-          break if /\A#{CRLF}\z/om =~ line
+          if line == CRLF
+            end_of_headers = true
+            break
+          end
           if (@request_bytes += line.bytesize) > MAX_HEADER_LENGTH
             raise HTTPStatus::RequestEntityTooLarge, 'headers too large'
           end
@@ -480,6 +485,9 @@ def read_header(socket)
           end
           @raw_header << line
         end
+
+        # Allow if @header already set to support chunked trailers
+        raise HTTPStatus::EOFError unless end_of_headers || @header
       end
       @header = HTTPUtils::parse_header(@raw_header.join)
 
diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb
index 84faefc..122a7c1 100644
--- a/test/webrick/test_httprequest.rb
+++ b/test/webrick/test_httprequest.rb
@@ -86,6 +86,7 @@ def test_invalid_content_length_header
       msg = <<~HTTP.gsub("\n", "\r\n")
         GET / HTTP/1.1
         Content-Length:#{cl}
+
       HTTP
       req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
       assert_raise(WEBrick::HTTPStatus::BadRequest){
@@ -101,7 +102,7 @@ def test_bare_lf_request_line
       \r
     HTTP
     req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
-    assert_raise(WEBrick::HTTPStatus::BadRequest){
+    assert_raise(WEBrick::HTTPStatus::EOFError){
       req.parse(StringIO.new(msg))
     }
   end
@@ -210,6 +211,7 @@ def test_duplicate_content_length_header
       GET / HTTP/1.1
       Content-Length: 1
       Content-Length: 2
+
     HTTP
     req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
     assert_raise(WEBrick::HTTPStatus::BadRequest){
@@ -633,6 +635,25 @@ def test_eof_raised_when_line_is_nil
     }
   end
 
+  def test_eof_raised_with_missing_line_between_headers_and_body
+    msg = <<~HTTP.gsub("\n", "\r\n")
+      GET / HTTP/1.0
+    HTTP
+    req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
+    assert_raise(WEBrick::HTTPStatus::EOFError) {
+      req.parse(StringIO.new(msg))
+    }
+
+    msg = <<~HTTP.gsub("\n", "\r\n")
+      GET / HTTP/1.0
+      Foo: 1
+    HTTP
+    req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
+    assert_raise(WEBrick::HTTPStatus::EOFError) {
+      req.parse(StringIO.new(msg))
+    }
+  end
+
   def test_cookie_join
     req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
     req.parse(StringIO.new("GET / HTTP/1.1\r\ncookie: a=1\r\ncookie: b=2\r\n\r\n"))