Christopher,

On 4/17/21 9:16 AM, Christopher Faulet wrote:
I'm a bit annoyed by the renaming of existing normalizers. Except if you plan to do your modification before the release, it is a bad idea to change the name of  a configuration parameter once introduced in a stable release. The feature itself may be experimental because some bugs are expected but from the configuration point of view, it should be stable.

To be honest I did not expect that the second round of patches was already good enough to merge, so I wanted to send a quick notice regarding my further plans before it was too late and you already applied the patches.

I definitely planned to do the finalization before the release, but I understand that we are *very* late in the cycle, so it's understandable that you are a bit annoyed by my announcement.

Please find 3 additional patches (to be applied on top of the 8 of v2).

1. One of them performs the renaming into a naming scheme that I consider to be stable enough for future extensions.
2. Expands the documentation a little on best practices regarding use.
3. Marks the action as experimental in the documentation.

However, if you want to wait a bit to finish your work, I can push your patches in the next branch, pending for the next 2.5. This way, you'll have all the time to modify it. And because it is a standalone feature, we may plan to backport it to 2.4 if necessary.


With the renaming already made I consider the configuration syntax to be stable enough for a 2.4. I'll leave the final decision regarding that up to you, though. Especially since 2.4 is going to be an LTS.

Best regards
Tim Düsterhus
>From 02a0a18f3739dc9b0ed6297c76d6742f8f59c1eb Mon Sep 17 00:00:00 2001
From: Tim Duesterhus <[email protected]>
Date: Sat, 17 Apr 2021 11:21:10 +0200
Subject: [PATCH 1/3] MEDIUM: http_act: Rename uri-normalizers
To: [email protected]
Cc: [email protected]

This patch renames all existing uri-normalizers into a more consistent naming
scheme:

1. The part of the URI that is being touched.
2. The modification being performed as an explicit verb.
---
 doc/configuration.txt                  | 24 +++++++--------
 include/haproxy/action-t.h             | 12 ++++----
 reg-tests/http-rules/normalize_uri.vtc | 42 +++++++++++++-------------
 src/http_act.c                         | 36 +++++++++++-----------
 4 files changed, 57 insertions(+), 57 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 1e2f72b61..a9ed869d7 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6012,18 +6012,18 @@ http-request early-hint <name> <fmt> [ { if | unless } <condition> ]
   See RFC 8297 for more information.
 
 http-request normalize-uri <normalizer> [ { if | unless } <condition> ]
-http-request normalize-uri dotdot [ full ] [ { if | unless } <condition> ]
-http-request normalize-uri merge-slashes [ { if | unless } <condition> ]
-http-request normalize-uri percent-upper [ strict ] [ { if | unless } <condition> ]
-http-request normalize-uri sort-query [ { if | unless } <condition> ]
+http-request normalize-uri path-merge-slashes [ { if | unless } <condition> ]
+http-request normalize-uri path-strip-dotdot [ full ] [ { if | unless } <condition> ]
+http-request normalize-uri percent-to-uppercase [ strict ] [ { if | unless } <condition> ]
+http-request normalize-uri query-sort-by-name [ { if | unless } <condition> ]
 
   Performs normalization of the request's URI. The following normalizers are
   available:
 
-  - dotdot: Normalizes "/../" segments within the "path" component. This merges
-      segments that attempt to access the parent directory with their preceding
-      segment. Empty segments do not receive special treatment. Use the
-      "merge-slashes" normalizer first if this is undesired.
+  - path-strip-dotdot: Normalizes "/../" segments within the "path" component.
+      This merges segments that attempt to access the parent directory with
+      their preceding segment. Empty segments do not receive special treatment.
+      Use the "path-merge-slashes" normalizer first if this is undesired.
 
       Example:
       - /foo/../     -> /
@@ -6040,14 +6040,14 @@ http-request normalize-uri sort-query [ { if | unless } <condition> ]
       - /../bar/     -> /bar/
       - /bar/../../  -> /
 
-  - merge-slashes: Merges adjacent slashes within the "path" component into a
-      single slash.
+  - path-merge-slashes: Merges adjacent slashes within the "path" component
+      into a single slash.
 
       Example:
       - //        -> /
       - /foo//bar -> /foo/bar
 
-  - percent-upper: Uppercases letters within percent-encoded sequences
+  - percent-to-uppercase: Uppercases letters within percent-encoded sequences
       (RFC 3986#6.2.21).
 
       Example:
@@ -6060,7 +6060,7 @@ http-request normalize-uri sort-query [ { if | unless } <condition> ]
       Example:
       - /%zz -> HTTP 400
 
-  - sort-query: Sorts the query string parameters by parameter name.
+  - query-sort-by-name: Sorts the query string parameters by parameter name.
       Parameters are assumed to be delimited by '&'. Shorter names sort before
       longer names and identical parameter names maintain their relative order.
 
diff --git a/include/haproxy/action-t.h b/include/haproxy/action-t.h
index cce2a2e23..43e6b1add 100644
--- a/include/haproxy/action-t.h
+++ b/include/haproxy/action-t.h
@@ -102,12 +102,12 @@ enum act_timeout_name {
 };
 
 enum act_normalize_uri {
-	ACT_NORMALIZE_URI_MERGE_SLASHES,
-	ACT_NORMALIZE_URI_DOTDOT,
-	ACT_NORMALIZE_URI_DOTDOT_FULL,
-	ACT_NORMALIZE_URI_SORT_QUERY,
-	ACT_NORMALIZE_URI_PERCENT_UPPER,
-	ACT_NORMALIZE_URI_PERCENT_UPPER_STRICT,
+	ACT_NORMALIZE_URI_PATH_MERGE_SLASHES,
+	ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT,
+	ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT_FULL,
+	ACT_NORMALIZE_URI_QUERY_SORT_BY_NAME,
+	ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE,
+	ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE_STRICT,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be
diff --git a/reg-tests/http-rules/normalize_uri.vtc b/reg-tests/http-rules/normalize_uri.vtc
index e900677e9..b997b31b8 100644
--- a/reg-tests/http-rules/normalize_uri.vtc
+++ b/reg-tests/http-rules/normalize_uri.vtc
@@ -17,11 +17,11 @@ haproxy h1 -conf {
         timeout client  1s
         timeout server  1s
 
-    frontend fe_merge_slashes
-        bind "fd@${fe_merge_slashes}"
+    frontend fe_path_merge_slashes
+        bind "fd@${fe_path_merge_slashes}"
 
         http-request set-var(txn.before) url
-        http-request normalize-uri merge-slashes
+        http-request normalize-uri path-merge-slashes
         http-request set-var(txn.after) url
 
         http-response add-header before  %[var(txn.before)]
@@ -29,15 +29,15 @@ haproxy h1 -conf {
 
         default_backend be
 
-    frontend fe_dotdot
-        bind "fd@${fe_dotdot}"
+    frontend fe_path_strip_dotdot
+        bind "fd@${fe_path_strip_dotdot}"
 
         http-request set-var(txn.before) url
-        http-request normalize-uri dotdot
+        http-request normalize-uri path-strip-dotdot
         http-request set-var(txn.after) url
 
         http-request set-uri %[var(txn.before)]
-        http-request normalize-uri dotdot full
+        http-request normalize-uri path-strip-dotdot full
         http-request set-var(txn.after_full) url
 
         http-response add-header before  %[var(txn.before)]
@@ -46,11 +46,11 @@ haproxy h1 -conf {
 
         default_backend be
 
-    frontend fe_sort_query
-        bind "fd@${fe_sort_query}"
+    frontend fe_sort_query_by_name
+        bind "fd@${fe_sort_query_by_name}"
 
         http-request set-var(txn.before) url
-        http-request normalize-uri sort-query
+        http-request normalize-uri query-sort-by-name
         http-request set-var(txn.after) url
 
         http-response add-header before  %[var(txn.before)]
@@ -58,11 +58,11 @@ haproxy h1 -conf {
 
         default_backend be
 
-    frontend fe_percent_upper
-        bind "fd@${fe_percent_upper}"
+    frontend fe_percent_to_uppercase
+        bind "fd@${fe_percent_to_uppercase}"
 
         http-request set-var(txn.before) url
-        http-request normalize-uri percent-upper
+        http-request normalize-uri percent-to-uppercase
         http-request set-var(txn.after) url
 
         http-response add-header before  %[var(txn.before)]
@@ -70,11 +70,11 @@ haproxy h1 -conf {
 
         default_backend be
 
-    frontend fe_percent_upper_strict
-        bind "fd@${fe_percent_upper_strict}"
+    frontend fe_percent_to_uppercase_strict
+        bind "fd@${fe_percent_to_uppercase_strict}"
 
         http-request set-var(txn.before) url
-        http-request normalize-uri percent-upper strict
+        http-request normalize-uri percent-to-uppercase strict
         http-request set-var(txn.after) url
 
         http-response add-header before  %[var(txn.before)]
@@ -87,7 +87,7 @@ haproxy h1 -conf {
 
 } -start
 
-client c1 -connect ${h1_fe_merge_slashes_sock} {
+client c1 -connect ${h1_fe_path_merge_slashes_sock} {
     txreq -url "/foo/bar"
     rxresp
     expect resp.http.before == "/foo/bar"
@@ -139,7 +139,7 @@ client c1 -connect ${h1_fe_merge_slashes_sock} {
     expect resp.http.after == "*"
 } -run
 
-client c2 -connect ${h1_fe_dotdot_sock} {
+client c2 -connect ${h1_fe_path_strip_dotdot_sock} {
     txreq -url "/foo/bar"
     rxresp
     expect resp.http.before == "/foo/bar"
@@ -207,7 +207,7 @@ client c2 -connect ${h1_fe_dotdot_sock} {
     expect resp.http.after-full == "*"
 } -run
 
-client c3 -connect ${h1_fe_sort_query_sock} {
+client c3 -connect ${h1_fe_sort_query_by_name_sock} {
     txreq -url "/?a=a"
     rxresp
     expect resp.http.before == "/?a=a"
@@ -274,7 +274,7 @@ client c3 -connect ${h1_fe_sort_query_sock} {
     expect resp.http.after == "*"
 } -run
 
-client c4 -connect ${h1_fe_percent_upper_sock} {
+client c4 -connect ${h1_fe_percent_to_uppercase_sock} {
     txreq -url "/a?a=a"
     rxresp
     expect resp.http.before == "/a?a=a"
@@ -297,7 +297,7 @@ client c4 -connect ${h1_fe_percent_upper_sock} {
     expect resp.http.after == "*"
 } -run
 
-client c5 -connect ${h1_fe_percent_upper_strict_sock} {
+client c5 -connect ${h1_fe_percent_to_uppercase_strict_sock} {
     txreq -url "/a?a=a"
     rxresp
     expect resp.http.before == "/a?a=a"
diff --git a/src/http_act.c b/src/http_act.c
index 06ecb9e05..aeb940fd5 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -215,7 +215,7 @@ static enum act_return http_action_normalize_uri(struct act_rule *rule, struct p
 		goto fail_alloc;
 
 	switch ((enum act_normalize_uri) rule->action) {
-		case ACT_NORMALIZE_URI_MERGE_SLASHES: {
+		case ACT_NORMALIZE_URI_PATH_MERGE_SLASHES: {
 			const struct ist path = http_get_path(uri);
 			struct ist newpath = ist2(replace->area, replace->size);
 
@@ -232,15 +232,15 @@ static enum act_return http_action_normalize_uri(struct act_rule *rule, struct p
 
 			break;
 		}
-		case ACT_NORMALIZE_URI_DOTDOT:
-		case ACT_NORMALIZE_URI_DOTDOT_FULL: {
+		case ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT:
+		case ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT_FULL: {
 			const struct ist path = http_get_path(uri);
 			struct ist newpath = ist2(replace->area, replace->size);
 
 			if (!isttest(path))
 				goto leave;
 
-			err = uri_normalizer_path_dotdot(iststop(path, '?'), rule->action == ACT_NORMALIZE_URI_DOTDOT_FULL, &newpath);
+			err = uri_normalizer_path_dotdot(iststop(path, '?'), rule->action == ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT_FULL, &newpath);
 
 			if (err != URI_NORMALIZER_ERR_NONE)
 				break;
@@ -250,7 +250,7 @@ static enum act_return http_action_normalize_uri(struct act_rule *rule, struct p
 
 			break;
 		}
-		case ACT_NORMALIZE_URI_SORT_QUERY: {
+		case ACT_NORMALIZE_URI_QUERY_SORT_BY_NAME: {
 			const struct ist path = http_get_path(uri);
 			struct ist newquery = ist2(replace->area, replace->size);
 
@@ -267,15 +267,15 @@ static enum act_return http_action_normalize_uri(struct act_rule *rule, struct p
 
 			break;
 		}
-		case ACT_NORMALIZE_URI_PERCENT_UPPER:
-		case ACT_NORMALIZE_URI_PERCENT_UPPER_STRICT: {
+		case ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE:
+		case ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE_STRICT: {
 			const struct ist path = http_get_path(uri);
 			struct ist newpath = ist2(replace->area, replace->size);
 
 			if (!isttest(path))
 				goto leave;
 
-			err = uri_normalizer_percent_upper(path, rule->action == ACT_NORMALIZE_URI_PERCENT_UPPER_STRICT, &newpath);
+			err = uri_normalizer_percent_upper(path, rule->action == ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE_STRICT, &newpath);
 
 			if (err != URI_NORMALIZER_ERR_NONE)
 				break;
@@ -345,40 +345,40 @@ static enum act_parse_ret parse_http_normalize_uri(const char **args, int *orig_
 		return ACT_RET_PRS_ERR;
 	}
 
-	if (strcmp(args[cur_arg], "merge-slashes") == 0) {
+	if (strcmp(args[cur_arg], "path-merge-slashes") == 0) {
 		cur_arg++;
 
-		rule->action = ACT_NORMALIZE_URI_MERGE_SLASHES;
+		rule->action = ACT_NORMALIZE_URI_PATH_MERGE_SLASHES;
 	}
-	else if (strcmp(args[cur_arg], "dotdot") == 0) {
+	else if (strcmp(args[cur_arg], "path-strip-dotdot") == 0) {
 		cur_arg++;
 
 		if (strcmp(args[cur_arg], "full") == 0) {
 			cur_arg++;
-			rule->action = ACT_NORMALIZE_URI_DOTDOT_FULL;
+			rule->action = ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT_FULL;
 		}
 		else if (!*args[cur_arg]) {
-			rule->action = ACT_NORMALIZE_URI_DOTDOT;
+			rule->action = ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT;
 		}
 		else if (strcmp(args[cur_arg], "if") != 0 && strcmp(args[cur_arg], "unless") != 0) {
 			memprintf(err, "unknown argument '%s' for 'dotdot' normalizer", args[cur_arg]);
 			return ACT_RET_PRS_ERR;
 		}
 	}
-	else if (strcmp(args[cur_arg], "sort-query") == 0) {
+	else if (strcmp(args[cur_arg], "query-sort-by-name") == 0) {
 		cur_arg++;
 
-		rule->action = ACT_NORMALIZE_URI_SORT_QUERY;
+		rule->action = ACT_NORMALIZE_URI_QUERY_SORT_BY_NAME;
 	}
-	else if (strcmp(args[cur_arg], "percent-upper") == 0) {
+	else if (strcmp(args[cur_arg], "percent-to-uppercase") == 0) {
 		cur_arg++;
 
 		if (strcmp(args[cur_arg], "strict") == 0) {
 			cur_arg++;
-			rule->action = ACT_NORMALIZE_URI_PERCENT_UPPER_STRICT;
+			rule->action = ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE_STRICT;
 		}
 		else if (!*args[cur_arg]) {
-			rule->action = ACT_NORMALIZE_URI_PERCENT_UPPER;
+			rule->action = ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE;
 		}
 		else if (strcmp(args[cur_arg], "if") != 0 && strcmp(args[cur_arg], "unless") != 0) {
 			memprintf(err, "unknown argument '%s' for 'percent-upper' normalizer", args[cur_arg]);
-- 
2.31.1

>From 77a2187ffbe3b6ec58ef4e44c06d62af38a54600 Mon Sep 17 00:00:00 2001
From: Tim Duesterhus <[email protected]>
Date: Fri, 16 Apr 2021 23:52:29 +0200
Subject: [PATCH 2/3] DOC: Add introduction to http-request normalize-uri
To: [email protected]
Cc: [email protected]

This patch adds an introduction to the http-request normalize-uri section,
explaining what to expect from the normalizers and possible issues that might
arise when not being careful.
---
 doc/configuration.txt | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index a9ed869d7..818c22088 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6017,8 +6017,18 @@ http-request normalize-uri path-strip-dotdot [ full ] [ { if | unless } <conditi
 http-request normalize-uri percent-to-uppercase [ strict ] [ { if | unless } <condition> ]
 http-request normalize-uri query-sort-by-name [ { if | unless } <condition> ]
 
-  Performs normalization of the request's URI. The following normalizers are
-  available:
+  Performs normalization of the request's URI.
+
+  Each normalizer handles a single type of normalization to allow for a
+  fine-grained selection of the level of normalization that is appropriate for
+  the supported backend.
+
+  As an example the "path-strip-dotdot" normalizer might be useful for a static
+  fileserver that directly maps the requested URI to the path within the local
+  filesystem. However it might break routing of an API that expects a specific
+  number of segments in the path.
+
+  The following normalizers are available:
 
   - path-strip-dotdot: Normalizes "/../" segments within the "path" component.
       This merges segments that attempt to access the parent directory with
-- 
2.31.1

>From 8303556063ea1d30c9b29268cb8973630f80d553 Mon Sep 17 00:00:00 2001
From: Tim Duesterhus <[email protected]>
Date: Sat, 17 Apr 2021 00:24:56 +0200
Subject: [PATCH 3/3] DOC: Note that URI normalization is experimental
To: [email protected]
Cc: [email protected]

Add a paragraph to the URI normalization documentation that URI normalization
is currently considered to be experimental.
---
 doc/configuration.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 818c22088..97d0ec8c3 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6019,6 +6019,11 @@ http-request normalize-uri query-sort-by-name [ { if | unless } <condition> ]
 
   Performs normalization of the request's URI.
 
+  URI normalization in HAProxy 2.4 is currently available as an experimental
+  technical preview. You should be prepared that the behavior of normalizers
+  might change to fix possible issues, possibly breaking proper request
+  processing in your infrastructure.
+
   Each normalizer handles a single type of normalization to allow for a
   fine-grained selection of the level of normalization that is appropriate for
   the supported backend.
-- 
2.31.1

Reply via email to