From fde0074fee77fd2b757df58d894f09d8317494ff Mon Sep 17 00:00:00 2001 From: Auke van Leeuwen Date: Thu, 16 May 2024 10:15:24 +0200 Subject: [PATCH] Improve performance of query filters (especially on large bodies) Fixes: #1838 --- .../zalando/logbook/core/QueryFilters.java | 83 +++++++++---------- .../logbook/core/QueryFiltersTest.java | 4 + 2 files changed, 45 insertions(+), 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 index 5c39b5efe..71c2df01f 100644 --- 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 predicate, final String replacement) { @@ -57,55 +57,54 @@ public static QueryFilter replaceQuery( public static QueryFilter replaceQuery( final Predicate predicate, final UnaryOperator replacementFunction) { - final Pattern pattern = compile("(?[^&]*?)=(?[^&]*)"); - - 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 predicate = name::equals; - final Pattern pattern = compile("&?(?[^&]+?)=(?:[^&]*)"); - - 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 nameValueHandler) { + final List 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 index a913346cd..9d4da683d 100644 --- 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,''", "'',''" })