From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 18 Feb 2020 14:34:58 +0000 (+0100)
Subject: MINOR: http-ana: Match on the path if the monitor-uri starts by a /
X-Git-Tag: v2.2-dev3~50
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6072beb214d04ee2344872e100f4fdc480dd2496;p=thirdparty%2Fhaproxy.git

MINOR: http-ana: Match on the path if the monitor-uri starts by a /

if the monitor-uri starts by a slash ('/'), the matching is performed against
the request's path instead of the request's uri. It is a workaround to let the
HTTP/2 requests match the monitor-uri. Indeed, in HTTP/2, clients are encouraged
to send absolute URIs only.

This patch is not tagged as a bug, because the previous behavior matched exactly
what the doc describes. But it may surprise that HTTP/2 requests don't match the
monitor-uri.

This patch may be backported to 2.1 because URIs of HTTP/2 are stored using the
absolute-form starting this version. For previous versions, this patch will only
helps explicitely absolute HTTP/1 requests (and only the HTX part because on the
legacy HTTP, all the URI is matched).

It should fix the issue #509.
---

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 6b8437b19e..18ac2c8576 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6375,6 +6375,11 @@ monitor-uri <uri>
   "monitor fail" and ACLs so that the result can be adjusted to whatever check
   can be imagined (most often the number of available servers in a backend).
 
+  Note: if <uri> starts by a slash ('/'), the matching is performed against the
+        request's path instead of the request's uri. It is a workaround to let
+        the HTTP/2 requests match the monitor-uri. Indeed, in HTTP/2, clients
+        are encouraged to send absolute URIs only.
+
   Example :
     # Use /haproxy_test to report haproxy's status
     frontend www
diff --git a/reg-tests/checks/http-monitor-uri.vtc b/reg-tests/checks/http-monitor-uri.vtc
new file mode 100644
index 0000000000..c938abc8c4
--- /dev/null
+++ b/reg-tests/checks/http-monitor-uri.vtc
@@ -0,0 +1,56 @@
+varnishtest "Test the HTTP directive monitor-uri"
+#REQUIRE_VERSION=2.2
+
+# This config tests the HTTP directive monitor-uri. Especially the path matching
+# when an absolute-form uri is received from the client. But also the
+# case-sensitivity of the matching.
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+    defaults
+        mode http
+        timeout connect 1s
+        timeout client  1s
+        timeout server  1s
+
+    frontend fe1
+        bind "fd@${fe1}"
+        monitor-uri /health
+
+    frontend fe2
+        bind "fd@${fe2}"
+        monitor-uri http://www.haproxy.org/health
+} -start
+
+client c1  -connect ${h1_fe1_sock} {
+    txreq -req GET -url /health
+    rxresp
+    expect resp.status == 200
+} -run
+
+client c2  -connect ${h1_fe1_sock} {
+    txreq -req GET -url http://www.haproxy.org/health \
+          -hdr "Host: www.haproxy.org"
+    rxresp
+    expect resp.status == 200
+} -run
+
+client c3  -connect ${h1_fe1_sock} {
+    txreq -req GET -url /hEAlth
+    rxresp
+    expect resp.status == 503
+} -run
+
+client c4  -connect ${h1_fe2_sock} {
+    txreq -req GET -url http://www.haproxy.org/health \
+          -hdr "Host: www.haproxy.org"
+    rxresp
+    expect resp.status == 200
+} -run
+
+client c5  -connect ${h1_fe2_sock} {
+    txreq -req GET -url /health
+    rxresp
+    expect resp.status == 503
+} -run
diff --git a/src/http_ana.c b/src/http_ana.c
index 57cb73f2ae..d1b6e88724 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -319,12 +319,17 @@ int http_wait_for_request(struct stream *s, struct channel *req, int an_bit)
 	}
 
 	/*
-	 * 2: check if the URI matches the monitor_uri.
-	 * We have to do this for every request which gets in, because
-	 * the monitor-uri is defined by the frontend.
+	 * 2: check if the URI matches the monitor_uri.  We have to do this for
+	 * every request which gets in, because the monitor-uri is defined by
+	 * the frontend. If the monitor-uri starts with a '/', the matching is
+	 * done against the request's path. Otherwise, the request's uri is
+	 * used. It is a workaround to let HTTP/2 health-checks work as
+	 * expected.
 	 */
 	if (unlikely((sess->fe->monitor_uri_len != 0) &&
-		     isteq(htx_sl_req_uri(sl), ist2(sess->fe->monitor_uri, sess->fe->monitor_uri_len)))) {
+		     ((*sess->fe->monitor_uri == '/' && isteq(http_get_path(htx_sl_req_uri(sl)),
+							      ist2(sess->fe->monitor_uri, sess->fe->monitor_uri_len))) ||
+		      isteq(htx_sl_req_uri(sl), ist2(sess->fe->monitor_uri, sess->fe->monitor_uri_len))))) {
 		/*
 		 * We have found the monitor URI
 		 */