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 #433

Closed
wants to merge 3 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.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.0-pre2", "< 3"
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
41 changes: 27 additions & 14 deletions lib/http/response/parser.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -49,20 +61,21 @@ def chunk
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