socketry / async-http

MIT License
298 stars 45 forks source link

Unexpected response headers in HTTP1.0 #57

Closed guppy0356 closed 4 years ago

guppy0356 commented 4 years ago

I try to contribute to webmock to fix InvalidURIError on https://github.com/bblimke/webmock/pull/890. I found unexpected response headers in Travis CI result. 👉 https://travis-ci.org/github/bblimke/webmock/jobs/696308825

Is it normal for response headers to contain keep-alive in http1.0 ? I think https://github.com/socketry/async-http/commit/a832d02cdcbdf4c64570cabd2008fc1a3d4c7daa has affect this response.

ioquatix commented 4 years ago

If possible can you show me the full request and response details.

Yes, HTTP/1.0 requires Connection: keep-alive for persistent connections.

guppy0356 commented 4 years ago

@ioquatix

I'm sorry to be late. I tried this program in Ruby 2.6.3.

require 'webmock'
include WebMock::API

WebMock.enable!

stub_request(:get, 'www.example.com').to_return(body: 'hi, there!')

result = Async do
  endpoint = Async::HTTP::Endpoint.parse('http://www.example.com')

  begin
    Async::HTTP::Client.open(endpoint, Async::HTTP::Protocol::HTTP10) do |client|
      response = client.send(
        :get,
        endpoint.path,
        {},
        nil
      )

      {
        status: response.status,
        headers: response.headers.to_h,
        body: response.read
      }
    end
  rescue Async::TimeoutError => e
    e
  end
end.wait

p result #=> {:status=>200, :headers=>{"connection"=>["keep-alive"]}, :body=>"hi, there!"}
ioquatix commented 4 years ago

That looks correct to me. It would probably make sense to scrub connection headers (and per-hop) headers from the response if you are computing a checksum/hash.

guppy0356 commented 4 years ago

@ioquatix thank you for your cooperation.

ioquatix commented 4 years ago

You are most welcome!