Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

back to io/wait #368

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
2590ac2
reverting the previous io/wait commits as a first measure, and reprod…
Aug 8, 2016
5a25cb7
added helper methods to the response, which will help inferring the p…
Aug 13, 2016
b04a0db
added alternative debugger for ruby > 2.0
Aug 13, 2016
f5b22e5
enhanced the master branch #content_length implementation, regarding …
Aug 13, 2016
ca55acf
passing content length to response body; when set, read until length …
Aug 13, 2016
21f9d34
CI has old version of bundler, and I can't set the newest platforms..…
Aug 13, 2016
4a39a8b
tolerating the first timeout on read/write wait; like in the failing …
Aug 14, 2016
f79bf10
added general header error, left the existing custom one for back-comp
Aug 19, 2016
f0f0231
wrong format for content length doesn't raise error, which doesn't pl…
Aug 23, 2016
48c3e85
corrected some faults coming from latest rebase
Aug 24, 2016
93c9ce9
explicit nil
Aug 24, 2016
73f0816
do not needlessly call the method, use the ivar
Aug 24, 2016
00279ae
Headers#get always returns an array, so work only with that; also, us…
Aug 24, 2016
9a2dea1
use Headers#include? to reduce allocations, add header conditions tog…
Aug 24, 2016
c36d357
according to RFC, response is only chunked only if the transfer encod…
Aug 24, 2016
83f3b32
added comment for future refactoring
Aug 24, 2016
08ec27d
removed the extra include? call over there
Aug 30, 2016
9c18fe3
rubocoping
Aug 30, 2016
6a13e2a
rubocop <3 ...
Jan 30, 2017
108e127
reintegrated kwargs in the body API, and passing the content length down
Feb 3, 2017
607e42b
rubo rubo rubocop
Feb 3, 2017
4a90c6c
rubocop, make up your mind...
Feb 3, 2017
21c0f60
removing deprecation
Feb 3, 2017
7fc577b
removed tests with deprecated error..
Feb 3, 2017
5b19215
rubocop again...
Feb 3, 2017
8d84978
more ruboscosps...
Feb 3, 2017
0346723
Merge branch 'master' into issue_357
HoneyryderChuck Feb 3, 2017
a77e80e
Merge branch 'master' into issue_357
HoneyryderChuck Feb 6, 2017
a6afeeb
rbc
Feb 6, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -51,7 +51,7 @@ def initialize(opts)

stream = body_stream_for(connection, opts)

@body = Response::Body.new(connection, stream, encoding)
@body = Response::Body.new(connection, stream, :encoding => encoding, :length => content_length)
else
@body = opts.fetch(:body)
end
Expand Down Expand Up @@ -99,8 +99,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 @@ -132,6 +137,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(connection, stream, encoding = Encoding::BINARY)
def initialize(connection, stream, length: nil, encoding: Encoding::BINARY)
@connection = connection
@streaming = nil
@contents = nil
@stream = stream
@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, connection, Encoding::UTF_8) }
subject(:body) { described_class.new(connection, 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(connection, inflater, Encoding::UTF_8)
described_class.new(connection, 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