BUG/MINOR: mux-h1: Fix tunnel mode detection on the response path
There are two issues with the way tunnel mode is detected on the response
path. First, when a response with an unknown content length is handled, the
request is also switched in tunnel mode. It is obviously wrong. Because it was
done on the server side only (so not during the request parsing), it is no
noticeable effects.
The second issue is about the way protocol upgrades are handled. The request is
switched in tunnel mode from the time the 101 response is processed. So an
unfinished request may be switched in tunnel mode too early. It is not a common
use, but a protocol upgrade on a POST is allowed. Thus, parsing of the payload
may be hijacked. It is especially bad for chunked payloads.
Now, conditions to switch the request in tunnel mode reflect what should be
done. Especially for the second issue. We wait the end of the request to switch
it in tunnel mode.
This patch must be backported to 2.0 and 1.9. Note that these versions are only
affected by the second issue but the patch cannot be easily splitted.
(cherry picked from commit f3158e94ee3a2577bb3b075202c9a6616a50bf92)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/mux_h1.c b/src/mux_h1.c
index df382e0..c25a0b9 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -880,36 +880,46 @@
/*
* Switch the request to tunnel mode. This function must only be called for
- * CONNECT requests. On the client side, the mux is mark as busy on input,
- * waiting the response.
+ * CONNECT requests. On the client side, if the response is not finished, the
+ * mux is mark as busy on input.
*/
static void h1_set_req_tunnel_mode(struct h1s *h1s)
{
h1s->req.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
h1s->req.state = H1_MSG_TUNNEL;
- if (!conn_is_back(h1s->h1c->conn))
+ if (!conn_is_back(h1s->h1c->conn) && h1s->res.state < H1_MSG_DONE)
h1s->h1c->flags |= H1C_F_IN_BUSY;
+ else if (h1s->h1c->flags & H1C_F_IN_BUSY) {
+ h1s->h1c->flags &= ~H1C_F_IN_BUSY;
+ tasklet_wakeup(h1s->h1c->wait_event.tasklet);
+ }
}
/*
* Switch the response to tunnel mode. This function must only be called on
- * successfull replies to CONNECT requests or on protocol switching. On the
- * server side, if the request is not finished, the mux is mark as busy on
- * input. Otherwise the request is also switch to tunnel mode.
+ * successfull replies to CONNECT requests or on protocol switching. In this
+ * last case, this function takes care to switch the request to tunnel mode if
+ * possible. On the server side, if the request is not finished, the mux is mark
+ * as busy on input.
*/
static void h1_set_res_tunnel_mode(struct h1s *h1s)
{
+ /* On protocol switching, switch the request to tunnel mode if it is in
+ * DONE state. Otherwise we will wait the end of the request to switch
+ * it in tunnel mode.
+ */
+ if (h1s->status == 101 && h1s->req.state == H1_MSG_DONE) {
+ h1s->req.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
+ h1s->req.state = H1_MSG_TUNNEL;
+ }
+
h1s->res.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
h1s->res.state = H1_MSG_TUNNEL;
if (conn_is_back(h1s->h1c->conn) && h1s->req.state < H1_MSG_DONE)
h1s->h1c->flags |= H1C_F_IN_BUSY;
- else {
- h1s->req.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
- h1s->req.state = H1_MSG_TUNNEL;
- if (h1s->h1c->flags & H1C_F_IN_BUSY) {
- h1s->h1c->flags &= ~H1C_F_IN_BUSY;
- tasklet_wakeup(h1s->h1c->wait_event.tasklet);
- }
+ else if (h1s->h1c->flags & H1C_F_IN_BUSY) {
+ h1s->h1c->flags &= ~H1C_F_IN_BUSY;
+ tasklet_wakeup(h1s->h1c->wait_event.tasklet);
}
}
@@ -1446,7 +1456,9 @@
break;
}
else if (h1m->state == H1_MSG_DONE) {
- if (h1s->req.state < H1_MSG_DONE || h1s->res.state < H1_MSG_DONE)
+ if (!(h1m->flags & H1_MF_RESP) && h1s->status == 101)
+ h1_set_req_tunnel_mode(h1s);
+ else if (h1s->req.state < H1_MSG_DONE || h1s->res.state < H1_MSG_DONE)
h1c->flags |= H1C_F_IN_BUSY;
break;
}
@@ -1706,9 +1718,11 @@
/* a CONNECT request is sent to the server. Switch it to tunnel mode. */
h1_set_req_tunnel_mode(h1s);
}
- else if ((h1s->meth == HTTP_METH_CONNECT && h1s->status == 200) || h1s->status == 101) {
+ else if ((h1m->flags & H1_MF_RESP) &&
+ ((h1s->meth == HTTP_METH_CONNECT && h1s->status == 200) || h1s->status == 101)) {
/* a successfull reply to a CONNECT or a protocol switching is sent
- * to the client . Switch the response to tunnel mode. */
+ * to the client. Switch the response to tunnel mode.
+ */
h1_set_res_tunnel_mode(h1s);
}
else if ((h1m->flags & H1_MF_RESP) &&
@@ -1777,7 +1791,9 @@
goto error;
done:
h1m->state = H1_MSG_DONE;
- if (h1s->h1c->flags & H1C_F_IN_BUSY) {
+ if (!(h1m->flags & H1_MF_RESP) && h1s->status == 101)
+ h1_set_req_tunnel_mode(h1s);
+ else if (h1s->h1c->flags & H1C_F_IN_BUSY) {
h1s->h1c->flags &= ~H1C_F_IN_BUSY;
tasklet_wakeup(h1s->h1c->wait_event.tasklet);
}
@@ -2474,6 +2490,7 @@
end:
if (conn_xprt_read0_pending(cs->conn)) {
h1s->flags |= H1S_F_REOS;
+ h1s->flags &= ~(H1S_F_BUF_FLUSH|H1S_F_SPLICED_DATA);
}
return ret;
}