Skip to content
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

Ensure consistent Connection header value #179

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mattbearman
Copy link

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

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:

```rb
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant