From 078bf579df5a2dba51484d2ba69eb8c9f532d47c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 23 May 2018 03:10:01 +0200 Subject: [PATCH 1/3] Handle early responses HTTP servers typically wait until the whole request body has been received before sending the response. However, some servers might send out a response early, before they've read the whole request body. For example, they might detect the request headers were invalid, and they want to tell that to the client immediately. For example, sending a large request to http://google.com will return an early `400 Bad Request` response. In this case HTTP.rb will raise an Errno::EPIPE exception, because it tried to write to the socket but it couldn't because the server closed its end. However, the server still returned a valid response, so it's not really erroneous behaviour (e.g. curl doesn't print any errors in the same situations). # Before HTTP.get("http://google.com", body: "a" * 1*1024*1024) #~> Errno::EPIPE # After HTTP.get("http://google.com", body: "a" * 1*1024*1024) #=> # This commit ignores Errno::EPIPE exceptions when writing request data to the socket. --- lib/http/request/writer.rb | 2 ++ spec/lib/http/request/writer_spec.rb | 12 +++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/http/request/writer.rb b/lib/http/request/writer.rb index b2f383fe..ca770b4b 100644 --- a/lib/http/request/writer.rb +++ b/lib/http/request/writer.rb @@ -100,6 +100,8 @@ def write(data) break unless data.bytesize > length data = data.byteslice(length..-1) end + rescue Errno::EPIPE + # server doesn't need any more data rescue IOError, SocketError, SystemCallError => ex raise ConnectionError, "error writing to socket: #{ex}", ex.backtrace end diff --git a/spec/lib/http/request/writer_spec.rb b/spec/lib/http/request/writer_spec.rb index 62780a6f..80a67809 100644 --- a/spec/lib/http/request/writer_spec.rb +++ b/spec/lib/http/request/writer_spec.rb @@ -75,11 +75,21 @@ end end - context "when writing to socket raises an exception" do + context "when server won't accept any more data" do before do expect(io).to receive(:write).and_raise(Errno::EPIPE) end + it "aborts silently" do + writer.stream + end + end + + context "when writing to socket raises an exception" do + before do + expect(io).to receive(:write).and_raise(Errno::ECONNRESET) + end + it "raises a ConnectionError" do expect { writer.stream }.to raise_error(HTTP::ConnectionError) end From 2a058d281f0a311aa3e4dc5b26ac0bf0623d90b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 23 May 2018 03:20:11 +0200 Subject: [PATCH 2/3] Stop writing after receiving Errno::EPIPE We want to stop writing after server has closed its end of the pipe (as no more data will be accepted) and proceed with reading the response. --- .rubocop.yml | 5 +++++ lib/http/request/writer.rb | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index c0f965fe..06c7710f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,6 +13,11 @@ Layout/SpaceAroundOperators: Layout/SpaceInsideHashLiteralBraces: EnforcedStyle: no_space +## Lint ######################################################################## + +Lint/HandleExceptions: + Enabled: false + ## Metrics ##################################################################### Metrics/AbcSize: diff --git a/lib/http/request/writer.rb b/lib/http/request/writer.rb index ca770b4b..67a5a1d6 100644 --- a/lib/http/request/writer.rb +++ b/lib/http/request/writer.rb @@ -76,6 +76,8 @@ def send_request write(data) unless data.empty? write(CHUNKED_END) if chunked? + rescue Errno::EPIPE + # server doesn't need any more data end # Returns the chunk encoded for to the specified "Transfer-Encoding" header. @@ -101,7 +103,7 @@ def write(data) data = data.byteslice(length..-1) end rescue Errno::EPIPE - # server doesn't need any more data + raise # re-raise Errno::EPIPE rescue IOError, SocketError, SystemCallError => ex raise ConnectionError, "error writing to socket: #{ex}", ex.backtrace end From 1c3d6a0dbbdfca71dfa49451f89e816530bb73b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 23 May 2018 13:17:00 +0200 Subject: [PATCH 3/3] Refactor Request::Writer#send_request --- .rubocop.yml | 5 ----- lib/http/request/writer.rb | 28 ++++++++++++++++++---------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 06c7710f..c0f965fe 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,11 +13,6 @@ Layout/SpaceAroundOperators: Layout/SpaceInsideHashLiteralBraces: EnforcedStyle: no_space -## Lint ######################################################################## - -Lint/HandleExceptions: - Enabled: false - ## Metrics ##################################################################### Metrics/AbcSize: diff --git a/lib/http/request/writer.rb b/lib/http/request/writer.rb index 67a5a1d6..06823654 100644 --- a/lib/http/request/writer.rb +++ b/lib/http/request/writer.rb @@ -60,24 +60,32 @@ def join_headers @request_header.join(CRLF) + CRLF * 2 end + # Writes HTTP request data into the socket. def send_request - # It's important to send the request in a single write call when - # possible in order to play nicely with Nagle's algorithm. Making - # two writes in a row triggers a pathological case where Nagle is - # expecting a third write that never happens. + each_chunk { |chunk| write chunk } + rescue Errno::EPIPE + # server doesn't need any more data + nil + end + + # Yields chunks of request data that should be sent to the socket. + # + # It's important to send the request in a single write call when possible + # in order to play nicely with Nagle's algorithm. Making two writes in a + # row triggers a pathological case where Nagle is expecting a third write + # that never happens. + def each_chunk data = join_headers @body.each do |chunk| data << encode_chunk(chunk) - write(data) + yield data data.clear end - write(data) unless data.empty? + yield data unless data.empty? - write(CHUNKED_END) if chunked? - rescue Errno::EPIPE - # server doesn't need any more data + yield CHUNKED_END if chunked? end # Returns the chunk encoded for to the specified "Transfer-Encoding" header. @@ -103,7 +111,7 @@ def write(data) data = data.byteslice(length..-1) end rescue Errno::EPIPE - raise # re-raise Errno::EPIPE + raise rescue IOError, SocketError, SystemCallError => ex raise ConnectionError, "error writing to socket: #{ex}", ex.backtrace end