From 9b18688ce2f19ab95bd2dce2e271dcf4e9efb7fe Mon Sep 17 00:00:00 2001
From: Alexander Schwartz <aschwart@redhat.com>
Date: Fri, 7 Jan 2022 09:38:51 +0100
Subject: [PATCH] fixes #9427 regex pattern is now pre-compiled

---
 .../storage/hotRod/IckleQueryOperators.java   | 15 ++++---
 .../hotRod/IckleQueryOperatorsTest.java       | 42 +++++++++++++++++++
 2 files changed, 52 insertions(+), 5 deletions(-)
 create mode 100644 model/map-hot-rod/src/test/java/org/keycloak/models/map/storage/hotRod/IckleQueryOperatorsTest.java

diff --git a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryOperators.java b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryOperators.java
index cc745979c5..abae9212d3 100644
--- a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryOperators.java
+++ b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/IckleQueryOperators.java
@@ -26,6 +26,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
@@ -35,10 +36,10 @@ import java.util.stream.Stream;
  * <p/>
  * For example,
  * <p/>
- * for operator {@link ModelCriteriaBuilder.Operator.EQ} we concatenate left operand and right operand with equal sign:
+ * for operator {@link ModelCriteriaBuilder.Operator#EQ} we concatenate left operand and right operand with equal sign:
  * {@code fieldName = :parameterName}
  * <p/>
- * however, for operator {@link ModelCriteriaBuilder.Operator.EXISTS} we add following:
+ * however, for operator {@link ModelCriteriaBuilder.Operator#EXISTS} we add following:
  * <p/>
  * {@code fieldName IS NOT NULL AND fieldName IS NOT EMPTY"}.
  *
@@ -46,7 +47,7 @@ import java.util.stream.Stream;
  * corresponding value is then saved into {@code Map<String, Object>} that is passed to each {@link ExpressionCombinator}.
  */
 public class IckleQueryOperators {
-    private static final String UNWANTED_CHARACTERS_REGEX = "[^a-zA-Z\\d]";
+    private static final Pattern UNWANTED_CHARACTERS_REGEX = Pattern.compile("[^a-zA-Z\\d]");
     public static final String C = "c";
     private static final Map<ModelCriteriaBuilder.Operator, String> OPERATOR_TO_STRING = new HashMap<>();
     private static final Map<ModelCriteriaBuilder.Operator, ExpressionCombinator> OPERATOR_TO_EXPRESSION_COMBINATORS = new HashMap<>();
@@ -123,14 +124,18 @@ public class IckleQueryOperators {
     }
 
     private static String removeForbiddenCharactersFromNamedParameter(String name) {
-        return name.replaceAll(UNWANTED_CHARACTERS_REGEX, "");
+        return UNWANTED_CHARACTERS_REGEX.matcher(name).replaceAll( "");
     }
 
     /**
      * Maps {@code namePrefix} to next available parameter name. For example, if {@code namePrefix == "id"}
      * and {@code existingNames} set already contains {@code id0} and {@code id1} it returns {@code id2}.
+     * Any character that is not an alphanumeric will be stripped, so that it works for prefixes like
+     * {@code "attributes.name"} as well.
      *
-     * This method is used for computing available names for name query parameters
+     * This method is used for computing available names for name query parameters.
+     * Instead of creating generic named parameters that would be hard to debug and read by humans, it creates readable
+     * named parameters from the prefix.
      *
      * @param existingNames set of parameter names that are already used in this Ickle query
      * @param namePrefix name of the parameter
diff --git a/model/map-hot-rod/src/test/java/org/keycloak/models/map/storage/hotRod/IckleQueryOperatorsTest.java b/model/map-hot-rod/src/test/java/org/keycloak/models/map/storage/hotRod/IckleQueryOperatorsTest.java
new file mode 100644
index 0000000000..f690209a9f
--- /dev/null
+++ b/model/map-hot-rod/src/test/java/org/keycloak/models/map/storage/hotRod/IckleQueryOperatorsTest.java
@@ -0,0 +1,42 @@
+package org.keycloak.models.map.storage.hotRod;
+
+import org.hamcrest.Matchers;
+import org.junit.Test;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class IckleQueryOperatorsTest {
+
+    @Test
+    public void testFindAvailableNamedParamSimple() {
+        Set<String> existingNames = new HashSet<>();
+
+        String param = IckleQueryOperators.findAvailableNamedParam(existingNames, "clientId");
+
+        assertThat("should create the first ID", param, Matchers.equalTo("clientId0"));
+    }
+
+    @Test
+    public void testFindAvailableNamedParamAlreadyExists() {
+        Set<String> existingNames = new HashSet<>();
+        existingNames.add("clientId0");
+
+        String param = IckleQueryOperators.findAvailableNamedParam(existingNames, "clientId");
+
+        assertThat("should create the next ID as clientId0 is already taken", param, Matchers.equalTo("clientId1"));
+    }
+
+    @Test
+    public void testFindAvailableNamedParamIllegalCharacterInPrefix() {
+        Set<String> existingNames = new HashSet<>();
+        existingNames.add("clientid0");
+
+        String param = IckleQueryOperators.findAvailableNamedParam(existingNames, "client.id");
+
+        assertThat("should remove non-characters and non-numbers from the ID", param, Matchers.equalTo("clientid1"));
+    }
+
+}
\ No newline at end of file