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

BUG/MEDIUM: connection: make cs_shutr/cs_shutw//cs_close() idempotent 

In 1.8 when muxes and conn_streams were introduced, the call to
conn_full_close() was replaced with a call to cs_close() which only
relied on shutr/shutw (commits 6978db35e ("MINOR: connection:
add cs_close() to close a conn_stream") and a553ae96f ("MEDIUM:
connection: replace conn_full_close() with cs_close()")). By then
this was fine, and the rare risk of non-idempotent calls was addressed
by the muxes implementing the functions (e.g. mux_pt).

Later with commit 325607397 ("MEDIUM: stream: do not forcefully close
the client connection anymore"), stream_free() started to call cs_close()
instead of forcibly closing the connection via conn_full_close(). At this
point this started to break idempotence because it was possible to emit
a shutw() (e.g. when option httpclose was set), then to have it called
agian upon stream_free() via cs_close(). By then it was not a problem
since only mux_pt would implement this and did check for idempotence.

When HTX was implemented and mux-h1/h2 offered support for shutw/shutr,
the idempotence changed a little bit because the last shutdown mode
(normal/silent) was recorded and used at the moment of closing. The
call to cs_close() uses the silent mode and will replace the current
one. This has an effect on data pending in the buffer if the FIN could
not be sent before cs_close(), because lingering may be disabled and
final data lost in the network stack.

Interestingly, during 2.4-dev3, this was addressed as the side effect
of an improvement by commit 3c82d8b32 ("MINOR: mux-h1: Rework how
shutdowns are handled"), where the H1 mux's shutdown function becomes
explicitly idempotent. However older versions (2.3 to 2.0) do not have
it.

This patch addresses the issue globally by making sure that cs_shutr()
and cs_shutw() are idempotent and cannot have their data truncated by
a late cs_close(). It fixes the truncation that is observed in 2.3 and
2.2 as described in issue #1450.

This must be backported as far as 2.0, along with commit f14d750bf
("BUG/MEDIUM: conn-stream: Don't reset CS flags on close") which it
depends on.
This commit is contained in:
Willy Tarreau 2021-11-14 13:42:17 +01:00
parent fdf53b4962
commit 8c13a92da0
1 changed files with 4 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/haproxy/connection.h
View File

@ -249,6 +249,8 @@ static inline void conn_xprt_shutw_hard(struct connection *c)
/* shut read */
static inline void cs_shutr(struct conn_stream *cs, enum cs_shr_mode mode)
{
if (cs->flags & CS_FL_SHR)
return;
/* clean data-layer shutdown */
if (cs->conn->mux && cs->conn->mux->shutr)
@ -259,6 +261,8 @@ static inline void cs_shutr(struct conn_stream *cs, enum cs_shr_mode mode)
/* shut write */
static inline void cs_shutw(struct conn_stream *cs, enum cs_shw_mode mode)
{
if (cs->flags & CS_FL_SHW)
return;
/* clean data-layer shutdown */
if (cs->conn->mux && cs->conn->mux->shutw)