Better error handling.

Author: ioquatix

lib/protocol/http1/connection.rb

@@ -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.

releases.md

@@ -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`.

test/protocol/http1/connection.rb

@@ -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"