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.
diff --git a/src/mux_h1.c b/src/mux_h1.c
index 7c5451c..b548849 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1107,41 +1107,57 @@
/*
* 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)) {
+ TRACE_STATE("switch H1 request in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
+
+ if (!conn_is_back(h1s->h1c->conn) && h1s->res.state < H1_MSG_DONE) {
h1s->h1c->flags |= H1C_F_IN_BUSY;
TRACE_STATE("switch h1c in busy mode", H1_EV_RX_DATA|H1_EV_H1C_BLK, h1s->h1c->conn, h1s);
}
+ else if (h1s->h1c->flags & H1C_F_IN_BUSY) {
+ h1s->h1c->flags &= ~H1C_F_IN_BUSY;
+ tasklet_wakeup(h1s->h1c->wait_event.tasklet);
+ TRACE_STATE("h1c no more busy", H1_EV_RX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1s->h1c->conn, h1s);
+ }
}
/*
* 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;
+ TRACE_STATE("switch H1 request in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
+ }
+
h1s->res.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
h1s->res.state = H1_MSG_TUNNEL;
+ TRACE_STATE("switch H1 response in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
+
if (conn_is_back(h1s->h1c->conn) && h1s->req.state < H1_MSG_DONE) {
h1s->h1c->flags |= H1C_F_IN_BUSY;
TRACE_STATE("switch h1c in busy mode", H1_EV_RX_DATA|H1_EV_H1C_BLK, h1s->h1c->conn, h1s);
}
- 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);
- TRACE_STATE("h1c no more busy", H1_EV_RX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1s->h1c->conn, h1s);
- }
+ else if (h1s->h1c->flags & H1C_F_IN_BUSY) {
+ h1s->h1c->flags &= ~H1C_F_IN_BUSY;
+ tasklet_wakeup(h1s->h1c->wait_event.tasklet);
+ TRACE_STATE("h1c no more busy", H1_EV_RX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1s->h1c->conn, h1s);
}
}
@@ -1202,17 +1218,13 @@
if (!(h1m->flags & H1_MF_RESP)) {
h1s->meth = h1sl.rq.meth;
- if (h1m->state == H1_MSG_TUNNEL) {
+ if (h1m->state == H1_MSG_TUNNEL)
h1_set_req_tunnel_mode(h1s);
- TRACE_STATE("switch H1 request in tunnel mode", H1_EV_RX_DATA|H1_EV_RX_HDRS, h1s->h1c->conn, h1s);
- }
}
else {
h1s->status = h1sl.st.status;
- if (h1m->state == H1_MSG_TUNNEL) {
+ if (h1m->state == H1_MSG_TUNNEL)
h1_set_res_tunnel_mode(h1s);
- TRACE_STATE("switch H1 response in tunnel mode", H1_EV_RX_DATA|H1_EV_RX_HDRS, h1s->h1c->conn, h1s);
- }
}
h1_process_input_conn_mode(h1s, h1m, htx);
*ofs += ret;
@@ -1413,7 +1425,9 @@
H1_EV_RX_DATA|H1_EV_RX_EOI, h1c->conn, h1s, htx);
}
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;
TRACE_STATE("switch h1c in busy mode", H1_EV_RX_DATA|H1_EV_H1C_BLK, h1c->conn, h1s);
}
@@ -1744,11 +1758,12 @@
if (!(h1m->flags & H1_MF_RESP) && h1s->meth == HTTP_METH_CONNECT) {
/* a CONNECT request is sent to the server. Switch it to tunnel mode. */
h1_set_req_tunnel_mode(h1s);
- TRACE_STATE("switch H1 request in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1c->conn, 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);
TRACE_STATE("switch H1 response in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1c->conn, h1s);
}
@@ -1859,7 +1874,11 @@
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);
+ TRACE_STATE("switch H1 request in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1c->conn, h1s);
+ }
+ else if (h1s->h1c->flags & H1C_F_IN_BUSY) {
h1s->h1c->flags &= ~H1C_F_IN_BUSY;
tasklet_wakeup(h1s->h1c->wait_event.tasklet);
TRACE_STATE("h1c no more busy", H1_EV_TX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1c->conn, h1s);
@@ -2700,6 +2719,7 @@
end:
if (conn_xprt_read0_pending(cs->conn)) {
h1s->flags |= H1S_F_REOS;
+ h1s->flags &= ~(H1S_F_BUF_FLUSH|H1S_F_SPLICED_DATA);
TRACE_STATE("read0 on connection", H1_EV_STRM_RECV, cs->conn, h1s);
}