From 208d65f0e9fbc3a3b6b46b8d1b92b0ba6765c56b Mon Sep 17 00:00:00 2001 From: Alexey Zapparov Date: Sun, 1 Oct 2017 19:37:14 +0200 Subject: [PATCH 1/4] Add regression spec that highlights the problem --- spec/regression_specs.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/regression_specs.rb b/spec/regression_specs.rb index eae2b5a6..67fd46a2 100644 --- a/spec/regression_specs.rb +++ b/spec/regression_specs.rb @@ -14,4 +14,11 @@ expect { HTTP.get(google_uri).to_s }.not_to raise_error end end + + describe "#422" do + it "reads body when 200 OK response contains Upgrade header" do + res = HTTP.get("https://httpbin.org/response-headers?Upgrade=h2,h2c") + expect(res.parse(:json)).to include("Upgrade" => "h2,h2c") + end + end end From b532896b0be73e9df31eb0573cabda958d66302e Mon Sep 17 00:00:00 2001 From: Alexey Zapparov Date: Sun, 1 Oct 2017 19:41:09 +0200 Subject: [PATCH 2/4] Use ixti's http_parser.rb repo I have updated nodejs/http-parser dependency there. But the problem is that JRuby most likely still affected :(( --- Gemfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Gemfile b/Gemfile index 4cff89f5..be26c3ae 100644 --- a/Gemfile +++ b/Gemfile @@ -5,6 +5,8 @@ ruby RUBY_VERSION gem "rake" +gem "http_parser.rb", :git => "https://github.com/ixti/http_parser.rb.git" + group :development do gem "guard-rspec", :require => false gem "nokogiri", :require => false From 23197272ca152f464d91f5b9947c05031c04f1cc Mon Sep 17 00:00:00 2001 From: Alexey Zapparov Date: Sun, 1 Oct 2017 20:37:36 +0200 Subject: [PATCH 3/4] Switch to use FFI HTTP Parser --- Gemfile | 2 -- http.gemspec | 2 +- lib/http.rb | 2 -- lib/http/response/parser.rb | 41 ++++++++++++++++++++++++------------- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/Gemfile b/Gemfile index be26c3ae..4cff89f5 100644 --- a/Gemfile +++ b/Gemfile @@ -5,8 +5,6 @@ ruby RUBY_VERSION gem "rake" -gem "http_parser.rb", :git => "https://github.com/ixti/http_parser.rb.git" - group :development do gem "guard-rspec", :require => false gem "nokogiri", :require => false diff --git a/http.gemspec b/http.gemspec index 1933dffe..0abfa7b1 100644 --- a/http.gemspec +++ b/http.gemspec @@ -27,7 +27,7 @@ Gem::Specification.new do |gem| gem.required_ruby_version = ">= 2.2" - gem.add_runtime_dependency "http_parser.rb", "~> 0.6.0" + gem.add_runtime_dependency "http-parser", "~> 1.2.0" gem.add_runtime_dependency "http-form_data", "~> 2.0" gem.add_runtime_dependency "http-cookie", "~> 1.0" gem.add_runtime_dependency "addressable", "~> 2.3" diff --git a/lib/http.rb b/lib/http.rb index a1338598..c430127a 100644 --- a/lib/http.rb +++ b/lib/http.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "http/parser" - require "http/errors" require "http/timeout/null" require "http/timeout/per_operation" diff --git a/lib/http/response/parser.rb b/lib/http/response/parser.rb index 1dbe3afe..a71fb05e 100644 --- a/lib/http/response/parser.rb +++ b/lib/http/response/parser.rb @@ -1,41 +1,53 @@ # frozen_string_literal: true +require "http-parser" + module HTTP class Response class Parser attr_reader :headers def initialize - @parser = HTTP::Parser.new(self) + @state = HttpParser::Parser.new_instance { |i| i.type = :response } + @parser = HttpParser::Parser.new(self) + reset end def add(data) - @parser << data + @parser.parse(@state, data) end alias << add def headers? - !!@headers + @finished[:headers] end def http_version - @parser.http_version.join(".") + @state.http_version end def status_code - @parser.status_code + @state.http_status end # # HTTP::Parser callbacks # - def on_headers_complete(headers) - @headers = headers + def on_header_field(_response, field) + @field = field + end + + def on_header_value(_response, value) + @headers.add(@field, value) if @field + end + + def on_headers_complete(_reposse) + @finished[:headers] = true end - def on_body(chunk) + def on_body(_response, chunk) if @chunk @chunk << chunk else @@ -57,20 +69,21 @@ def read(size) chunk end - def on_message_complete - @finished = true + def on_message_complete(_response) + @finished[:message] = true end def reset - @parser.reset! + @state.reset! - @finished = false - @headers = nil + @finished = Hash.new(false) + @headers = HTTP::Headers.new + @field = nil @chunk = nil end def finished? - @finished + @finished[:message] end end end From dccaf8ee9c6768ad88ee59943b81c6c0656225d1 Mon Sep 17 00:00:00 2001 From: Pavel Forkert Date: Fri, 6 Jul 2018 13:54:14 +0300 Subject: [PATCH 4/4] Fix HTTP::Response::Parser not throwing exceptions when the actual parsing fails --- lib/http/response/parser.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/http/response/parser.rb b/lib/http/response/parser.rb index a71fb05e..fd10942d 100644 --- a/lib/http/response/parser.rb +++ b/lib/http/response/parser.rb @@ -15,7 +15,8 @@ def initialize end def add(data) - @parser.parse(@state, data) + error = @parser.parse(@state, data) + raise IOError, "Could not parse data" if error end alias << add