diff --git a/lib/http/errors.rb b/lib/http/errors.rb index 8a1fc260..685db6a2 100644 --- a/lib/http/errors.rb +++ b/lib/http/errors.rb @@ -18,6 +18,6 @@ class StateError < ResponseError; end # Generic Timeout error class TimeoutError < Error; end - # Header name is invalid - class InvalidHeaderNameError < Error; end + # Header value is of unexpected format (similar to Net::HTTPHeaderSyntaxError) + class HeaderError < Error; end end diff --git a/lib/http/headers.rb b/lib/http/headers.rb index 0d6291dd..d35e8808 100644 --- a/lib/http/headers.rb +++ b/lib/http/headers.rb @@ -196,7 +196,7 @@ def coerce(object) # Transforms `name` to canonical HTTP header capitalization # # @param [String] name - # @raise [InvalidHeaderNameError] if normalized name does not + # @raise [HeaderError] if normalized name does not # match {HEADER_NAME_RE} # @return [String] canonical HTTP header name def normalize_header(name) @@ -206,7 +206,7 @@ def normalize_header(name) return normalized if normalized =~ COMPLIANT_NAME_RE - raise InvalidHeaderNameError, "Invalid HTTP header field name: #{name.inspect}" + raise HeaderError, "Invalid HTTP header field name: #{name.inspect}" end end end diff --git a/lib/http/response.rb b/lib/http/response.rb index 58097e4e..d6b39b86 100644 --- 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) + @body = Response::Body.new(stream, :encoding => encoding, :length => content_length) else @body = opts.fetch(:body) end @@ -98,8 +98,13 @@ def flush # (not an integer, e.g. empty string or string with non-digits). # @return [Integer] otherwise def content_length + # http://greenbytes.de/tech/webdav/rfc7230.html#rfc.section.3.3.3 + # Clause 3: "If a message is received with both a Transfer-Encoding + # and a Content-Length header field, the Transfer-Encoding overrides the Content-Length. + return nil if @headers.include?(Headers::TRANSFER_ENCODING) + value = @headers[Headers::CONTENT_LENGTH] - return unless value + return nil unless value begin Integer(value) @@ -131,6 +136,15 @@ def cookies end end + def chunked? + return false unless @headers.include?(Headers::TRANSFER_ENCODING) + + encoding = @headers.get(Headers::TRANSFER_ENCODING) + + # TODO: "chunked" is frozen in the request writer. How about making it accessible? + encoding.last == "chunked" + end + # Parse response body with corresponding MIME type adapter. # # @param [#to_s] as Parse as given MIME type diff --git a/lib/http/response/body.rb b/lib/http/response/body.rb index a4864a9f..557db352 100644 --- a/lib/http/response/body.rb +++ b/lib/http/response/body.rb @@ -15,12 +15,13 @@ class Body # @return [HTTP::Connection] attr_reader :connection - def initialize(stream, encoding = Encoding::BINARY) + def initialize(stream, length: nil, encoding: Encoding::BINARY) @stream = stream @connection = stream.is_a?(Inflater) ? stream.connection : stream @streaming = nil @contents = nil @encoding = encoding + @length = length || Float::INFINITY end # (see HTTP::Client#readpartial) @@ -53,7 +54,10 @@ def to_s @streaming = false @contents = String.new("").force_encoding(encoding) - while (chunk = @stream.readpartial) + length = @length + + while length > 0 && (chunk = @stream.readpartial) + length -= chunk.bytesize @contents << chunk.force_encoding(encoding) end rescue diff --git a/lib/http/timeout/per_operation.rb b/lib/http/timeout/per_operation.rb index 1239f265..daa69a16 100644 --- a/lib/http/timeout/per_operation.rb +++ b/lib/http/timeout/per_operation.rb @@ -59,29 +59,41 @@ def write(data) else # Read data from the socket def readpartial(size) + timeout = false loop do result = @socket.read_nonblock(size, :exception => false) return :eof if result.nil? return result if result != :wait_readable - unless IO.select([@socket], nil, nil, read_timeout) - raise TimeoutError, "Read timed out after #{read_timeout} seconds" - end + raise TimeoutError, "Read timed out after #{read_timeout} seconds" if timeout + # marking the socket for timeout. Why is this not being raised immediately? + # it seems there is some race-condition on the network level between calling + # #read_nonblock and #wait_readable, in which #read_nonblock signalizes waiting + # for reads, and when waiting for x seconds, it returns nil suddenly without completing + # the x seconds. In a normal case this would be a timeout on wait/read, but it can + # also mean that the socket has been closed by the server. Therefore we "mark" the + # socket for timeout and try to read more bytes. If it returns :eof, it's all good, no + # timeout. Else, the first timeout was a proper timeout. + # This hack has to be done because io/wait#wait_readable doesn't provide a value for when + # the socket is closed by the server, and HTTP::Parser doesn't provide the limit for the chunks. + timeout = true unless @socket.to_io.wait_readable(read_timeout) end end # Write data to the socket def write(data) + timeout = false loop do result = @socket.write_nonblock(data, :exception => false) return result unless result == :wait_writable - unless IO.select(nil, [@socket], nil, write_timeout) - raise TimeoutError, "Write timed out after #{write_timeout} seconds" - end + raise TimeoutError, "Write timed out after #{write_timeout} seconds" if timeout + + timeout = true unless @socket.to_io.wait_writable(write_timeout) end end + end end end diff --git a/spec/lib/http/headers_spec.rb b/spec/lib/http/headers_spec.rb index 863e0316..745e745d 100644 --- a/spec/lib/http/headers_spec.rb +++ b/spec/lib/http/headers_spec.rb @@ -31,12 +31,12 @@ it "fails with empty header name" do expect { headers.set "", "foo bar" }. - to raise_error HTTP::InvalidHeaderNameError + to raise_error HTTP::HeaderError end it "fails with invalid header name" do expect { headers.set "foo bar", "baz" }. - to raise_error HTTP::InvalidHeaderNameError + to raise_error HTTP::HeaderError end end @@ -79,12 +79,12 @@ it "fails with empty header name" do expect { headers.delete "" }. - to raise_error HTTP::InvalidHeaderNameError + to raise_error HTTP::HeaderError end it "fails with invalid header name" do expect { headers.delete "foo bar" }. - to raise_error HTTP::InvalidHeaderNameError + to raise_error HTTP::HeaderError end end @@ -113,12 +113,12 @@ it "fails with empty header name" do expect { headers.add("", "foobar") }. - to raise_error HTTP::InvalidHeaderNameError + to raise_error HTTP::HeaderError end it "fails with invalid header name" do expect { headers.add "foo bar", "baz" }. - to raise_error HTTP::InvalidHeaderNameError + to raise_error HTTP::HeaderError end end @@ -141,12 +141,12 @@ it "fails with empty header name" do expect { headers.get("") }. - to raise_error HTTP::InvalidHeaderNameError + to raise_error HTTP::HeaderError end it "fails with invalid header name" do expect { headers.get("foo bar") }. - to raise_error HTTP::InvalidHeaderNameError + to raise_error HTTP::HeaderError end end diff --git a/spec/lib/http/response/body_spec.rb b/spec/lib/http/response/body_spec.rb index e5776aaa..fec54231 100644 --- a/spec/lib/http/response/body_spec.rb +++ b/spec/lib/http/response/body_spec.rb @@ -5,7 +5,7 @@ before { allow(connection).to receive(:readpartial) { chunks.shift } } - subject(:body) { described_class.new(connection, 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!") @@ -47,7 +47,7 @@ end subject(:body) do inflater = HTTP::Response::Inflater.new(connection) - described_class.new(inflater, Encoding::UTF_8) + described_class.new(inflater, :encoding => Encoding::UTF_8) end it "decodes body" do diff --git a/spec/lib/http/response_spec.rb b/spec/lib/http/response_spec.rb index 481a4bb7..10a07a24 100644 --- a/spec/lib/http/response_spec.rb +++ b/spec/lib/http/response_spec.rb @@ -173,4 +173,13 @@ expect(response.connection).to eq connection end end + + describe "#chunked?" do + subject { response } + context "when encoding is set to chunked" do + let(:headers) { {"Transfer-Encoding" => "chunked"} } + it { is_expected.to be_chunked } + end + it { is_expected.not_to be_chunked } + end end