diff --git a/lib/protocol/http1/body/chunked.rb b/lib/protocol/http1/body/chunked.rb index 75ff159..e1c29ea 100644 --- a/lib/protocol/http1/body/chunked.rb +++ b/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 diff --git a/lib/protocol/http1/body/fixed.rb b/lib/protocol/http1/body/fixed.rb index f5c3f3a..ae9c40b 100644 --- a/lib/protocol/http1/body/fixed.rb +++ b/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 diff --git a/lib/protocol/http1/body/remainder.rb b/lib/protocol/http1/body/remainder.rb index ac87474..2efdced 100644 --- a/lib/protocol/http1/body/remainder.rb +++ b/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 diff --git a/test/protocol/http1/body/chunked.rb b/test/protocol/http1/body/chunked.rb index 4a5f9e1..21488d8 100644 --- a/test/protocol/http1/body/chunked.rb +++ b/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 diff --git a/test/protocol/http1/body/fixed.rb b/test/protocol/http1/body/fixed.rb index f21aacf..516033c 100644 --- a/test/protocol/http1/body/fixed.rb +++ b/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 diff --git a/test/protocol/http1/body/remainder.rb b/test/protocol/http1/body/remainder.rb index 542363a..fba3be8 100644 --- a/test/protocol/http1/body/remainder.rb +++ b/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