Opened 2 years ago
Closed 2 years ago
Last modified 2 years ago
#2115 closed enhancement (wontfix)
Consider using "proxy_set_header Connection $http_connection;" in all docs and examples
|Reported by:||Owned by:|
|uname -a:||(none, this is a website change)|
|nginx -V:||(none, this is a website change)|
There are a number of places in shipped config and documentation that include the following config line:
proxy_set_header Connection "upgrade";
Please consider changing it instead to:
proxy_set_header Connection $http_connection;
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.
Change History (3)
comment:1 by , 2 years ago
|Status:||new → closed|
comment:2 by , 2 years ago
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?
comment:3 by , 2 years ago
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.
Thank you for your suggestion.
First of all, using
$http_connectionis almost always wrong. If you want to configure nginx to only use
Connection: upgradein 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
Connectionheader should be either
closeor 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
It looks like the github issue you've linked is a result of incorrect nginx configurations trying to use endpoint-specific
Connection: upgradefor 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.