Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions lib/protocol/http1/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -349,18 +349,25 @@ def read(length)

# Read a line from the connection.
#
# @returns [String | Nil] the line read, or nil if the connection is closed.
# @raises [EOFError] if the connection is closed.
# @returns [String | Nil] the line read, or nil if the connection is gracefully closed.
# @raises [LineLengthError] if the line is too long.
# @raises [ProtocolError] if the line is not terminated properly.
def read_line?
if line = @stream.gets(CRLF, @maximum_line_length)
unless line.chomp!(CRLF)
# This basically means that the request line, response line, header, or chunked length line is too long.
raise LineLengthError, "Line too long!"
if line.bytesize == @maximum_line_length
# This basically means that the request line, response line, header, or chunked length line is too long:
raise LineLengthError, "Line too long!"
else
# This means the line was not terminated properly, which is a protocol violation:
raise ProtocolError, "Line not terminated properly!"
end
end
end

return line

# I considered rescuing Errno::ECONNRESET here, but it seems like that would be ignoring a potentially serious error.
end

# Read a line from the connection.
Expand Down
4 changes: 4 additions & 0 deletions releases.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Releases

## Unreleased

- Tidy up implementation of `read_line?` to handle line length errors and protocol violations more clearly.

## v0.35.0

- Add traces provider for `Protocol::HTTP1::Connection`.
Expand Down
35 changes: 35 additions & 0 deletions test/protocol/http1/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,41 @@
describe Protocol::HTTP1::Connection do
include_context ConnectionContext

with "#read_line?" do
it "reads a line from the stream" do
client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\n\r\n"
client.stream.close

expect(server.read_line?).to be == "GET / HTTP/1.1"
expect(server.read_line?).to be == "Host: localhost"
expect(server.read_line?).to be == ""
expect(server.read_line?).to be_nil
end

it "raises LineLengthError if line is too long" do
# We create a thread since the write is liable to block until we try to read:
thread = Thread.new do
client.stream.write "GET / HTTP/1.#{"1" * 10000}"
client.stream.close
end

expect do
server.read_line?
end.to raise_exception(Protocol::HTTP1::LineLengthError)
ensure
thread.join
end

it "raises ProtocolError if line is not terminated properly" do
client.stream.write "GET / HTTP/1.1\r"
client.stream.close

expect do
server.read_line?
end.to raise_exception(Protocol::HTTP1::ProtocolError)
end
end

with "#read_request" do
it "reads request without body" do
client.stream.write "GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 0\r\n\r\n"
Expand Down
Loading