From 2590ac217e7910a2c6aab43850a7452c94cf3a9a Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Mon, 8 Aug 2016 12:53:06 +0200 Subject: [PATCH 01/27] reverting the previous io/wait commits as a first measure, and reproducing the previous errors --- lib/http/timeout/per_operation.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/http/timeout/per_operation.rb b/lib/http/timeout/per_operation.rb index 1239f265..2d6ea2c5 100644 --- a/lib/http/timeout/per_operation.rb +++ b/lib/http/timeout/per_operation.rb @@ -65,7 +65,7 @@ def readpartial(size) return :eof if result.nil? return result if result != :wait_readable - unless IO.select([@socket], nil, nil, read_timeout) + unless @socket.to_io.wait_readable(read_timeout) raise TimeoutError, "Read timed out after #{read_timeout} seconds" end end @@ -77,7 +77,7 @@ def write(data) result = @socket.write_nonblock(data, :exception => false) return result unless result == :wait_writable - unless IO.select(nil, [@socket], nil, write_timeout) + unless @socket.to_io.wait_writable(write_timeout) raise TimeoutError, "Write timed out after #{write_timeout} seconds" end end From 5a25cb7e43c2fc63c0d550b8068d4566539c4abc Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Sun, 14 Aug 2016 01:08:39 +0200 Subject: [PATCH 02/27] added helper methods to the response, which will help inferring the proper content length to read from socket --- lib/http/errors.rb | 3 +++ lib/http/response.rb | 25 +++++++++++++++++++++++++ spec/lib/http/response_spec.rb | 29 +++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/lib/http/errors.rb b/lib/http/errors.rb index 8a1fc260..5a8364f5 100644 --- a/lib/http/errors.rb +++ b/lib/http/errors.rb @@ -20,4 +20,7 @@ class TimeoutError < Error; end # Header name is invalid class InvalidHeaderNameError < Error; end + + # Header value is of unexpected format (similar to Net::HTTPHeaderSyntaxError) + class HeaderSyntaxError < Error; end end diff --git a/lib/http/response.rb b/lib/http/response.rb index ee936742..a8bbe0fe 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -132,6 +132,31 @@ def cookies end end + def content_length + @content_length ||= begin + # 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[Headers::TRANSFER_ENCODING] + + clen = headers[Headers::CONTENT_LENGTH] + if clen + len = clen.slice(/\d+/) || + raise(HeaderSyntaxError, 'wrong Content-Length format') + len.to_i + end + end + end + + def chunked? + encoding = headers.get(Headers::TRANSFER_ENCODING) + case encoding + when String then encoding == CHUNKED + when Enumerable then encoding.include?("chunked") + else false + end + end + # Parse response body with corresponding MIME type adapter. # # @param [#to_s] as Parse as given MIME type diff --git a/spec/lib/http/response_spec.rb b/spec/lib/http/response_spec.rb index 481a4bb7..40dd13ed 100644 --- a/spec/lib/http/response_spec.rb +++ b/spec/lib/http/response_spec.rb @@ -173,4 +173,33 @@ expect(response.connection).to eq connection end end + + describe "#content_length" do + subject(:length) { response.content_length } + context "when set" do + let(:headers) { { "Content-Length" => "10" } } + it { is_expected.to be(10) } + context "and response is set as chunked" do + before { allow(response).to receive(:chunked?).and_return(true) } + it "ignores the length set" do + end + end + context "but format is invalid" do + let(:headers) { { "Content-Length" => "ten" } } + it { expect { length }.to raise_error(Http::HeaderSyntaxError) } + end + end + context "when not set" do + it { is_expected.to be_nil } + 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 From b04a0db16bf401e7f635efbd903e25a2077e8009 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Sun, 14 Aug 2016 01:13:26 +0200 Subject: [PATCH 03/27] added alternative debugger for ruby > 2.0 --- Gemfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Gemfile b/Gemfile index 55ea2dd2..9a84d3bd 100644 --- a/Gemfile +++ b/Gemfile @@ -12,6 +12,9 @@ group :development do gem "pry-debugger", :require => false gem "pry-stack_explorer", :require => false end + platform :ruby_21, :ruby_22, :ruby_23 do + gem "pry-byebug", :require => false + end end group :test do From f5b22e5996f07f35885b49c1d529df2067d1d34b Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Sun, 14 Aug 2016 01:49:06 +0200 Subject: [PATCH 04/27] enhanced the master branch #content_length implementation, regarding RFC compliance --- lib/http/response.rb | 23 ++++++----------------- spec/lib/http/response_spec.rb | 18 ------------------ 2 files changed, 6 insertions(+), 35 deletions(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index a8bbe0fe..969b7a15 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -99,13 +99,18 @@ 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[Headers::TRANSFER_ENCODING] + value = @headers[Headers::CONTENT_LENGTH] return unless value begin Integer(value) rescue ArgumentError - nil + raise(HeaderSyntaxError, 'wrong Content-Length format') end end @@ -132,22 +137,6 @@ def cookies end end - def content_length - @content_length ||= begin - # 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[Headers::TRANSFER_ENCODING] - - clen = headers[Headers::CONTENT_LENGTH] - if clen - len = clen.slice(/\d+/) || - raise(HeaderSyntaxError, 'wrong Content-Length format') - len.to_i - end - end - end - def chunked? encoding = headers.get(Headers::TRANSFER_ENCODING) case encoding diff --git a/spec/lib/http/response_spec.rb b/spec/lib/http/response_spec.rb index 40dd13ed..a70a8373 100644 --- a/spec/lib/http/response_spec.rb +++ b/spec/lib/http/response_spec.rb @@ -28,24 +28,6 @@ end end - describe "#content_length" do - subject { response.content_length } - - context "without Content-Length header" do - it { is_expected.to be_nil } - end - - context "with Content-Length: 5" do - let(:headers) { {"Content-Length" => "5"} } - it { is_expected.to eq 5 } - end - - context "with invalid Content-Length" do - let(:headers) { {"Content-Length" => "foo"} } - it { is_expected.to be_nil } - end - end - describe "mime_type" do subject { response.mime_type } From ca55acf6ae89906246163e41893b65043a3f62fc Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Sun, 14 Aug 2016 01:50:33 +0200 Subject: [PATCH 05/27] passing content length to response body; when set, read until length is exhausted --- lib/http/response/body.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/http/response/body.rb b/lib/http/response/body.rb index 0bc1643f..840b2983 100644 --- a/lib/http/response/body.rb +++ b/lib/http/response/body.rb @@ -54,6 +54,10 @@ def to_s @contents = String.new("").force_encoding(encoding) while (chunk = @stream.readpartial) + length = @length || Float::INFINITY + + while (length > 0 && chunk = @client.readpartial) + length -= chunk.bytesize @contents << chunk.force_encoding(encoding) end rescue From 21f9d34d7ac8de03510581ad20cfb9909ef716eb Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Sun, 14 Aug 2016 01:57:38 +0200 Subject: [PATCH 06/27] CI has old version of bundler, and I can't set the newest platforms... shame --- Gemfile | 3 --- 1 file changed, 3 deletions(-) diff --git a/Gemfile b/Gemfile index 9a84d3bd..55ea2dd2 100644 --- a/Gemfile +++ b/Gemfile @@ -12,9 +12,6 @@ group :development do gem "pry-debugger", :require => false gem "pry-stack_explorer", :require => false end - platform :ruby_21, :ruby_22, :ruby_23 do - gem "pry-byebug", :require => false - end end group :test do From 4a39a8b1ecc160bd84d5c3a1f0b558f703b6b417 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Sun, 14 Aug 2016 15:56:53 +0200 Subject: [PATCH 07/27] tolerating the first timeout on read/write wait; like in the failing case, the socket is anyway going to be closed by the server, so allow to try a read one more time; if it's eof, all good, otherwise it's a timeout; did something analogous for writes --- lib/http/timeout/per_operation.rb | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/http/timeout/per_operation.rb b/lib/http/timeout/per_operation.rb index 2d6ea2c5..fc0805b3 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 @socket.to_io.wait_readable(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 v = @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 @socket.to_io.wait_writable(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 From f79bf106d6516203f51a24b59a4d1da3b9816eaa Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Fri, 19 Aug 2016 13:56:00 +0200 Subject: [PATCH 08/27] added general header error, left the existing custom one for back-comp --- lib/http/errors.rb | 8 +++---- lib/http/headers.rb | 4 ++-- lib/http/response.rb | 2 +- spec/lib/http/response_spec.rb | 38 ++++++++++++++++------------------ 4 files changed, 25 insertions(+), 27 deletions(-) diff --git a/lib/http/errors.rb b/lib/http/errors.rb index 5a8364f5..eb17c5b0 100644 --- a/lib/http/errors.rb +++ b/lib/http/errors.rb @@ -18,9 +18,9 @@ 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 HeaderSyntaxError < Error; end + class HeaderError < Error; end + + # @deprecated will be removed in v3.0.0 + InvalidHeaderNameError = HeaderError 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 969b7a15..ce685a8f 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -110,7 +110,7 @@ def content_length begin Integer(value) rescue ArgumentError - raise(HeaderSyntaxError, 'wrong Content-Length format') + raise(HeaderError, 'wrong Content-Length format') end end diff --git a/spec/lib/http/response_spec.rb b/spec/lib/http/response_spec.rb index a70a8373..cdd9cfa0 100644 --- a/spec/lib/http/response_spec.rb +++ b/spec/lib/http/response_spec.rb @@ -28,6 +28,24 @@ end end + describe "#content_length" do + subject { response.content_length } + + context "without Content-Length header" do + it { is_expected.to be_nil } + end + + context "with Content-Length: 5" do + let(:headers) { {"Content-Length" => "5"} } + it { is_expected.to eq 5 } + end + + context "with invalid Content-Length" do + let(:headers) { {"Content-Length" => "foo"} } + it { expect { subject }.to raise_error(Http::HeaderError) } + end + end + describe "mime_type" do subject { response.mime_type } @@ -156,26 +174,6 @@ end end - describe "#content_length" do - subject(:length) { response.content_length } - context "when set" do - let(:headers) { { "Content-Length" => "10" } } - it { is_expected.to be(10) } - context "and response is set as chunked" do - before { allow(response).to receive(:chunked?).and_return(true) } - it "ignores the length set" do - end - end - context "but format is invalid" do - let(:headers) { { "Content-Length" => "ten" } } - it { expect { length }.to raise_error(Http::HeaderSyntaxError) } - end - end - context "when not set" do - it { is_expected.to be_nil } - end - end - describe "#chunked?" do subject { response } context "when encoding is set to chunked" do From f0f023140270a2fa279d2e30000480271ad549c2 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Tue, 23 Aug 2016 14:39:05 +0300 Subject: [PATCH 09/27] wrong format for content length doesn't raise error, which doesn't please me, but oh well --- lib/http/response.rb | 1 - spec/lib/http/response_spec.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index ce685a8f..e978982b 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -110,7 +110,6 @@ def content_length begin Integer(value) rescue ArgumentError - raise(HeaderError, 'wrong Content-Length format') end end diff --git a/spec/lib/http/response_spec.rb b/spec/lib/http/response_spec.rb index cdd9cfa0..857622f2 100644 --- a/spec/lib/http/response_spec.rb +++ b/spec/lib/http/response_spec.rb @@ -42,7 +42,7 @@ context "with invalid Content-Length" do let(:headers) { {"Content-Length" => "foo"} } - it { expect { subject }.to raise_error(Http::HeaderError) } + it { is_expected.to be_nil } end end From 48c3e85237ffee7ebf5406bd0af8bf82847b6af9 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Wed, 24 Aug 2016 10:02:26 +0300 Subject: [PATCH 10/27] corrected some faults coming from latest rebase --- lib/http/response/body.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/http/response/body.rb b/lib/http/response/body.rb index 840b2983..6050e923 100644 --- a/lib/http/response/body.rb +++ b/lib/http/response/body.rb @@ -53,10 +53,9 @@ def to_s @streaming = false @contents = String.new("").force_encoding(encoding) - while (chunk = @stream.readpartial) length = @length || Float::INFINITY - while (length > 0 && chunk = @client.readpartial) + while (length > 0 && chunk = @stream.readpartial) length -= chunk.bytesize @contents << chunk.force_encoding(encoding) end From 93c9ce9ea356eaf7041172686ddfa04f4c0c736a Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Wed, 24 Aug 2016 10:03:12 +0300 Subject: [PATCH 11/27] explicit nil --- lib/http/response.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/http/response.rb b/lib/http/response.rb index e978982b..afdf51c7 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -110,6 +110,7 @@ def content_length begin Integer(value) rescue ArgumentError + nil end end From 73f08164d37ed3db8d08f191f58129380b669c2f Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Wed, 24 Aug 2016 10:04:12 +0300 Subject: [PATCH 12/27] do not needlessly call the method, use the ivar --- lib/http/response.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index afdf51c7..f7a2ae71 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -102,7 +102,7 @@ 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[Headers::TRANSFER_ENCODING] + return nil if @headers[Headers::TRANSFER_ENCODING] value = @headers[Headers::CONTENT_LENGTH] return unless value From 00279ae1827d55c0678e722a4af6f31788a63ace Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Wed, 24 Aug 2016 10:12:31 +0300 Subject: [PATCH 13/27] Headers#get always returns an array, so work only with that; also, use Headers#include? to reduce allocations --- lib/http/response.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index f7a2ae71..f8b37a71 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -138,12 +138,11 @@ def cookies end def chunked? - encoding = headers.get(Headers::TRANSFER_ENCODING) - case encoding - when String then encoding == CHUNKED - when Enumerable then encoding.include?("chunked") - else false - end + return false unless @headers.include?(Headers::TRANSFER_ENCODING) + + encoding = @headers.get(Headers::TRANSFER_ENCODING) + + encoding.include?("chunked") end # Parse response body with corresponding MIME type adapter. From 9a2dea17fe7e160d4284d08d238de6a44f50704e Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Wed, 24 Aug 2016 10:13:09 +0300 Subject: [PATCH 14/27] use Headers#include? to reduce allocations, add header conditions together --- lib/http/response.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index f8b37a71..1b01f73d 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -102,10 +102,10 @@ 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[Headers::TRANSFER_ENCODING] + return nil if @headers.include?(Headers::TRANSFER_ENCODING) || + !@headers.include?(Headers::CONTENT_LENGTH) value = @headers[Headers::CONTENT_LENGTH] - return unless value begin Integer(value) From c36d357ab56c28bbd165bdb36eccae11fa10620d Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Wed, 24 Aug 2016 10:21:09 +0300 Subject: [PATCH 15/27] according to RFC, response is only chunked only if the transfer encoding ends with chunked --- lib/http/response.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index 1b01f73d..bbced923 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -142,7 +142,7 @@ def chunked? encoding = @headers.get(Headers::TRANSFER_ENCODING) - encoding.include?("chunked") + encoding.last == "chunked" end # Parse response body with corresponding MIME type adapter. From 83f3b3222a3c53d69bea6f21b4443033f955a616 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Wed, 24 Aug 2016 10:25:50 +0300 Subject: [PATCH 16/27] added comment for future refactoring --- lib/http/response.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/http/response.rb b/lib/http/response.rb index bbced923..9e524cf3 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -142,6 +142,7 @@ def chunked? encoding = @headers.get(Headers::TRANSFER_ENCODING) + # TODO: "chunked" is frozen in the request writer. How about making it accessible? encoding.last == "chunked" end From 08ec27d139c947a41b11f5c27d01de541ebe1d48 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Tue, 30 Aug 2016 17:54:01 +0200 Subject: [PATCH 17/27] removed the extra include? call over there --- lib/http/response.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index 9e524cf3..6088fae9 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -102,10 +102,10 @@ 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) || - !@headers.include?(Headers::CONTENT_LENGTH) + return nil if @headers.include?(Headers::TRANSFER_ENCODING) value = @headers[Headers::CONTENT_LENGTH] + return nil unless value begin Integer(value) From 9c18fe39111162e5675cec2fbed18ce6e5ea50e8 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Tue, 30 Aug 2016 22:00:25 +0200 Subject: [PATCH 18/27] rubocoping --- lib/http/response.rb | 2 +- lib/http/timeout/per_operation.rb | 8 ++++---- spec/lib/http/response_spec.rb | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index 6088fae9..af160455 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -101,7 +101,7 @@ def flush 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. + # 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] diff --git a/lib/http/timeout/per_operation.rb b/lib/http/timeout/per_operation.rb index fc0805b3..daa69a16 100644 --- a/lib/http/timeout/per_operation.rb +++ b/lib/http/timeout/per_operation.rb @@ -72,12 +72,12 @@ def readpartial(size) # #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 + # 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 + # 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 v = @socket.to_io.wait_readable(read_timeout) + timeout = true unless @socket.to_io.wait_readable(read_timeout) end end diff --git a/spec/lib/http/response_spec.rb b/spec/lib/http/response_spec.rb index 857622f2..10a07a24 100644 --- a/spec/lib/http/response_spec.rb +++ b/spec/lib/http/response_spec.rb @@ -173,11 +173,11 @@ 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" } } + let(:headers) { {"Transfer-Encoding" => "chunked"} } it { is_expected.to be_chunked } end it { is_expected.not_to be_chunked } From 6a13e2afbd281e5ba781a592ee780c472c14f068 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Mon, 30 Jan 2017 12:40:18 +0100 Subject: [PATCH 19/27] rubocop <3 ... --- lib/http/response/body.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/response/body.rb b/lib/http/response/body.rb index 6050e923..bc45e2bd 100644 --- a/lib/http/response/body.rb +++ b/lib/http/response/body.rb @@ -55,7 +55,7 @@ def to_s length = @length || Float::INFINITY - while (length > 0 && chunk = @stream.readpartial) + while length > 0 && (chunk = @stream.readpartial) length -= chunk.bytesize @contents << chunk.force_encoding(encoding) end From 108e127de20d334a4ede923c066eae281139de14 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Fri, 3 Feb 2017 20:12:42 +0100 Subject: [PATCH 20/27] reintegrated kwargs in the body API, and passing the content length down --- lib/http/response.rb | 2 +- lib/http/response/body.rb | 5 +++-- spec/lib/http/response/body_spec.rb | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index af160455..b811e6ca 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -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 diff --git a/lib/http/response/body.rb b/lib/http/response/body.rb index bc45e2bd..33f02d13 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(connection, stream, encoding = Encoding::BINARY) + def initialize(connection, stream, encoding: Encoding::BINARY, length: nil) @connection = connection @streaming = nil @contents = nil @stream = stream @encoding = encoding + @length = length || Float::INFINITY end # (see HTTP::Client#readpartial) @@ -53,7 +54,7 @@ def to_s @streaming = false @contents = String.new("").force_encoding(encoding) - length = @length || Float::INFINITY + length = @length while length > 0 && (chunk = @stream.readpartial) length -= chunk.bytesize diff --git a/spec/lib/http/response/body_spec.rb b/spec/lib/http/response/body_spec.rb index 5bc4a1ca..3dea2ffa 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, 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!") @@ -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 From 607e42b1ce796a06bd48f5f29f46576706029eb8 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Fri, 3 Feb 2017 20:23:18 +0100 Subject: [PATCH 21/27] rubo rubo rubocop --- lib/http/response/body.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/response/body.rb b/lib/http/response/body.rb index 33f02d13..846f72b2 100644 --- a/lib/http/response/body.rb +++ b/lib/http/response/body.rb @@ -15,7 +15,7 @@ class Body # @return [HTTP::Connection] attr_reader :connection - def initialize(connection, stream, encoding: Encoding::BINARY, length: nil) + def initialize(connection, stream, :length => nil, :encoding => Encoding::BINARY) @connection = connection @streaming = nil @contents = nil From 4a90c6c1e0e31d15d1d2e0e1dcb2d3bbf88efb00 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Fri, 3 Feb 2017 20:36:48 +0100 Subject: [PATCH 22/27] rubocop, make up your mind... --- lib/http/response.rb | 2 +- lib/http/response/body.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index b811e6ca..f1fd3c15 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -51,7 +51,7 @@ def initialize(opts) stream = body_stream_for(connection, opts) - @body = Response::Body.new(connection, stream, encoding: encoding, length: content_length) + @body = Response::Body.new(connection, stream, :encoding => encoding, :length => content_length) else @body = opts.fetch(:body) end diff --git a/lib/http/response/body.rb b/lib/http/response/body.rb index 846f72b2..3c747a95 100644 --- a/lib/http/response/body.rb +++ b/lib/http/response/body.rb @@ -15,7 +15,7 @@ class Body # @return [HTTP::Connection] attr_reader :connection - def initialize(connection, stream, :length => nil, :encoding => Encoding::BINARY) + def initialize(connection, stream, length: nil, encoding: Encoding::BINARY) @connection = connection @streaming = nil @contents = nil From 21c0f60eef057e72cd4e1330ec826aae9e03da53 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Fri, 3 Feb 2017 20:37:33 +0100 Subject: [PATCH 23/27] removing deprecation --- lib/http/errors.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/http/errors.rb b/lib/http/errors.rb index eb17c5b0..d773f863 100644 --- a/lib/http/errors.rb +++ b/lib/http/errors.rb @@ -21,6 +21,4 @@ class TimeoutError < Error; end # Header value is of unexpected format (similar to Net::HTTPHeaderSyntaxError) class HeaderError < Error; end - # @deprecated will be removed in v3.0.0 - InvalidHeaderNameError = HeaderError end From 7fc577bbc4838a2a36ee8512c1cc7ecce525ec13 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Fri, 3 Feb 2017 20:42:39 +0100 Subject: [PATCH 24/27] removed tests with deprecated error.. --- spec/lib/http/headers_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 From 5b192150af8d0625fefe027113403e744fed843f Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Fri, 3 Feb 2017 20:45:55 +0100 Subject: [PATCH 25/27] rubocop again... --- spec/lib/http/response/body_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/http/response/body_spec.rb b/spec/lib/http/response/body_spec.rb index 3dea2ffa..6f7ab850 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, connection, encoding: 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!") @@ -47,7 +47,7 @@ end subject(:body) do inflater = HTTP::Response::Inflater.new(connection) - described_class.new(connection, inflater, encoding: Encoding::UTF_8) + described_class.new(connection, inflater, :encoding => Encoding::UTF_8) end it "decodes body" do From 8d84978d7a50da4c3632cdf18ff975ff4fd870be Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Fri, 3 Feb 2017 20:59:03 +0100 Subject: [PATCH 26/27] more ruboscosps... --- lib/http/errors.rb | 1 - spec/lib/http/response/body_spec.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/http/errors.rb b/lib/http/errors.rb index d773f863..685db6a2 100644 --- a/lib/http/errors.rb +++ b/lib/http/errors.rb @@ -20,5 +20,4 @@ class TimeoutError < Error; end # Header value is of unexpected format (similar to Net::HTTPHeaderSyntaxError) class HeaderError < Error; end - end diff --git a/spec/lib/http/response/body_spec.rb b/spec/lib/http/response/body_spec.rb index 6f7ab850..d7ac1b1e 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, connection,:encoding => 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!") From a6afeebe5f39817c1b335d2ed294122399e81162 Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Mon, 6 Feb 2017 22:19:47 +0100 Subject: [PATCH 27/27] rbc --- lib/http/response.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index 8205b637..d6b39b86 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -50,7 +50,6 @@ 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) else @body = opts.fetch(:body)