From: Willy Tarreau Date: Sun, 11 Nov 2012 21:19:57 +0000 (+0100) Subject: MEDIUM: http: refrain from sending "Connection: close" when Upgrade is present X-Git-Tag: v1.5-dev13~48 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=50fc7777c645ee1f647181f27aaad06778d9f55d;p=thirdparty%2Fhaproxy.git MEDIUM: http: refrain from sending "Connection: close" when Upgrade is present 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. --- diff --git a/include/types/proto_http.h b/include/types/proto_http.h index 4db7882e01..ef9e125623 100644 --- a/include/types/proto_http.h +++ b/include/types/proto_http.h @@ -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 */ diff --git a/src/proto_http.c b/src/proto_http.c index 8e96ae684d..c1fd6a74fb 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -1631,6 +1631,7 @@ static int http_upgrade_v09_to_v10(struct http_txn *txn) * be removed, we remove them now. The 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 ||