From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Tue, 5 Dec 2023 14:58:49 +0000 (+0100)
Subject: MEDIUM: proxy: set PR_O_HTTP_UPG on implicit upgrades
X-Git-Tag: v3.0-dev1~70
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=8a6cc6e3eaee941312e9be02970ca11d2516e74e;p=thirdparty%2Fhaproxy.git

MEDIUM: proxy: set PR_O_HTTP_UPG on implicit upgrades

When a TCP frontend uses an HTTP backend, the stream is automatically
upgraded and it results in a similar behavior as if a switch-mode http
rule was evaluated since stream_set_http_mode() gets called in both
situations and minimal HTTP analyzers are set.

In the current implementation, some postparsing checks are generating
errors or warnings when the frontend is in TCP mode with some HTTP options
set and no upgrade is expected (no switch-rule http). But as you can guess,
unfortunately this leads in issues when such "HTTP" only options are used
in a frontend that has implicit switching rules (that is, when the
frontend uses an HTTP backend for example), because in this case the
PR_O_HTTP_UPG will not be set, so the postparsing checks will consider
that some options are not relevant and will raise some warnings.

Consider the following example:

  backend back
    mode http
    server s1 git.haproxy.org:80
  frontend front
    mode tcp
    bind localhost:8080
    http-request set-var(txn.test) str(TRUE),debug(WORKING,stderr)
    use_backend back

By starting an haproxy instance with the above example conf, we end up
having this warning:

  [WARNING]  (400280) : config : 'http-request' rules ignored for frontend 'front' as they require HTTP mode.

However, by making a request on the frontend, we notice that the request
rules are still executed, and that's because the stream is effectively
upgraded as a result of an implicit upgrade:

  [debug] WORKING: type=str <TRUE>

So this confirms the previous description: since implicit and explicit
upgrades result in approximately the same behavior on the frontend side,
we should consider them both when doing postparsing checks.

This is what we try to address in the following commit: PR_O_HTTP_UPG
flag is now more generic in the sense that it refers to either implicit
(through default_backend or use_backend rules) or explicit (switch-mode
rules) upgrades. Indeed, everytime an HTTP or dynamic backend (where the
mode cannot be assumed during parsing) is encountered in default_backend
directive or use_backend rules, we explicitly position the upgrade flag
so that further checks that depend on the proxy being in HTTP context
don't report false warnings.
---

diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h
index 2f7bf7bbea..67a45bf941 100644
--- a/include/haproxy/proxy-t.h
+++ b/include/haproxy/proxy-t.h
@@ -92,7 +92,7 @@ enum PR_SRV_STATE_FILE {
 #define PR_O_IGNORE_PRB 0x00000200      /* ignore empty requests (aborts and timeouts) */
 #define PR_O_NULLNOLOG  0x00000400      /* a connect without request will not be logged */
 #define PR_O_WREQ_BODY  0x00000800      /* always wait for the HTTP request body */
-#define PR_O_HTTP_UPG   0x00001000      /* Contain a "switch-mode http" tcp-request rule */
+#define PR_O_HTTP_UPG   0x00001000      /* implicit (default/use backend) or explicit (switch-mode) http upgrade */
 /* unused: 0x00002000 */
 #define PR_O_PERSIST    0x00004000      /* server persistence stays effective even when server is down */
 #define PR_O_LOGASAP    0x00008000      /* log as soon as possible, without waiting for the stream to complete */
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 2744f97b7e..f2fb4676ba 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -3120,6 +3120,12 @@ init_proxies_list_stage1:
 						   curproxy->id);
 					err_code |= ERR_WARN;
 				}
+				if (target->mode == PR_MODE_HTTP) {
+					/* at least one of the used backends will provoke an
+					 * HTTP upgrade
+					 */
+					curproxy->options |= PR_O_HTTP_UPG;
+				}
 			}
 		}
 
@@ -3153,6 +3159,11 @@ init_proxies_list_stage1:
 				if (node->type != LOG_FMT_TEXT || node->list.n != &rule->be.expr) {
 					rule->dynamic = 1;
 					free(pxname);
+					/* backend is not yet known so we cannot assume its type,
+					 * thus we should consider that at least one of the used
+					 * backends may provoke HTTP upgrade
+					 */
+					curproxy->options |= PR_O_HTTP_UPG;
 					continue;
 				}
 				/* Only one element in the list, a simple string: free the expression and
@@ -3187,6 +3198,12 @@ init_proxies_list_stage1:
 			} else {
 				ha_free(&rule->be.name);
 				rule->be.backend = target;
+				if (target->mode == PR_MODE_HTTP) {
+					/* at least one of the used backends will provoke an
+					 * HTTP upgrade
+					 */
+					curproxy->options |= PR_O_HTTP_UPG;
+				}
 			}
 			err_code |= warnif_tcp_http_cond(curproxy, rule->cond);
 		}