From a0d416251b4caa357b7b34a2489a54afcae3460e Mon Sep 17 00:00:00 2001 From: Alexey Zapparov Date: Mon, 21 Oct 2019 23:39:31 +0200 Subject: [PATCH] Backport HTTP parser upgrade (#489) from master --- .travis.yml | 7 ++--- CHANGES.md | 8 ++++++ README.md | 7 ++--- http.gemspec | 6 ++--- lib/http.rb | 2 -- lib/http/response/parser.rb | 54 +++++++++++++++++++++++++++---------- lib/http/version.rb | 2 +- spec/regression_specs.rb | 7 +++++ 8 files changed, 67 insertions(+), 26 deletions(-) diff --git a/.travis.yml b/.travis.yml index e9b943a8..2789a241 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,18 +17,19 @@ env: JRUBY_OPTS="$JRUBY_OPTS --debug" rvm: # Include JRuby first because it takes the longest - - jruby-9.1.16.0 + - jruby-9.2.5.0 - 2.3 - 2.4 - 2.5 + - 2.6 matrix: fast_finish: true include: # Only run RuboCop and Yardstick metrics on the latest Ruby - - rvm: 2.5 + - rvm: 2.6 env: SUITE="rubocop" - - rvm: 2.5 + - rvm: 2.6 env: SUITE="yardstick" branches: diff --git a/CHANGES.md b/CHANGES.md index 8a33bc42..74017039 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,10 @@ +## 4.2.0 (WIP) + +* [#489](https://github.com/httprb/http/pull/489) + Fix HTTP parser. + ([@ixti], [@fxposter]) + + ## 4.1.1 (2019-03-12) * Add `HTTP::Headers::ACCEPT_ENCODING` constant. @@ -771,4 +778,5 @@ end [@tycoon]: https://github.com/tycooon [@paul]: https://github.com/paul [@RickCSong]: https://github.com/RickCSong +[@fxposter]: https://github.com/fxposter [@mamoonraja]: https://github.com/mamoonraja diff --git a/README.md b/README.md index 1f80e111..a2d94da6 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![Build Status](https://secure.travis-ci.org/httprb/http.svg?branch=4-x-stable)](https://travis-ci.org/httprb/http) [![Code Climate](https://codeclimate.com/github/httprb/http.svg?branch=4-x-stable)](https://codeclimate.com/github/httprb/http) [![Coverage Status](https://coveralls.io/repos/httprb/http/badge.svg?branch=4-x-stable)](https://coveralls.io/r/httprb/http) -[![MIT licensed](https://img.shields.io/badge/license-MIT-blue.svg)](https://github.com/httprb/http/blob/master/LICENSE.txt) +[![MIT licensed](https://img.shields.io/badge/license-MIT-blue.svg)](https://github.com/httprb/http/blob/4-x-stable/LICENSE.txt) [Documentation] @@ -167,7 +167,8 @@ versions: * Ruby 2.3.x * Ruby 2.4.x * Ruby 2.5.x -* JRuby 9.1.x.x +* Ruby 2.6.x +* JRuby 9.2.x.x If something doesn't work on one of these versions, it's a bug. @@ -197,5 +198,5 @@ dropped. ## Copyright -Copyright (c) 2011-2018 Tony Arcieri, Alexey V. Zapparov, Erik Michaels-Ober, Zachary Anker. +Copyright (c) 2011-2019 Tony Arcieri, Alexey V. Zapparov, Erik Michaels-Ober, Zachary Anker. See LICENSE.txt for further details. diff --git a/http.gemspec b/http.gemspec index 0d2f9499..9169a1fc 100644 --- a/http.gemspec +++ b/http.gemspec @@ -27,10 +27,10 @@ Gem::Specification.new do |gem| gem.required_ruby_version = ">= 2.3" - gem.add_runtime_dependency "http_parser.rb", "~> 0.6.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" + gem.add_runtime_dependency "http-cookie", "~> 1.0" + gem.add_runtime_dependency "http-form_data", "~> 2.0" + gem.add_runtime_dependency "http-parser", "~> 1.2.0" gem.add_development_dependency "bundler", "~> 2.0" diff --git a/lib/http.rb b/lib/http.rb index 9b8ec5b4..afc57fe6 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..19989d59 100644 --- a/lib/http/response/parser.rb +++ b/lib/http/response/parser.rb @@ -1,41 +1,66 @@ # frozen_string_literal: true +require "http-parser" + module HTTP class Response + # @api private + # + # NOTE(ixti): This class is a subject of future refactoring, thus don't + # expect this class API to be stable until this message disappears and + # class is not marked as private anymore. 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 + # @return [self] def add(data) - @parser << data + # XXX(ixti): API doc of HttpParser::Parser is misleading, it says that + # it returns boolean true if data was parsed successfully, but instead + # it's response tells if there was an error; So when it's `true` that + # means parse failed, and `false` means parse was successful. + # case of success. + return self unless @parser.parse(@state, data) + + raise IOError, "Could not parse 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 +82,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 diff --git a/lib/http/version.rb b/lib/http/version.rb index 18b0946d..d17e0ef7 100644 --- a/lib/http/version.rb +++ b/lib/http/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module HTTP - VERSION = "4.1.1" + VERSION = "4.2.0.pre" end 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