BUG/MINOR: peers: don't lose the peer's version when the hello is fragmented

The peers applet parses the protocol version from the first line of the
hello message in the GETVERSION state. However, maj_ver and min_ver are
local variables which are reset on each pass through the handler's state
machine.

If the host or peer line is not available yet, the handler returns and
resumes later in GETHOST or GETPEER, with the version lost. As a result,
PEER_F_DWNGRD is not updated and retains its previous value. A peer
announcing protocol version 2.0 may then receive timed updates it does not
support, while a 2.1 peer may receive updates without expiration.

Preserve the minor version in appctx->st1 until the peer is identified,
then use it to set or clear PEER_F_DWNGRD and reset st1. This field is
otherwise only used for error status codes, which error paths overwrite
before entering EXIT.

Since GETPEER can only be reached after a valid version has been parsed,
there is no need for an unknown-version sentinel. Remove the now unused
initialization of maj_ver and min_ver.

This has been there since 1.7 with commit 523cc9e85 ("MEDIUM: peers: Fix a
peer stick-tables synchronization issue."). This fix should be backported
to all stable versions.

diff --git a/src/peers.c b/src/peers.c
index b13c8ca7f1755a83b3701f5d75c104eb7c2804ee..7db63eeba64e5a52de52dabee1df6a85cc1d28e2 100644 (file)
--- a/src/peers.c
+++ b/src/peers.c
@@ -3091,7 +3091,6 @@ void peer_io_handler(struct appctx *appctx)
        while (1) {
                prev_state = appctx->st0;
 switchstate:
-               maj_ver = min_ver = (unsigned int)-1;
                switch(appctx->st0) {
                        case PEER_SESS_ST_ACCEPT:
                                prev_state = appctx->st0;
@@ -3108,6 +3107,10 @@ switchstate:
                                        goto switchstate;
                                }
 
+                               /* st1 holds the minor version in GETHOST/GETPEER,
+                                * or the status code in EXIT.
+                                */
+                               appctx->st1 = min_ver;
                                appctx->st0 = PEER_SESS_ST_GETHOST;
                                __fallthrough;
                        case PEER_SESS_ST_GETHOST:
@@ -3153,14 +3156,11 @@ switchstate:
                                        curpeer->heartbeat = TICK_ETERNITY;
                                        curpeer->coll++;
                                }
-                               if (maj_ver != (unsigned int)-1 && min_ver != (unsigned int)-1) {
-                                       if (min_ver == PEER_DWNGRD_MINOR_VER) {
-                                               curpeer->flags |= PEER_F_DWNGRD;
-                                       }
-                                       else {
-                                               curpeer->flags &= ~PEER_F_DWNGRD;
-                                       }
-                               }
+                               if (appctx->st1 == PEER_DWNGRD_MINOR_VER)
+                                       curpeer->flags |= PEER_F_DWNGRD;
+                               else
+                                       curpeer->flags &= ~PEER_F_DWNGRD;
+                               appctx->st1 = 0;
                                curpeer->appctx = appctx;
                                curpeer->flags |= PEER_F_ALIVE;
                                appctx->svcctx = curpeer;