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

BUG/MEDIIM: stconn: Flush output data before forwarding close to write side 

In process_stream(), we wait to have an empty output channel to forward a
close to the write side (a shutw). However, at the stream-connector level,
when a close is detected on one side and we don't want to keep half-close
connections, the shutw is unconditionally forwarded to the write side. This
typically happens on server side.

At first glance, this bug may truncate messages. But depending on the muxes
and the stream states, the bug may be more visible. On recent versions
(2.8-dev and 2.7) and on 2.2 and 2.0, the stream may be freezed, waiting for
the client timeout, if the client mux is unable to forward data because the
client is too slow _AND_ the response channel is not empty _AND_ the server
closes its connection _AND_ the server mux has forwarded all data to the
upper layer _AND_ the client decides to send some data and to close its
connection. On 2.6 and 2.4, it is worst. Instead of a freeze, the client mux
is woken up in loop.

Of course, conditions are pretty hard to meet. Especially because it is highly
time dependent. For what it's worth, I reproduce it with tcploop on client and
server sides and a basic HTTP configuration for HAProxy:

  * client: tcploop -v 8889 C S:"GET / HTTP/1.1\r\nConnection: upgrade\r\n\r\n" P5000 S:"1234567890" K
  * server: tcploop -v 8000 L A R S:"HTTP/1.1 101 ok\r\nConnection: upgrade\r\n\r\n" P2000 S2660000 F R

On 2.8-dev, without this patch, the stream is freezed and when the client
connection timed out, client data are truncated and '--cL' is reported in
logs. With the patch, the client data are forwarded to the server and the
connection is closed. A '--CD' is reported in logs.

It is an old bug. It was probably introduced with the multiplexers. To fix
it, in stconn (Formerly the stream-interface), we must wait all output data
be flushed before forwarding close to write side.

This patch must be backported as far as 2.2 and must be evaluated for 2.0.
This commit is contained in:
Christopher Faulet 2022-12-05 07:42:00 +01:00
parent 30fc27750d
commit 7f59d68fe2
1 changed files with 4 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/stconn.c
View File

@ -518,7 +518,7 @@ static void sc_app_shutr(struct stconn *sc)
if (sc->flags & SC_FL_ISBACK)
__sc_strm(sc)->conn_exp = TICK_ETERNITY;
}
else if (sc->flags & SC_FL_NOHALF) {
else if ((sc->flags & SC_FL_NOHALF) && channel_is_empty(ic)) {
/* we want to immediately forward this close to the write side */
return sc_app_shutw(sc);
}
@ -662,7 +662,7 @@ static void sc_app_shutr_conn(struct stconn *sc)
if (sc->flags & SC_FL_ISBACK)
__sc_strm(sc)->conn_exp = TICK_ETERNITY;
}
else if (sc->flags & SC_FL_NOHALF) {
else if ((sc->flags & SC_FL_NOHALF) && channel_is_empty(ic)) {
/* we want to immediately forward this close to the write side */
return sc_app_shutw_conn(sc);
}
@ -890,7 +890,7 @@ static void sc_app_shutr_applet(struct stconn *sc)
if (sc->flags & SC_FL_ISBACK)
__sc_strm(sc)->conn_exp = TICK_ETERNITY;
}
else if (sc->flags & SC_FL_NOHALF) {
else if ((sc->flags & SC_FL_NOHALF) && channel_is_empty(ic)) {
/* we want to immediately forward this close to the write side */
return sc_app_shutw_applet(sc);
}
@ -1249,7 +1249,7 @@ static void sc_conn_read0(struct stconn *sc)
if (oc->flags & CF_SHUTW)
goto do_close;
if (sc->flags & SC_FL_NOHALF) {
if ((sc->flags & SC_FL_NOHALF) && channel_is_empty(ic)) {
/* we want to immediately forward this close to the write side */
/* force flag on ssl to keep stream in cache */
sc_conn_shutw(sc, CO_SHW_SILENT);