MINOR: uri_normalizer: Add a sort-query normalizer

This normalizer sorts the `&` delimited query parameters by parameter name.

See GitHub Issue #714.

doc/configuration.txt

@@ -6014,6 +6014,7 @@ http-request early-hint <name> <fmt> [ { if | unless } <condition> ]
 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 sort-query [ { if | unless } <condition> ]
 
   Performs normalization of the request's URI. The following normalizers are
   available:
@@ -6045,6 +6046,15 @@ http-request normalize-uri merge-slashes [ { if | unless } <condition> ]
       - //        -> /
       - /foo//bar -> /foo/bar
 
+  - sort-query: 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.
+
+      Example:
+      - /?c=3&a=1&b=2         -> /?a=1&b=2&c=3
+      - /?aaa=3&a=1&aa=2      -> /?a=1&aa=2&aaa=3
+      - /?a=3&b=4&a=1&b=5&a=2 -> /?a=3&a=1&a=2&b=4&b=5
+
 http-request redirect <rule> [ { if | unless } <condition> ]
 
   This performs an HTTP redirection based on a redirect rule. This is exactly

include/haproxy/action-t.h

@@ -105,6 +105,7 @@ enum act_normalize_uri {
 	ACT_NORMALIZE_URI_MERGE_SLASHES,
 	ACT_NORMALIZE_URI_DOTDOT,
 	ACT_NORMALIZE_URI_DOTDOT_FULL,
+	ACT_NORMALIZE_URI_SORT_QUERY,
 };
 
 /* NOTE: if <.action_ptr> is defined, the referenced function will always be

include/haproxy/uri_normalizer.h

@@ -20,6 +20,7 @@
 
 enum uri_normalizer_err uri_normalizer_path_dotdot(const struct ist path, int full, struct ist *dst);
 enum uri_normalizer_err uri_normalizer_path_merge_slashes(const struct ist path, struct ist *dst);
+enum uri_normalizer_err uri_normalizer_query_sort(const struct ist query, const char delim, struct ist *dst);
 
 #endif /* _HAPROXY_URI_NORMALIZER_H */
 

reg-tests/http-rules/normalize_uri.vtc

@@ -8,7 +8,7 @@ feature ignore_unknown_macro
 server s1 {
     rxreq
     txresp
-} -repeat 21 -start
+} -repeat 34 -start
 
 haproxy h1 -conf {
     defaults
@@ -46,6 +46,18 @@ haproxy h1 -conf {
 
         default_backend be
 
+    frontend fe_sort_query
+        bind "fd@${fe_sort_query}"
+
+        http-request set-var(txn.before) url
+        http-request normalize-uri sort-query
+        http-request set-var(txn.after) url
+
+        http-response add-header before  %[var(txn.before)]
+        http-response add-header after  %[var(txn.after)]
+
+        default_backend be
+
     backend be
         server s1 ${s1_addr}:${s1_port}
 
@@ -170,3 +182,70 @@ client c2 -connect ${h1_fe_dotdot_sock} {
     expect resp.http.after == "*"
     expect resp.http.after-full == "*"
 } -run
+
+client c3 -connect ${h1_fe_sort_query_sock} {
+    txreq -url "/?a=a"
+    rxresp
+    expect resp.http.before == "/?a=a"
+    expect resp.http.after == "/?a=a"
+
+    txreq -url "/?a=a&z=z"
+    rxresp
+    expect resp.http.before == "/?a=a&z=z"
+    expect resp.http.after == "/?a=a&z=z"
+
+    txreq -url "/?z=z&a=a"
+    rxresp
+    expect resp.http.before == "/?z=z&a=a"
+    expect resp.http.after == "/?a=a&z=z"
+
+    txreq -url "/?a=z&z=a"
+    rxresp
+    expect resp.http.before == "/?a=z&z=a"
+    expect resp.http.after == "/?a=z&z=a"
+
+    txreq -url "/?z=a&a=z"
+    rxresp
+    expect resp.http.before == "/?z=a&a=z"
+    expect resp.http.after == "/?a=z&z=a"
+
+    txreq -url "/?c&b&a&z&x&y"
+    rxresp
+    expect resp.http.before == "/?c&b&a&z&x&y"
+    expect resp.http.after == "/?a&b&c&x&y&z"
+
+    txreq -url "/?a=&aa=&aaa=&aaaa="
+    rxresp
+    expect resp.http.before == "/?a=&aa=&aaa=&aaaa="
+    expect resp.http.after == "/?a=&aa=&aaa=&aaaa="
+
+    txreq -url "/?aaaa=&a=&aa=&aaa="
+    rxresp
+    expect resp.http.before == "/?aaaa=&a=&aa=&aaa="
+    expect resp.http.after == "/?a=&aa=&aaa=&aaaa="
+
+    txreq -url "/?a=5&a=3&a=1&a=2&a=4"
+    rxresp
+    expect resp.http.before == "/?a=5&a=3&a=1&a=2&a=4"
+    expect resp.http.after == "/?a=5&a=3&a=1&a=2&a=4"
+
+    txreq -url "/?a=5&b=3&a=1&a=2&b=4"
+    rxresp
+    expect resp.http.before == "/?a=5&b=3&a=1&a=2&b=4"
+    expect resp.http.after == "/?a=5&a=1&a=2&b=3&b=4"
+
+    txreq -url "/"
+    rxresp
+    expect resp.http.before == "/"
+    expect resp.http.after == "/"
+
+    txreq -url "/?"
+    rxresp
+    expect resp.http.before == "/?"
+    expect resp.http.after == "/?"
+
+    txreq -req OPTIONS -url "*"
+    rxresp
+    expect resp.http.before == "*"
+    expect resp.http.after == "*"
+} -run

src/http_act.c

@@ -250,6 +250,23 @@ static enum act_return http_action_normalize_uri(struct act_rule *rule, struct p
 
 			break;
 		}
+		case ACT_NORMALIZE_URI_SORT_QUERY: {
+			const struct ist path = http_get_path(uri);
+			struct ist newquery = ist2(replace->area, replace->size);
+
+			if (!isttest(path))
+				goto leave;
+
+			err = uri_normalizer_query_sort(istfind(path, '?'), '&', &newquery);
+
+			if (err != URI_NORMALIZER_ERR_NONE)
+				break;
+
+			if (!http_replace_req_query(htx, newquery))
+				goto fail_rewrite;
+
+			break;
+		}
 	}
 
 	switch (err) {
@@ -330,6 +347,11 @@ static enum act_parse_ret parse_http_normalize_uri(const char **args, int *orig_
 			return ACT_RET_PRS_ERR;
 		}
 	}
+	else if (strcmp(args[cur_arg], "sort-query") == 0) {
+		cur_arg++;
+
+		rule->action = ACT_NORMALIZE_URI_SORT_QUERY;
+	}
 	else {
 		memprintf(err, "unknown normalizer '%s'", args[cur_arg]);
 		return ACT_RET_PRS_ERR;

src/uri_normalizer.c

@@ -13,6 +13,8 @@
 #include <import/ist.h>
 
 #include <haproxy/api.h>
+#include <haproxy/buf.h>
+#include <haproxy/chunk.h>
 #include <haproxy/uri_normalizer.h>
 
 /* Merges `/../` with preceding path segments.
@@ -140,6 +142,93 @@ enum uri_normalizer_err uri_normalizer_path_merge_slashes(const struct ist path,
 	return err;
 }
 
+/* Compares two query parameters by name. Query parameters are ordered
+ * as with memcmp. Shorter parameter names are ordered lower. Identical
+ * parameter names are compared by their pointer to maintain a stable
+ * sort.
+ */
+static int query_param_cmp(const void *a, const void *b)
+{
+	const struct ist param_a = *(struct ist*)a;
+	const struct ist param_b = *(struct ist*)b;
+	const struct ist param_a_name = iststop(param_a, '=');
+	const struct ist param_b_name = iststop(param_b, '=');
+
+	int cmp = istdiff(param_a_name, param_b_name);
+
+	if (cmp != 0)
+		return cmp;
+
+	/* The contents are identical: Compare the pointer. */
+	if (istptr(param_a) < istptr(param_b))
+		return -1;
+
+	if (istptr(param_a) > istptr(param_b))
+		return 1;
+
+	return 0;
+}
+
+/* Sorts the parameters within the given query string. */
+enum uri_normalizer_err uri_normalizer_query_sort(const struct ist query, const char delim, struct ist *dst)
+{
+	enum uri_normalizer_err err;
+
+	const size_t size = istclear(dst);
+	struct ist newquery = *dst;
+
+	struct ist scanner = query;
+
+	const struct buffer *trash = get_trash_chunk();
+	struct ist *params = (struct ist *)b_orig(trash);
+	const size_t max_param = b_size(trash) / sizeof(*params); 
+	size_t param_count = 0;
+
+	size_t i;
+
+	/* The query will have the same length. */
+	if (size < istlen(query)) {
+		err = URI_NORMALIZER_ERR_ALLOC;
+		goto fail;
+	}
+
+	/* Handle the leading '?'. */
+	newquery = __istappend(newquery, istshift(&scanner));
+
+	while (istlen(scanner) > 0) {
+		const struct ist param = istsplit(&scanner, delim);
+
+		if (param_count + 1 > max_param) {
+			err = URI_NORMALIZER_ERR_ALLOC;
+			goto fail;
+		}
+
+		params[param_count] = param;
+		param_count++;
+	}
+
+	qsort(params, param_count, sizeof(*params), query_param_cmp);
+
+	for (i = 0; i < param_count; i++) {
+		if (i > 0)
+			newquery = __istappend(newquery, '&');
+
+		if (istcat(&newquery, params[i], size) < 0) {
+			/* This is impossible, because we checked the size of the destination buffer. */
+			my_unreachable();
+			err = URI_NORMALIZER_ERR_INTERNAL_ERROR;
+			goto fail;
+		}
+	}
+
+	*dst = newquery;
+
+	return URI_NORMALIZER_ERR_NONE;
+
+  fail:
+
+	return err;
+}
 
 /*
  * Local variables: