Consider using "proxy_set_header Connection $http_connection;" in all docs and examples

[sbrudenell@…]

Hi,

There are a number of places in shipped config and documentation that include the following config line:

proxy_set_header Connection "upgrade";

(especially: ​here)

Please consider changing it instead to:

proxy_set_header Connection $http_connection;

An unconditional Connection: upgrade seems to cause failures in some strict-parsing servers, particularly anything based on ASP.NET which rejects the request at a framework level.

Please see ​this github issue for some more discussion, and reports from affected people.

I know the fault really lies with administrators and container-authors blindly copying and pasting config from examples, but I think it would indirectly help a lot of people if you changed the examples.

Thanks!

[Maxim Dounin]

Thank you for your suggestion.

First of all, using $http_connection is almost always wrong. If you want to configure nginx to only use Connection: upgrade in upstream connections, consider using map-based configuration provided in the ​WebSocket proxying article you've linked. Depending on usage of keepalive connections to upstreams, the default value for the Connection header should be either close or an empty string.

Second, WebSocket proxying is expected to be configured only for specific endpoints which handle WebSocket connection requests. It is wrong to configure it globally for all proxying, even with map-based example.

It looks like the github issue you've linked is a result of incorrect nginx configurations trying to use endpoint-specific Connection: upgrade for proxying all requests. Correct approach is to fix such configurations. Trying to fix nginx docs, which already provide appropriate examples, is hardly the way to go.

[sbrudenell@…]

Correct approach is to fix such configurations

I agree of course. But the fact is that blindly copying and pasting configuration is incredibly common.

If you agree that Connection: upgrade for all requests is "incorrect", would you consider at least removing this as an example from the documentation?

[Maxim Dounin]

The example in the article does not imply using Connection: upgrade globally for all proxying. Rather, it specifically limits it to a particular endpoint by using the /chat/ location. Further, it is immediately followed by a more sophisticated example with map, providing a clear path from a very specific configuration to be used only for WebSocket proxying to some mixed-content applications. Removing the simple example looks like a wrong solution.