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 4db7882e01c45fef4db8bdd314bb5d976b9887bc..ef9e125623764b615e41cf0b1b5ec286624e3bcf 100644 (file)
--- 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 8e96ae684d414902e60226020917154845f1f28d..c1fd6a74fbae3b34f64d3049add7ffc3aaa8438c 100644 (file)
--- 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 <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 ||