Allow http_parser to decide if there is still body to read

It mostly fixes problem when connection which uses "Transfer-Encoding: chunked" header is closed to early before the terminating chunk is received. In such case "http" returned partial body instead of raising error.
httprb · Aug 3, 2017 · e101a4d · e101a4d
1 parent 256539e
commit e101a4d
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 10 deletions.
diff --git a/lib/http/connection.rb b/lib/http/connection.rb
@@ -211,6 +211,7 @@ def read_more(size)
 
       value = @socket.readpartial(size)
       if value == :eof
+        @parser << ""
         :eof
       elsif value
         @parser << value

diff --git a/lib/http/response.rb b/lib/http/response.rb
@@ -50,7 +50,7 @@ def initialize(opts)
         encoding   = opts[:encoding] || charset || Encoding::BINARY
         stream     = body_stream_for(connection, opts)
 
-        @body = Response::Body.new(stream, :encoding => encoding, :length => content_length)
+        @body = Response::Body.new(stream, :encoding => encoding)
       else
         @body = opts.fetch(:body)
       end

diff --git a/lib/http/response/body.rb b/lib/http/response/body.rb
@@ -15,13 +15,12 @@ class Body
       # @return [HTTP::Connection]
       attr_reader :connection
 
-      def initialize(stream, length: nil, encoding: Encoding::BINARY)
+      def initialize(stream, encoding: Encoding::BINARY)
         @stream     = stream
         @connection = stream.is_a?(Inflater) ? stream.connection : stream
         @streaming  = nil
         @contents   = nil
         @encoding   = find_encoding(encoding)
-        @length     = length || Float::INFINITY
       end
 
       # (see HTTP::Client#readpartial)
@@ -48,10 +47,7 @@ def to_s
           @streaming  = false
           @contents   = String.new("").force_encoding(@encoding)
 
-          length = @length
-
-          while length > 0 && (chunk = @stream.readpartial)
-            length -= chunk.bytesize
+          while (chunk = @stream.readpartial)
             @contents << chunk.force_encoding(@encoding)
           end
         rescue

diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb
@@ -327,7 +327,7 @@ def simple_response(body, status = 200)
 
         allow(socket_spy).to receive(:close) { nil }
         allow(socket_spy).to receive(:closed?) { true }
-        allow(socket_spy).to receive(:readpartial) { chunks[0] }
+        allow(socket_spy).to receive(:readpartial) { chunks.shift || :eof }
         allow(socket_spy).to receive(:write) { chunks[0].length }
 
         allow(TCPSocket).to receive(:open) { socket_spy }
@@ -338,5 +338,58 @@ def simple_response(body, status = 200)
         expect(body).to eq "<!doctype html>"
       end
     end
+
+    context "when uses chunked transfer encoding" do
+      let(:chunks) do
+        [
+          <<-RESPONSE.gsub(/^\s*\| */, "").gsub(/\n/, "\r\n") << body
+          | HTTP/1.1 200 OK
+          | Content-Type: application/json
+          | Transfer-Encoding: chunked
+          | Connection: close
+          |
+          RESPONSE
+        ]
+      end
+      let(:body) do
+        <<-BODY.gsub(/^\s*\| */, "").gsub(/\n/, "\r\n")
+        | 9
+        | {"state":
+        | 5
+        | "ok"}
+        | 0
+        |
+        BODY
+      end
+
+      before do
+        socket_spy = double
+
+        allow(socket_spy).to receive(:close) { nil }
+        allow(socket_spy).to receive(:closed?) { true }
+        allow(socket_spy).to receive(:readpartial) { chunks.shift || :eof }
+        allow(socket_spy).to receive(:write) { chunks[0].length }
+
+        allow(TCPSocket).to receive(:open) { socket_spy }
+      end
+
+      it "properly reads body" do
+        body = client.get(dummy.endpoint).to_s
+        expect(body).to eq '{"state":"ok"}'
+      end
+
+      context "with broken body (too early closed connection)" do
+        let(:body) do
+          <<-BODY.gsub(/^\s*\| */, "").gsub(/\n/, "\r\n")
+          | 9
+          | {"state":
+          BODY
+        end
+
+        it "raises HTTP::ConnectionError" do
+          expect { client.get(dummy.endpoint).to_s }.to raise_error(HTTP::ConnectionError)
+        end
+      end
+    end
   end
 end
diff --git a/spec/lib/http/response/body_spec.rb b/spec/lib/http/response/body_spec.rb
@@ -3,9 +3,12 @@
   let(:connection) { double(:sequence_id => 0) }
   let(:chunks)     { [String.new("Hello, "), String.new("World!")] }
 
-  before           { allow(connection).to receive(:readpartial) { chunks.shift } }
+  before do
+    allow(connection).to receive(:readpartial) { chunks.shift }
+    allow(connection).to receive(:body_completed?) { chunks.empty? }
+  end
 
-  subject(:body)   { described_class.new(connection, :encoding => Encoding::UTF_8) }
+  subject(:body) { described_class.new(connection, :encoding => Encoding::UTF_8) }
 
   it "streams bodies from responses" do
     expect(subject.to_s).to eq("Hello, World!")