Bound HTTP/1 body reads.

lib/protocol/http1/body/chunked.rb

@@ -15,6 +15,9 @@ module Body
 			class Chunked < HTTP::Body::Readable
 				CRLF = "\r\n"
 
+				# The maximum amount of body data returned by a single read.
+				BLOCK_SIZE = 1024 * 64
+
 				# Initialize the chunked body.
 				#
 				# @parameter connection [Protocol::HTTP1::Connection] the connection to read the body from.
@@ -27,6 +30,7 @@ def initialize(connection, headers)
 
 					@length = 0
 					@count = 0
+					@remaining = nil
 				end
 
 				# @attribute [Integer] the number of chunks read so far.
@@ -69,15 +73,39 @@ def close(error = nil)
 				# @returns [String | Nil] the next chunk of data, or `nil` if the body is finished.
 				# @raises [EOFError] if the connection is closed before the expected length is read.
 				def read
-					if !@finished
-						if @connection
+					while !@finished
+						unless @connection
+							raise EOFError, "connection closed before expected length was read!"
+						end
+
+						if @remaining
+							if @remaining > 0
+								chunk = @connection.readpartial([@remaining, BLOCK_SIZE].min)
+								@remaining -= chunk.bytesize
+								@length += chunk.bytesize
+
+								return chunk
+							end
+
+							terminator = @connection.read(CRLF.bytesize)
+
+							unless terminator&.bytesize == CRLF.bytesize
+								raise EOFError, "connection closed before expected length was read!"
+							end
+
+							unless terminator == CRLF
+								raise BadRequest, "Invalid chunk terminator: #{terminator.inspect}"
+							end
+
+							@remaining = nil
+							@count += 1
+						else
 							length, _extensions = @connection.read_line.split(";", 2)
 
 							unless length =~ VALID_CHUNK_LENGTH
 								raise BadRequest, "Invalid chunk length: #{length.inspect}"
 							end
 
-							# It is possible this line contains chunk extension, so we use `to_i` to only consider the initial integral part:
 							length = Integer(length, 16)
 
 							if length == 0
@@ -91,27 +119,16 @@ def read
 								return nil
 							end
 
-							# Read trailing CRLF:
-							chunk = @connection.read(length + 2)
-
-							if chunk.bytesize == length + 2
-								# ...and chomp it off:
-								chunk.chomp!(CRLF)
-
-								@length += length
-								@count += 1
-
-								return chunk
-							else
-								# The connection has been closed before we have read the requested length:
-								@connection.close_read
-								@connection = nil
-							end
+							@remaining = length
 						end
-
-						# If the connection has been closed before we have read the final chunk, raise an error:
-						raise EOFError, "connection closed before expected length was read!"
 					end
+				rescue EOFError
+					if connection = @connection
+						@connection = nil
+						connection.close_read
+					end
+
+					raise EOFError, "connection closed before expected length was read!"
 				end
 
 				# @returns [String] a human-readable representation of the body.

lib/protocol/http1/body/fixed.rb

@@ -10,6 +10,9 @@ module HTTP1
 		module Body
 			# Represents a fixed length body.
 			class Fixed < HTTP::Body::Readable
+				# The maximum amount of body data returned by a single read.
+				BLOCK_SIZE = 1024 * 64
+
 				# Initialize the body with the given connection and length.
 				#
 				# @parameter connection [Protocol::HTTP1::Connection] the connection to read the body from.
@@ -55,7 +58,7 @@ def read
 					if @remaining > 0
 						if @connection
 							# `readpartial` will raise `EOFError` if the connection is finished, or `IOError` if the connection is closed.
-							chunk = @connection.readpartial(@remaining)
+							chunk = @connection.readpartial([@remaining, BLOCK_SIZE].min)
 
 							@remaining -= chunk.bytesize
 

releases.md

@@ -1,5 +1,9 @@
 # Releases
 
+## Unreleased
+
+  - Bound fixed-length and chunked body reads, and validate chunk terminators.
+
 ## v0.40.0
 
   - Use `Protocol::HTTP::Status` for standard HTTP status descriptions and remove the duplicate `Protocol::HTTP1::Reason` table.

test/protocol/http1/body/chunked.rb

@@ -94,6 +94,24 @@
 			expect(connection).to be(:half_closed_remote?)
 		end
 
+		it "bounds the amount of data returned" do
+			content = "x" * (subject::BLOCK_SIZE + 1)
+			buffer = StringIO.new("#{content.bytesize.to_s(16)}\r\n#{content}\r\n0\r\n\r\n")
+			body = subject.new(Protocol::HTTP1::Connection.new(buffer, state: :open), headers)
+
+			expect(body.read.bytesize).to be == subject::BLOCK_SIZE
+			expect(body.read.bytesize).to be == 1
+			expect(body.read).to be_nil
+		end
+
+		it "rejects an invalid chunk terminator" do
+			buffer = StringIO.new("#{content.bytesize.to_s(16)}\r\n#{content}XX")
+			body = subject.new(Protocol::HTTP1::Connection.new(buffer, state: :open), headers)
+
+			expect(body.read).to be == content
+			expect{body.read}.to raise_exception(Protocol::HTTP1::BadRequest)
+		end
+
 		with "trailer" do
 			let(:postfix) {"ETag: abcd\r\n"}
 
@@ -131,6 +149,7 @@
 			let(:buffer) {StringIO.new("#{(content.bytesize + 1).to_s(16)}\r\n#{content}")}
 
 			it "raises error" do
+				expect(body.read).to be == content
 				expect{body.read}.to raise_exception(EOFError)
 
 				expect(connection).to be(:half_closed_remote?)

test/protocol/http1/body/fixed.rb

@@ -85,6 +85,15 @@
 			expect(body).to be(:empty?)
 		end
 
+		it "bounds the amount of data returned" do
+			content = "x" * (subject::BLOCK_SIZE + 1)
+			body = subject.new(Protocol::HTTP1::Connection.new(StringIO.new(content), state: :open), content.bytesize)
+
+			expect(body.read.bytesize).to be == subject::BLOCK_SIZE
+			expect(body.read.bytesize).to be == 1
+			expect(body.read).to be_nil
+		end
+
 		with "length smaller than stream size" do
 			let(:body) {subject.new(connection, 5)}