Ensure consistent Connection header value

[mattbearman]

Depending on how you start a connection, the Connection header may be unexpectedly set to close

Creating a new instance of Net::HTTP and then calling its request method (or any of its methods that call request, eg: get) will first set the Connection header to close (if it's not already set), then start the connection, and re-call itself

Where as initiating a request with class methods, or by manually starting a connection before calling request will not set the Connection header

As the default connection in HTTP 1.1 is keep-alive, having the Connection header silently set to close is unexpected

Examples:

uri = URI('https://example.com/test')

http = Net::HTTP.new(uri.host, uri.port)
http.use_ssl = true
req = Net::HTTP::Get.new(uri.path)
req['connection'] # => nil
res = http.request(req)
req['connection'] # => 'close'

http = Net::HTTP.new(uri.host, uri.port)
http.use_ssl = true
http.start
req = Net::HTTP::Get.new(uri.path)
req['connection'] # => nil
res = http.request(req)
req['connection'] # => nil

From what I can see this functionality was added 24 years ago when implicit starts to GET and HEAD requests were introduced. Not long later the shortcut class methods (eg: Net::HTTP.get) were added, which called start as part of the method body, bypassing the implicit start therefore not setting the Connection: close header, creating inconsistent behaviour

This PR stops the Connection header from being set to close when calling request without an open connection, as that matches the behaviour of the more commonly used class methods, as well as allowing the HTTP 1.1 default of keep-alive to be used unless the Connection header is explicitly set

[jeremyevans]

I believe the current behavior is by design. If you would like to pipeline requests, you should wrap them inside a Net::HTTP.start block (or call start manually as in your second example). Otherwise, the connection/file descriptor can accidentally be left open:

Net::HTTP.start(uri.host, uri.port) do |http|
  req = Net::HTTP::Get.new(uri.path)
  req['connection'] # => nil
  res = http.request(req)
  req['connection'] # => 'close'
end

It sounds like you may want the connection/file descriptor left open even without explicit connection management via start, but I can assure you that this is not a universal desire. It you really want that behavior:

def (Net::HTTPRequest).new(...)
  req = super
  req['connection'] = 'keep-alive'
  req
end

[mattbearman]

@jeremyevans thanks for taking the time to review this. I must admit it hadn't occurred to me this was by design.

I can see how it makes sense to use connection=>close when you're just making a single request to a server, rather than forcing the user to manually close a keep-alive connection

We weren't trying to deliberately leave connections hanging, the opposite actually. It was some legacy code I was working on that was using net/http in the connection=>close way, combined with a server that freaked out if the connection wasn't keep alive. I probably should have included the http.finish in my examples to make that a bit clearer