Skip to content

Ignore Content-Length from env.request_headers #52

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
24 changes: 17 additions & 7 deletions lib/async/http/faraday/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,16 @@ class BodyReadWrapper < ::Protocol::HTTP::Body::Readable
#
# @parameter body [Interface(:read)] The input body to wrap.
# @parameter block_size [Integer] The size of the blocks to read from the body.
def initialize(body, block_size: 4096)
# @parameter length [Integer | Nil] The length of the body, if known.
def initialize(body, length = nil, block_size: 4096)
@body = body
@length = length
@block_size = block_size
end

# @attribute [Integer | Nil] The total length of the body, or `nil` if the length is unknown.
attr :length

# Close the body if possible.
def close(error = nil)
@body.close if @body.respond_to?(:close)
Expand Down Expand Up @@ -111,7 +116,7 @@ def self.setup_parallel_manager(**options)
SocketError
].freeze

# Create a Farady compatible adapter.
# Create a Faraday compatible adapter.
#
# @parameter timeout [Integer] The timeout for requests.
# @parameter options [Hash] Additional options to pass to the underlying Async::HTTP::Client.
Expand Down Expand Up @@ -168,22 +173,27 @@ def call(env)

def perform_request(env)
with_client(env) do |endpoint, client|
if headers = env.request_headers
headers = ::Protocol::HTTP::Headers[headers]

# Use content-length to inform body length if given, but remove the header since it will be
# set for us later anyway, and not doing so could result in a duplicate content-length headers
# if capitalization differs
content_length = headers.delete("content-length")&.to_i
end

if body = env.body
# We need to ensure the body is wrapped in a Readable object so that it can be read in chunks:
# Faraday's body only responds to `#read`.
if body.is_a?(::Protocol::HTTP::Body::Readable)
# Good to go
Comment on lines 188 to 189
Copy link
Author

Choose a reason for hiding this comment

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

Should this case try to rebuild the body with the given length too?

Copy link
Member

Choose a reason for hiding this comment

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

No I wouldn’t worry about this case either, it’s expected the body would know its size already if appropriate.

The main issue we are addressing is Faradays opaque “body#read” which may have length but the only way to know that is via the supplied header.

elsif body.respond_to?(:read)
body = BodyReadWrapper.new(body)
body = BodyReadWrapper.new(body, content_length)
else
body = ::Protocol::HTTP::Body::Buffered.wrap(body)
end
end

if headers = env.request_headers
headers = ::Protocol::HTTP::Headers[headers]
end

method = env.method.to_s.upcase

request = ::Protocol::HTTP::Request.new(endpoint.scheme, endpoint.authority, method, endpoint.path, nil, headers, body)
Expand Down
Loading