BUG/MINOR: mux-h2: commit the current stream ID even on reject
The H2 spec says that a HEADERS frame turns an idle stream to the open
state, and it may then turn to half-closed(remote) on ES, then to close,
all at once, if we respond with RST (e.g. on error). Due to the fact that
we process a complete frame at once since h2_dec_hdrs() may reassemble
CONTINUATION frames until everything is complete, the state was only
committed after the frame was completley valid (otherwise multiple passes
could result in subsequent frames being rejected as the stream ID would
be equal to the highest one).
However this is not correct because it means that a client may retry on
the same ID as a previously failed one, which technically is forbidden
(for example the client couldn't know which of them a WINDOW_UPDATE or
RST_STREAM frame is for).
In practice, due to the error paths, this would only be possible when
failing to decode HPACK while leaving the HPACK stream intact, thus
when the valid decoded HPACK stream cannot be turned into a valid HTTP
representation, e.g. when the resulting headers are too large for example.
The solution to avoid this consists in committing the stream ID on this
error path as well. h2spec continues to be happy.
Thanks to Annika Wickert and Tim Windelschmidt for reporting this issue.
This fix must be backported to all stable versions.
(cherry picked from commit 250b630fb933d652533487cba9e2ae448582b550)
[ad; remove trace added in 2.9]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit f69d53e6c20b432e80b63a3abf8737ba6180f74c)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit da9e680757ad791de32ef8dd239c917958abe848)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit db291635f44c435adbf037fffe6a2d15537e5008)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 4099139..f13a659 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2829,6 +2829,13 @@
*/
sess_log(h2c->conn->owner);
h2s = (struct h2s*)h2_error_stream;
+
+ /* This stream ID is now opened anyway until we send the RST on
+ * it, it must not be reused.
+ */
+ if (h2c->dsi > h2c->max_id)
+ h2c->max_id = h2c->dsi;
+
goto send_rst;
}