gamzinav/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MEDIUM: http: refrain from sending "Connection: close" when Upgrade is present

Browse Source 
Some servers are not totally HTTP-compliant when it comes to parsing the
Connection header. This is particularly true with WebSocket where it happens
from time to time that a server doesn't support having a "close" token along
with the "Upgrade" token in the Connection header. This broken behaviour has
also been noticed on some clients though the problem is less frequent on the
response path.

Sometimes the workaround consists in enabling "option http-pretend-keepalive"
to leave the request Connection header untouched, but this is not always the
most convenient solution. This patch introduces a new solution : haproxy now
also looks for the "Upgrade" token in the Connection header and if it finds
it, then it refrains from adding any other token to the Connection header
(though "keep-alive" and "close" may still be removed if found). The same is
done for the response headers.

This way, WebSocket much with less changes even when facing non-compliant
clients or servers. At least it fixes the DISCONNECT issue that was seen
on the websocket.org test.

Note that haproxy does not change its internal mode, it just refrains from
adding new tokens to the connection header.
This commit is contained in:
Willy Tarreau 2012-11-11 22:19:57 +01:00
parent 70c6fd82c3
commit 50fc7777c6
2 changed files with 22 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
include/types/proto_http.h
View File

@ -82,8 +82,9 @@
#define TX_CON_CLO_SET 0x00400000 /* "connection: close" is now set */
#define TX_CON_KAL_SET 0x00800000 /* "connection: keep-alive" is now set */
/* Unused: 0x1000000, 0x2000000 */
/* Unused: 0x1000000 */
#define TX_HDR_CONN_UPG 0x02000000 /* The "Upgrade" token was found in the "Connection" header */
#define TX_WAIT_NEXT_RQ 0x04000000 /* waiting for the second request to start, use keep-alive timeout */
#define TX_HDR_CONN_PRS 0x08000000 /* "connection" header already parsed (req or res), results below */

27
src/proto_http.c
View File

@ -1631,6 +1631,7 @@ static int http_upgrade_v09_to_v10(struct http_txn *txn)
* be removed, we remove them now. The <to_del> flags are used for that :
* - bit 0 means remove "close" headers (in HTTP/1.0 requests/responses)
* - bit 1 means remove "keep-alive" headers (in HTTP/1.1 reqs/resp to 1.1).
* Presence of the "Upgrade" token is also checked and reported.
* The TX_HDR_CONN_* flags are adjusted in txn->flags depending on what was
* found, and TX_CON_*_SET is adjusted depending on what is left so only
* harmless combinations may be removed. Do not call that after changes have
@ -1667,6 +1668,9 @@ void http_parse_connection_header(struct http_txn *txn, struct http_msg *msg, in
else
txn->flags |= TX_CON_CLO_SET;
}
else if (ctx.vlen >= 7 && word_match(ctx.line + ctx.val, ctx.vlen, "upgrade", 7)) {
txn->flags |= TX_HDR_CONN_UPG;
}
}
txn->flags |= TX_HDR_CONN_PRS;
@ -2545,7 +2549,7 @@ int http_wait_for_request(struct session *s, struct channel *req, int an_bit)
msg->flags |= HTTP_MSGF_VER_11;
/* "connection" has not been parsed yet */
txn->flags &= ~(TX_HDR_CONN_PRS | TX_HDR_CONN_CLO | TX_HDR_CONN_KAL);
txn->flags &= ~(TX_HDR_CONN_PRS | TX_HDR_CONN_CLO | TX_HDR_CONN_KAL | TX_HDR_CONN_UPG);
/* if the frontend has "option http-use-proxy-header", we'll check if
* we have what looks like a proxied connection instead of a connection,
@ -3661,9 +3665,14 @@ int http_process_request(struct session *s, struct channel *req, int an_bit)
}
}
/* 11: add "Connection: close" or "Connection: keep-alive" if needed and not yet set. */
if (((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_TUN) ||
((s->fe->options|s->be->options) & PR_O_HTTP_CLOSE)) {
/* 11: add "Connection: close" or "Connection: keep-alive" if needed and not yet set.
* If an "Upgrade" token is found, the header is left untouched in order not to have
* to deal with some servers bugs : some of them fail an Upgrade if anything but
* "Upgrade" is present in the Connection header.
*/
if (!(txn->flags & TX_HDR_CONN_UPG) &&
(((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_TUN) ||
((s->fe->options|s->be->options) & PR_O_HTTP_CLOSE))) {
unsigned int want_flags = 0;
if (msg->flags & HTTP_MSGF_VER_11) {
@ -4970,7 +4979,7 @@ int http_wait_for_response(struct session *s, struct channel *rep, int an_bit)
msg->flags |= HTTP_MSGF_VER_11;
/* "connection" has not been parsed yet */
txn->flags &= ~(TX_HDR_CONN_PRS|TX_HDR_CONN_CLO|TX_HDR_CONN_KAL|TX_CON_CLO_SET|TX_CON_KAL_SET);
txn->flags &= ~(TX_HDR_CONN_PRS|TX_HDR_CONN_CLO|TX_HDR_CONN_KAL|TX_HDR_CONN_UPG|TX_CON_CLO_SET|TX_CON_KAL_SET);
/* transfer length unknown*/
msg->flags &= ~HTTP_MSGF_XFER_LEN;
@ -5447,9 +5456,13 @@ int http_process_res_common(struct session *t, struct channel *rep, int an_bit,
/*
* 8: adjust "Connection: close" or "Connection: keep-alive" if needed.
* If an "Upgrade" token is found, the header is left untouched in order
* not to have to deal with some client bugs : some of them fail an upgrade
* if anything but "Upgrade" is present in the Connection header.
*/
if (((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_TUN) ||
((t->fe->options|t->be->options) & PR_O_HTTP_CLOSE)) {
if (!(txn->flags & TX_HDR_CONN_UPG) &&
(((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_TUN) ||
((t->fe->options|t->be->options) & PR_O_HTTP_CLOSE))) {
unsigned int want_flags = 0;
if ((txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL ||