Improve performance of query filters (especially on large bodies)

Fixes: #1838
zalando · May 27, 2024 · c2a2a29 · c2a2a29
1 parent 186a5ac
commit c2a2a29
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 42 deletions.
diff --git a/logbook-core/src/main/java/org/zalando/logbook/core/QueryFilters.java b/logbook-core/src/main/java/org/zalando/logbook/core/QueryFilters.java
@@ -3,12 +3,13 @@
 import org.apiguardian.api.API;
 import org.zalando.logbook.QueryFilter;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.StringTokenizer;
+import java.util.function.BiFunction;
 import java.util.function.Predicate;
 import java.util.function.UnaryOperator;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
-import static java.util.regex.Pattern.compile;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 import static org.apiguardian.api.API.Status.MAINTAINED;
 import static org.apiguardian.api.API.Status.STABLE;
@@ -45,7 +46,6 @@ public static QueryFilter replaceQuery(
         return replaceQuery(name::equals, replacementFunction);
     }
 
-
     @API(status = EXPERIMENTAL)
     public static QueryFilter replaceQuery(
             final Predicate<String> predicate, final String replacement) {
@@ -57,55 +57,54 @@ public static QueryFilter replaceQuery(
     public static QueryFilter replaceQuery(
             final Predicate<String> predicate, final UnaryOperator<String> replacementFunction) {
 
-        final Pattern pattern = compile("(?<name>[^&]*?)=(?<value>[^&]*)");
-
-        return query -> {
-            final Matcher matcher = pattern.matcher(query);
-            final StringBuffer result = new StringBuffer(query.length());
+        return query -> processParsedQueryParams(query, (String paramName, String paramValue) -> {
+            if (paramValue == null) {
+                return paramName;
+            } else {
+                String newValue = predicate.test(paramName) ? replacementFunction.apply(paramValue) : paramValue;
 
-            while (matcher.find()) {
-                if (predicate.test(matcher.group("name"))) {
-                    matcher.appendReplacement(result, "${name}");
-                    result.append('=');
-                    result.append(replacementFunction.apply(matcher.group("value")));
-                } else {
-                    matcher.appendReplacement(result, "$0");
-                }
+                return paramName + "=" + newValue;
             }
-            matcher.appendTail(result);
-
-            return result.toString();
-        };
+        });
     }
 
     @API(status = EXPERIMENTAL)
     public static QueryFilter removeQuery(final String name) {
         final Predicate<String> predicate = name::equals;
-        final Pattern pattern = compile("&?(?<name>[^&]+?)=(?:[^&]*)");
-
-        return query -> {
-            final Matcher matcher = pattern.matcher(query);
-            final StringBuffer result = new StringBuffer(query.length());
-
-            while (matcher.find()) {
-                if (predicate.test(matcher.group("name"))) {
-                    matcher.appendReplacement(result, "");
-                } else {
-                    matcher.appendReplacement(result, "$0");
-                }
+
+        return query -> processParsedQueryParams(query, (String paramName, String paramValue) -> {
+            if (predicate.test(paramName)) {
+                return null; // indicate removal
+            } else {
+                return paramName + "=" + paramValue;
             }
-            matcher.appendTail(result);
+        });
+    }
 
-            final String output = result.toString();
+    private static String processParsedQueryParams(String query, BiFunction<String, String, String> nameValueHandler) {
+        final List<String> result = new ArrayList<>();
+
+        // see https://url.spec.whatwg.org/#urlencoded-parsing
+        StringTokenizer tokenizer = new StringTokenizer(query, "&");
+        while (tokenizer.hasMoreTokens()) {
+            String token = tokenizer.nextToken();
+            int equalsIndex = token.indexOf('=');
+
+            // a token does not always contain an '=', if not the token is the name
+            String newParam;
+            if (equalsIndex == -1) {
+                newParam = nameValueHandler.apply(token, null);
+            } else {
+                String name = token.substring(0, equalsIndex);
+                String value = token.substring(equalsIndex + 1);
+                newParam = nameValueHandler.apply(name, value);
+            }
 
-            if (output.startsWith("&")) {
-                // ideally this case would be covered by the regex, but
-                // I wasn't able to make it work
-                return output.substring(1);
+            if (newParam != null) {
+                result.add(newParam);
             }
+        }
 
-            return output;
-        };
+        return String.join("&", result);
     }
-
 }
diff --git a/logbook-core/src/test/java/org/zalando/logbook/core/QueryFiltersTest.java b/logbook-core/src/test/java/org/zalando/logbook/core/QueryFiltersTest.java
@@ -37,6 +37,9 @@ void shouldFilterQueryParameterWithDynamicReplacing() {
             "active&name=Alice,active&name=XXX",
             "name=Alice&active&age=5,name=XXX&active&age=5",
             "name=Alice&secret=123,name=XXX&secret=XXX",
+            "secret&name,secret&name",
+            "=secret&=name,=secret&=name",
+            "secret=&name=,secret=XXX&name=XXX",
     })
     @ParameterizedTest
     void shouldReplaceMatchingQueryParameters(
@@ -57,6 +60,7 @@ void shouldReplaceMatchingQueryParameters(
             "sort=price&q=boots&direction=asc,sort=price&direction=asc",
             "sort=price&direction=asc,sort=price&direction=asc",
             "q=boots&test=true&q=boots,test=true",
+            "q&q=1&q=2&q=3&test=true&q=4&q=5,'test=true'",
             "q=1&q=2&q=3,''",
             "'',''"
     })