Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix HTTP parsing (second attempt) #489

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion http.gemspec
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions 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"
Expand Down
42 changes: 28 additions & 14 deletions lib/http/response/parser.rb
@@ -1,41 +1,54 @@
# 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
error = @parser.parse(@state, data)
raise IOError, "Could not parse data" if error
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 +70,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
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