Skip to content

Commit

Permalink
back to io/wait
Browse files Browse the repository at this point in the history
This is a squashed commit of PR: #368
  • Loading branch information
Tiago Cardoso authored and ixti committed Feb 6, 2017
1 parent 51fa147 commit cb44c5d
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 24 deletions.
4 changes: 2 additions & 2 deletions lib/http/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions lib/http/headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
18 changes: 16 additions & 2 deletions lib/http/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions lib/http/response/body.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
24 changes: 18 additions & 6 deletions lib/http/timeout/per_operation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions spec/lib/http/headers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions spec/lib/http/response/body_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!")
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions spec/lib/http/response_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit cb44c5d

Please sign in to comment.