Skip to content

Commit

Permalink
Switch to http-parser (FFI)
Browse files Browse the repository at this point in the history
Huge sthanks to [@fxposter](https://github.com/fxposter) for helping on
catching and fixing the problem with underlying parser.

This commit is a squashed and enhanced (with some comments) version of
[PR#489](#489).

Closes #489
  • Loading branch information
ixti committed Jan 14, 2019
1 parent 5187a1d commit f1db84f
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 17 deletions.
2 changes: 1 addition & 1 deletion http.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ 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-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"
Expand Down
2 changes: 0 additions & 2 deletions lib/http.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# frozen_string_literal: true

require "http/parser"

require "http/errors"
require "http/timeout/null"
require "http/timeout/per_operation"
Expand Down
54 changes: 40 additions & 14 deletions lib/http/response/parser.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions spec/regression_specs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit f1db84f

Please sign in to comment.