RepoMirrors
/
haproxy
mirror of http://git.haproxy.org/git/haproxy.git/
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

BUG/MEDIUM: mux-h1/mux-h2: Reject upgrades with payload on H2 side only 

Since 1d2d77b27 ("MEDIUM: mux-h1: Return a 501-not-implemented for upgrade
requests with a body"), it is no longer possible to perform a protocol
upgrade for requests with a payload. The main reason was to be able to
support protocol upgrade for H1 client requesting a H2 server. In that case,
the upgrade request is converted to a CONNECT request. So, it is not
possible to convey a payload in that case.

But, it is a problem for anyone wanting to perform upgrades on H1 server
using requests with a payload. It is uncommon but valid. So, now, it is the
H2 multiplexer responsibility to reject upgrade requests, on server side, if
there is a payload. An INTERNAL_ERROR is returned for the H2S in that
case. On H1 side, the upgrade is now allowed, but only if the server waits
for the end of the request to return the 101-Switching-protocol
response. Indeed, it is quite hard to synchronise the frontend side and the
backend side in that case. Asking to servers to fully consume the request
payload before returned the response seems reasonable.

This patch should fix the issue #2684. It could be backported after a period
of observation, as far as 2.4 if possible. But only if it is not too
hard. It depends on "MINOR: mux-h1: Set EOI on SE during demux when both
side are in DONE state".
This commit is contained in:
Christopher Faulet 2024-09-06 09:16:16 +02:00
parent ad1ef94612
commit 001fb1a548
3 changed files with 16 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/h1_htx.c
View File

@ -182,11 +182,9 @@ static int h1_postparse_req_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
flags |= h1m_htx_sl_flags(h1m);
/* Remove Upgrade header in problematic cases :
* - body present
* - "h2c" or "h2" token specified as token
*/
if (((flags & (HTX_SL_F_CONN_UPG|HTX_SL_F_BODYLESS)) == HTX_SL_F_CONN_UPG) ||
((h1m->flags & (H1_MF_CONN_UPG|H1_MF_UPG_H2C)) == (H1_MF_CONN_UPG|H1_MF_UPG_H2C))) {
if ((h1m->flags & (H1_MF_CONN_UPG|H1_MF_UPG_H2C)) == (H1_MF_CONN_UPG|H1_MF_UPG_H2C)) {
int i;
for (i = 0; hdrs[i].n.len; i++) {

10
src/mux_h1.c
View File

@ -2130,8 +2130,16 @@ static size_t h1_process_demux(struct h1c *h1c, struct buffer *buf, size_t count
}
if ((h1m->flags & H1_MF_RESP) &&
((h1s->meth == HTTP_METH_CONNECT && h1s->status >= 200 && h1s->status < 300) || h1s->status == 101))
((h1s->meth == HTTP_METH_CONNECT && h1s->status >= 200 && h1s->status < 300) || h1s->status == 101)) {
if (h1s->req.state != H1_MSG_DONE) {
TRACE_STATE("Reject tunnel because request is not finished", H1_EV_RX_DATA|H1_EV_H1S_BLK, h1c->conn, h1s);
h1s->flags |= H1S_F_PARSING_ERROR;
htx->flags |= HTX_FL_PARSING_ERROR;
h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
break;
}
h1_set_tunnel_mode(h1s);
}
else {
if (h1s->req.state < H1_MSG_DONE || h1s->res.state < H1_MSG_DONE) {
/* Unfinished transaction: block this input side waiting the end of the output side */

6
src/mux_h2.c
View File

@ -5989,6 +5989,12 @@ static size_t h2s_snd_bhdrs(struct h2s *h2s, struct htx *htx)
if ((sl->flags & HTX_SL_F_CONN_UPG) && isteqi(list[hdr].n, ist("connection"))) {
/* rfc 7230 #6.1 Connection = list of tokens */
struct ist connection_ist = list[hdr].v;
if (!(sl->flags & HTX_SL_F_BODYLESS)) {
TRACE_STATE("cannot convert upgrade for request with payload", H2_EV_TX_FRAME|H2_EV_TX_HDR, h2c->conn, h2s);
goto fail;
}
do {
if (isteqi(iststop(connection_ist, ','),
ist("upgrade"))) {