Better handling of underlying @stream.

lib/protocol/http1/body/chunked.rb

@@ -23,14 +23,17 @@ def initialize(stream, headers)
 				end
 
 				def empty?
-					@finished
+					@stream.nil?
 				end
 
 				def close(error = nil)
-					# We only close the connection if we haven't completed reading the entire body:
-					unless @finished
-						@stream.close
-						@finished = true
+					if @stream
+						# We only close the connection if we haven't completed reading the entire body:
+						unless @finished
+							@stream.close_read
+						end
+
+						@stream = nil
 					end
 
 					super
@@ -40,35 +43,40 @@ def close(error = nil)
 
 				# Follows the procedure outlined in https://tools.ietf.org/html/rfc7230#section-4.1.3
 				def read
-					return nil if @finished
-
-					length, _extensions = 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
-						@finished = true
-
-						read_trailer
+					if !@finished
+						if @stream
+							length, _extensions = 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
+								read_trailer
+
+								@stream = nil
+								@finished = true
+
+								return nil
+							end
+
+							# Read trailing CRLF:
+							chunk = @stream.read(length + 2)
+
+							# ...and chomp it off:
+							chunk.chomp!(CRLF)
+
+							@length += length
+							@count += 1
+
+							return chunk
+						end
 
-						return nil
+						raise EOFError, "Stream closed before expected length was read!"
 					end
-
-					# Read trailing CRLF:
-					chunk = @stream.read(length + 2)
-
-					# ...and chomp it off:
-					chunk.chomp!(CRLF)
-
-					@length += length
-					@count += 1
-
-					return chunk
 				end
 
 				def inspect

lib/protocol/http1/body/fixed.rb

@@ -19,13 +19,17 @@ def initialize(stream, length)
 				attr :remaining
 
 				def empty?
-					@remaining == 0
+					@stream.nil? or @remaining == 0
 				end
 
 				def close(error = nil)
-					# If we are closing the body without fully reading it, the underlying connection is now in an undefined state.
-					if @remaining != 0
-						@stream.close
+					if @stream
+						# If we are closing the body without fully reading it, the underlying connection is now in an undefined state.
+						if @remaining != 0
+							@stream.close_read
+						end
+
+						@stream = nil
 					end
 
 					super
@@ -34,25 +38,21 @@ def close(error = nil)
 				# @raises EOFError if the stream is closed before the expected length is read.
 				def read
 					if @remaining > 0
-						# `readpartial` will raise `EOFError` if the stream is closed/finished:
-						if chunk = @stream.readpartial(@remaining)
-							@remaining -= chunk.bytesize
-
-							return chunk
+						if @stream
+							# `readpartial` will raise `EOFError` if the stream is closed/finished:
+							if chunk = @stream.readpartial(@remaining)
+								@remaining -= chunk.bytesize
+
+								return chunk
+							end
 						end
+
+						raise EOFError, "Stream closed before expected length was read!"
 					end
 				end
 
-				def join
-					buffer = @stream.read(@remaining)
-
-					@remaining = 0
-
-					return buffer
-				end
-
 				def inspect
-					"\#<#{self.class} length=#{@length} remaining=#{@remaining}>"
+					"\#<#{self.class} length=#{@length} remaining=#{@remaining} state=#{@stream ? 'open' : 'closed'}>"
 				end
 			end
 		end

lib/protocol/http1/body/remainder.rb

@@ -14,47 +14,32 @@ class Remainder < HTTP::Body::Readable
 				# block_size may be removed in the future. It is better managed by stream.
 				def initialize(stream)
 					@stream = stream
-					@empty = false
 				end
 
 				def empty?
-					@empty or @stream.closed?
+					@stream.nil?
 				end
 
 				def close(error = nil)
-					# We can't really do anything in this case except close the connection.
-					@stream.close
-					@empty = true
+					if @stream
+						@stream.close_read
+						# We can't really do anything in this case except close the connection.
+						@stream = nil
+					end
 
 					super
 				end
 
-				# TODO this is a bit less efficient in order to maintain compatibility with `IO`.
 				def read
-					@stream.readpartial(BLOCK_SIZE)
+					@stream&.readpartial(BLOCK_SIZE)
 				rescue EOFError, IOError
-					@empty = true
-
+					@stream = nil
 					# I noticed that in some cases you will get EOFError, and in other cases IOError!?
 					return nil
 				end
 
-				def call(stream)
-					self.each do |chunk|
-						stream.write(chunk)
-					end
-
-					stream.flush
-				end
-
-				def join
-					@stream.read
-				ensure
-					@empty = true
-				end
-
 				def inspect
-					"\#<#{self.class} #{@stream.closed? ? 'closed' : 'open'}>"
+					"\#<#{self.class} state=#{@stream ? 'open' : 'closed'}>"
 				end
 			end
 		end

test/protocol/http1/body/chunked.rb

@@ -25,14 +25,20 @@
 		end
 	end
 
-	with "#stop" do
-		it "closes the stream" do
-			body.close(EOFError)
-			expect(buffer).to be(:closed?)
+	with "#close" do
+		it "invokes close_read on the stream if closing without reading all chunks" do
+			expect(buffer).to receive(:close_read)
+
+			body.close
+
+			expect(body).to be(:empty?)
 		end
 
-		it "marks body as finished" do
+		it "invokes close_read on the stream if closing with an error" do
+			expect(buffer).to receive(:close_read)
+
 			body.close(EOFError)
+
 			expect(body).to be(:empty?)
 		end
 	end

test/protocol/http1/body/fixed.rb

@@ -75,12 +75,17 @@
 	with "#join" do
 		it "returns all content" do
 			expect(body.join).to be == "Hello World"
-			expect(body.join).to be == ""
 		end
 
 		it "updates number of bytes retrieved" do
-			body.read
+			chunk = body.read
+
 			expect(body).to be(:empty?)
+
+			expect(body).to have_attributes(
+				length: be == chunk.bytesize,
+				remaining: be == 0
+			)
 		end
 	end
 end

test/protocol/http1/body/remainder.rb

@@ -59,7 +59,6 @@
 			expect(body).not.to be(:empty?)
 
 			expect(body.join).to be == "Hello World"
-			expect(body.join).to be == ""
 
 			expect(body).to be(:empty?)
 		end