Skip to content

Ignore non-final responses. #137

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

Merged
merged 4 commits into from
Oct 24, 2023
Merged
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
4 changes: 2 additions & 2 deletions async-http.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ Gem::Specification.new do |spec|
spec.add_dependency "async", ">= 1.25"
spec.add_dependency "async-io", ">= 1.28"
spec.add_dependency "async-pool", ">= 0.2"
spec.add_dependency "protocol-http", "~> 0.24.0"
spec.add_dependency "protocol-http1", "~> 0.15.0"
spec.add_dependency "protocol-http", "~> 0.25.0"
spec.add_dependency "protocol-http1", "~> 0.16.0"
spec.add_dependency "protocol-http2", "~> 0.15.0"
spec.add_dependency "traces", ">= 0.10.0"
end
22 changes: 20 additions & 2 deletions fixtures/async/http/a_protocol.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,26 @@ module HTTP
end
end

with "huge body", timeout: 600 do
let(:body) {::Protocol::HTTP::Body::File.open("/dev/zero", size: 512*1024**2)}
with "interim response" do
let(:app) do
::Protocol::HTTP::Middleware.for do |request|
request.write_interim_response(
::Protocol::HTTP::Response[103, [["link", "</style.css>; rel=preload; as=style"]]]
)

::Protocol::HTTP::Response[200, {}, ["Hello World"]]
end
end

it "can read informational response" do
response = client.get("/")
expect(response).to be(:success?)
expect(response.read).to be == "Hello World"
end
end

with "huge body" do
let(:body) {::Protocol::HTTP::Body::File.open("/dev/zero", size: 8*1024**2)}

let(:app) do
::Protocol::HTTP::Middleware.for do |request|
Expand Down
4 changes: 4 additions & 0 deletions lib/async/http/protocol/http1/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ def hijack?
def hijack!
@connection.hijack!
end

def write_interim_response(response)
@connection.write_interim_response(response.version, response.status, response.headers)
end
end
end
end
Expand Down
18 changes: 11 additions & 7 deletions lib/async/http/protocol/http1/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,21 @@ module Protocol
module HTTP1
class Response < Protocol::Response
def self.read(connection, request)
if parts = connection.read_response(request.method)
self.new(connection, *parts)
while parts = connection.read_response(request.method)
response = self.new(connection, *parts)

if response.final?
return response
end
end
end

UPGRADE = 'upgrade'

# @attribute [String] The HTTP response line reason.
# @attribute [String] The HTTP response line reason.
attr :reason

# @parameter reason [String] HTTP response line reason phrase
# @parameter reason [String] HTTP response line reason phrase.
def initialize(connection, version, status, reason, headers, body)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks here like the docs for the initialize args aren't fully here, or perhaps I am missing something that ain't visible right here in the commit diff.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, the documentation could do with some improvement I suppose.

@connection = connection
@reason = reason
Expand All @@ -34,7 +38,7 @@ def initialize(connection, version, status, reason, headers, body)
def connection
@connection
end

def hijack?
@body.nil?
end
Expand Down
10 changes: 10 additions & 0 deletions lib/async/http/protocol/http2/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,16 @@ def send_response(response)
@stream.send_headers(nil, headers, ::Protocol::HTTP2::END_STREAM)
end
end

def write_interim_response(response)
protocol_headers = [
[STATUS, response.status]
]

headers = ::Protocol::HTTP::Headers::Merged.new(protocol_headers, response.headers)

@stream.send_headers(nil, headers)
end
end
end
end
Expand Down
6 changes: 6 additions & 0 deletions lib/async/http/protocol/http2/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ def accept_push_promise_stream(promised_stream_id, headers)
# This should be invoked from the background reader, and notifies the task waiting for the headers that we are done.
def receive_initial_headers(headers, end_stream)
headers.each do |key, value|
# It's guaranteed that this should be the first header:
if key == STATUS
status = Integer(value)

# Ignore informational headers:
return if status >= 100 && status < 200

@response.status = Integer(value)
elsif key == PROTOCOL
@response.protocol = value
Expand Down
8 changes: 3 additions & 5 deletions lib/async/http/protocol/http2/stream.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,11 @@ def receive_trailing_headers(headers, end_stream)
end

def process_headers(frame)
if @headers.nil?
@headers = ::Protocol::HTTP::Headers.new
self.receive_initial_headers(super, frame.end_stream?)
elsif frame.end_stream?
if frame.end_stream? && @headers
self.receive_trailing_headers(super, frame.end_stream?)
else
raise ::Protocol::HTTP2::HeaderError, "Unable to process headers!"
@headers ||= ::Protocol::HTTP::Headers.new
self.receive_initial_headers(super, frame.end_stream?)
end

# TODO this might need to be in an ensure block:
Expand Down
3 changes: 3 additions & 0 deletions lib/async/http/protocol/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ def hijack?
false
end

def write_interim_response(response)
end

def peer
if connection = self.connection
connection.peer
Expand Down