Ignore Content-Length from env.request_headers

[pilaf]

Closes #51

This makes the adapter ignore Content-Length if given in Faraday's env.request_headers, but only if capitalized, since Async::HTTP will set content-length anyway.

I'd like to add tests too, but I wasn't sure how to test what headers end up in the final request without setting up a dummy server to receive them. Any suggestions?

[ioquatix]

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.

[pilaf]

@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.

[ioquatix]

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

[pilaf]

This double initialization is ugly but I had no choice since Protocol::HTTP::Body::Buffered.wrap doesn't take a length argument.

[ioquatix]

I understand how you are thinking about the problem.

A buffered body can compute its size accurately and efficiently from the chunks. That's different from a readable body where we can only call #read to stream the data.

Therefore, I wouldn't worry about this code path - just leave it as the original implementation.

[pilaf]

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

[ioquatix]

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.

[pilaf]

@ioquatix Sorry for the delay, please check the latest commit, I think it mostly addresses your comments.

Again I couldn't write tests for it, but I did test that this code works separately.

[ioquatix]

Thanks, this looks like what I was thinking of, I'll try to add some tests, and if it looks good we should be able to move forward.

[ioquatix]

This is perfect, thanks.