Skip to content

Commit 2b38d56

Browse files
committed
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
1 parent e4efb4a commit 2b38d56

File tree

2 files changed

+31
-2
lines changed

2 files changed

+31
-2
lines changed

lib/webrick/httprequest.rb

+9-1
Original file line numberDiff line numberDiff line change
@@ -470,8 +470,13 @@ def read_request_line(socket)
470470

471471
def read_header(socket)
472472
if socket
473+
end_of_headers = false
474+
473475
while line = read_line(socket)
474-
break if /\A#{CRLF}\z/om =~ line
476+
if line == CRLF
477+
end_of_headers = true
478+
break
479+
end
475480
if (@request_bytes += line.bytesize) > MAX_HEADER_LENGTH
476481
raise HTTPStatus::RequestEntityTooLarge, 'headers too large'
477482
end
@@ -480,6 +485,9 @@ def read_header(socket)
480485
end
481486
@raw_header << line
482487
end
488+
489+
# Allow if @header already set to support chunked trailers
490+
raise HTTPStatus::EOFError unless end_of_headers || @header
483491
end
484492
@header = HTTPUtils::parse_header(@raw_header.join)
485493

test/webrick/test_httprequest.rb

+22-1
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ def test_invalid_content_length_header
8686
msg = <<~HTTP.gsub("\n", "\r\n")
8787
GET / HTTP/1.1
8888
Content-Length:#{cl}
89+
8990
HTTP
9091
req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
9192
assert_raise(WEBrick::HTTPStatus::BadRequest){
@@ -101,7 +102,7 @@ def test_bare_lf_request_line
101102
\r
102103
HTTP
103104
req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
104-
assert_raise(WEBrick::HTTPStatus::BadRequest){
105+
assert_raise(WEBrick::HTTPStatus::EOFError){
105106
req.parse(StringIO.new(msg))
106107
}
107108
end
@@ -210,6 +211,7 @@ def test_duplicate_content_length_header
210211
GET / HTTP/1.1
211212
Content-Length: 1
212213
Content-Length: 2
214+
213215
HTTP
214216
req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
215217
assert_raise(WEBrick::HTTPStatus::BadRequest){
@@ -633,6 +635,25 @@ def test_eof_raised_when_line_is_nil
633635
}
634636
end
635637

638+
def test_eof_raised_with_missing_line_between_headers_and_body
639+
msg = <<~HTTP.gsub("\n", "\r\n")
640+
GET / HTTP/1.0
641+
HTTP
642+
req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
643+
assert_raise(WEBrick::HTTPStatus::EOFError) {
644+
req.parse(StringIO.new(msg))
645+
}
646+
647+
msg = <<~HTTP.gsub("\n", "\r\n")
648+
GET / HTTP/1.0
649+
Foo: 1
650+
HTTP
651+
req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
652+
assert_raise(WEBrick::HTTPStatus::EOFError) {
653+
req.parse(StringIO.new(msg))
654+
}
655+
end
656+
636657
def test_cookie_join
637658
req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
638659
req.parse(StringIO.new("GET / HTTP/1.1\r\ncookie: a=1\r\ncookie: b=2\r\n\r\n"))

0 commit comments

Comments
 (0)