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

Merged
merged 3 commits into from
Jun 30, 2025
Merged
Changes from 1 commit
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
8 changes: 7 additions & 1 deletion lib/async/http/faraday/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,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 @@ -181,6 +181,12 @@ def perform_request(env)
end

if headers = env.request_headers
# Ignore Content-Length if given, it will be set for us later anyway (lowercased)
Copy link
Member

Choose a reason for hiding this comment

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

This is looking good.

I'd suggest extracting this into length and then assigning to BodyReadWrapper, which should have this:

class BodyReadWrapper
  def initialize(body, length = nil, block_size: 4096)
    @body = body
    @length = length
    @block_size = block_size
  end

  # ...

  def length
    @length # if known
  end

You'll need to move the if headers = env.request_headers first to extract a local variable if possible.

Is headers.key?(...) case sensitive?

Testing is a bit tricky, but maybe we can mock the client. If you can get the code updated, I can help write the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ioquatix

Is headers.key?(...) case sensitive?

Yes. I'd change this to check for both Content-Length and content-length if the intent is to use that value for BodyReadWrapper#length, if you agree.

Copy link
Member

Choose a reason for hiding this comment

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

It might be easier, for a first pass, to construct headers = ::Protocol::HTTP::Headers[headers] first, and then length = headers.delete("content-length").

if headers.has_key?("Content-Length")
headers = headers.dup
headers.delete("Content-Length")
end

headers = ::Protocol::HTTP::Headers[headers]
end

Expand Down
Loading