You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by js...@apache.org on 2026/09/25 17:04:21 UTC
(unomi) 01/01: Simplify identifier validation patterns to a single character class (unomi-3.0.x)
This is an automated email from the ASF dual-hosted git repository.
jsinovassin pushed a commit to branch backport/simplify-id-patterns-3.0.x
in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 2e448a52bd23da7d4cbc3d51d226cf31bbe8191a
Author: jsinovassin <js...@jahia.com>
AuthorDate: Fri Sep 25 18:58:14 2026 +0200
Simplify identifier validation patterns to a single character class (unomi-3.0.x)
Backport of #873 to unomi-3.0.x, re-cut from the current branch head.
The identifier pattern `^(\w|[-_@\.]){0,60}$` has two alternatives that both accept `_`, so a
run of underscores followed by one rejected character makes the matcher explore every division
of the run. The equivalent `^[\w@.-]{0,60}$` accepts the same identifiers and evaluates in
linear time. The three shipped schemas that carry the pattern are updated: requestIds.json,
event.json and item.json.
Differences from master, because this line has no tenancy:
- SchemaIdPatternSafetyIT sends no API key, since the context endpoint is public here. That is
the surface the unbounded evaluation was reachable from on this line.
- AllITs registers SchemaIdPatternSafetyIT alone; the other suites the master commit lists do
not exist here.
- RequestIdsSchemaPatternSafetyTest is added. The unit test that comes with #873 lives in the
json-schema services module and covers event.json and item.json, while requestIds.json ships
from the rest module and is the schema ContextRequestDeserializer applies to a public
POST /cxs/context.json. It gets a guard of its own here rather than integration coverage only.
Tests: neither module had unit tests, so junit is added at test scope to both.
---
extensions/json-schema/services/pom.xml | 5 +
.../META-INF/cxs/schemas/events/event.json | 10 +-
.../resources/META-INF/cxs/schemas/items/item.json | 2 +-
.../unomi/schema/impl/SchemaPatternSafetyTest.java | 116 ++++++++++++++++++++
.../test/java/org/apache/unomi/itests/AllITs.java | 1 +
.../unomi/itests/SchemaIdPatternSafetyIT.java | 103 ++++++++++++++++++
rest/pom.xml | 5 +
.../META-INF/cxs/schemas/rest/requestIds.json | 6 +-
.../schema/RequestIdsSchemaPatternSafetyTest.java | 118 +++++++++++++++++++++
9 files changed, 357 insertions(+), 9 deletions(-)
diff --git a/extensions/json-schema/services/pom.xml b/extensions/json-schema/services/pom.xml
index 7b41dcedf..e3fc11006 100644
--- a/extensions/json-schema/services/pom.xml
+++ b/extensions/json-schema/services/pom.xml
@@ -121,6 +121,11 @@
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
</dependency>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
diff --git a/extensions/json-schema/services/src/main/resources/META-INF/cxs/schemas/events/event.json b/extensions/json-schema/services/src/main/resources/META-INF/cxs/schemas/events/event.json
index b4673a760..2e9f75c6c 100644
--- a/extensions/json-schema/services/src/main/resources/META-INF/cxs/schemas/events/event.json
+++ b/extensions/json-schema/services/src/main/resources/META-INF/cxs/schemas/events/event.json
@@ -13,24 +13,24 @@
"properties" : {
"eventType" : {
"type" : "string",
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\w@.-]{0,60}$"
},
"profileId" : {
"type" : [ "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\w@.-]{0,60}$"
},
"sessionId" : {
"type" : [ "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\w@.-]{0,60}$"
},
"scope" : {
"type" : [ "string"],
"validateScope": true,
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\w@.-]{0,60}$"
},
"sourceId" : {
"type" : [ "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\w@.-]{0,60}$"
},
"persistent" : {
"type" : "boolean"
diff --git a/extensions/json-schema/services/src/main/resources/META-INF/cxs/schemas/items/item.json b/extensions/json-schema/services/src/main/resources/META-INF/cxs/schemas/items/item.json
index b5035f831..239d87750 100644
--- a/extensions/json-schema/services/src/main/resources/META-INF/cxs/schemas/items/item.json
+++ b/extensions/json-schema/services/src/main/resources/META-INF/cxs/schemas/items/item.json
@@ -12,7 +12,7 @@
"properties" : {
"itemId" : {
"type" : ["null","string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$",
+ "pattern" : "^[\\w@.-]{0,60}$",
"description" : "The identifier for the item"
},
"itemType" : {
diff --git a/extensions/json-schema/services/src/test/java/org/apache/unomi/schema/impl/SchemaPatternSafetyTest.java b/extensions/json-schema/services/src/test/java/org/apache/unomi/schema/impl/SchemaPatternSafetyTest.java
new file mode 100644
index 000000000..db9591779
--- /dev/null
+++ b/extensions/json-schema/services/src/test/java/org/apache/unomi/schema/impl/SchemaPatternSafetyTest.java
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.schema.impl;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.Test;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Guards the identifier-validation patterns in the shipped schemas against a linear-time regression.
+ *
+ * These patterns run on identifiers taken from public tracking requests, so they must evaluate in
+ * linear time. A pattern with overlapping quantified alternatives (for example
+ * {@code ^(\w|[-_@\.]){0,60}$}, where {@code _} matches both branches) degrades to exponential
+ * backtracking on the java.util.regex engine, so a bounded id can pin a validation thread. This test
+ * fails if any shipped id pattern exhibits that behaviour, and also pins down the accepted/rejected
+ * identifier space so a "simplification" cannot quietly change validation semantics.
+ */
+public class SchemaPatternSafetyTest {
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ private static final String[] SCHEMA_RESOURCES = {
+ "/META-INF/cxs/schemas/events/event.json",
+ "/META-INF/cxs/schemas/items/item.json"
+ };
+
+ private List<String> collectPatterns(String resource) throws Exception {
+ try (InputStream is = getClass().getResourceAsStream(resource)) {
+ assertTrue("Missing shipped schema resource " + resource, is != null);
+ List<String> patterns = new ArrayList<>();
+ collectPatterns(MAPPER.readTree(is), patterns);
+ return patterns;
+ }
+ }
+
+ private void collectPatterns(JsonNode node, List<String> patterns) {
+ if (node.isObject()) {
+ for (Iterator<Map.Entry<String, JsonNode>> it = node.fields(); it.hasNext(); ) {
+ Map.Entry<String, JsonNode> field = it.next();
+ if ("pattern".equals(field.getKey()) && field.getValue().isTextual()) {
+ patterns.add(field.getValue().asText());
+ } else {
+ collectPatterns(field.getValue(), patterns);
+ }
+ }
+ } else if (node.isArray()) {
+ for (JsonNode child : node) {
+ collectPatterns(child, patterns);
+ }
+ }
+ }
+
+ @Test(timeout = 5000)
+ public void idPatternsAreLinearTimeAndKeepSemantics() throws Exception {
+ // The classic exponential-backtracking probe for the old id pattern: a run of characters that
+ // are valid under both alternatives, followed by one invalid character forcing full backtracking.
+ StringBuilder probe = new StringBuilder();
+ for (int i = 0; i < 59; i++) {
+ probe.append('_');
+ }
+ probe.append('!');
+
+ List<String> patterns = new ArrayList<>();
+ for (String resource : SCHEMA_RESOURCES) {
+ patterns.addAll(collectPatterns(resource));
+ }
+ assertFalse("Expected id patterns in the shipped schemas", patterns.isEmpty());
+
+ for (String patternString : patterns) {
+ Pattern pattern = Pattern.compile(patternString);
+
+ // Must finish effectively instantly; the test-level timeout catches exponential backtracking.
+ assertFalse("Probe input must be rejected by " + patternString,
+ pattern.matcher(probe).matches());
+
+ // Previously accepted identifiers must still be accepted.
+ assertTrue(pattern.matcher("user_name-123@example.com").matches());
+ assertTrue(pattern.matcher("").matches());
+ assertTrue(pattern.matcher("a.b-c_d@e").matches());
+
+ // Previously rejected identifiers must still be rejected.
+ assertFalse(pattern.matcher("white space").matches());
+ assertFalse(pattern.matcher("slash/id").matches());
+ StringBuilder tooLong = new StringBuilder();
+ for (int i = 0; i < 61; i++) {
+ tooLong.append('a');
+ }
+ assertFalse(pattern.matcher(tooLong).matches());
+ }
+ }
+}
diff --git a/itests/src/test/java/org/apache/unomi/itests/AllITs.java b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
index be8f596a8..4fad7c472 100644
--- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java
+++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
@@ -53,6 +53,7 @@ import org.junit.runners.Suite.SuiteClasses;
ModifyConsentIT.class,
PatchIT.class,
ContextServletIT.class,
+ SchemaIdPatternSafetyIT.class,
RuleServiceIT.class,
PrivacyServiceIT.class,
GroovyActionsServiceIT.class,
diff --git a/itests/src/test/java/org/apache/unomi/itests/SchemaIdPatternSafetyIT.java b/itests/src/test/java/org/apache/unomi/itests/SchemaIdPatternSafetyIT.java
new file mode 100644
index 000000000..1f5167e71
--- /dev/null
+++ b/itests/src/test/java/org/apache/unomi/itests/SchemaIdPatternSafetyIT.java
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.itests;
+
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerSuite;
+
+/**
+ * Identifiers arriving on the public tracking endpoints are matched against a shipped validation
+ * pattern before anything else happens, without administrator credentials. If that match were
+ * unbounded, one small request would occupy a request thread indefinitely.
+ * <p>
+ * These tests assert the property that matters operationally — the request <em>returns</em> — rather
+ * than any particular status code. The inputs used are ones that took exponential time before the fix
+ * (seconds at 26 characters, and far beyond that at the lengths used here), so a regression does not
+ * produce a wrong answer, it produces a request that never comes back. Each is therefore bounded by a
+ * timeout: if the boundary regresses, the test fails by timing out rather than by assertion.
+ */
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerSuite.class)
+public class SchemaIdPatternSafetyIT extends BaseIT {
+
+ private final static String CONTEXT_URL = "/cxs/context.json";
+
+ /** Generous next to the sub-millisecond a bounded evaluation needs, decisive against an unbounded one. */
+ private final static int RESPONSE_TIMEOUT_MS = 30_000;
+
+ /**
+ * An identifier built to defeat the shipped id-validation pattern: a run of characters that the
+ * pattern's alternatives both accept, followed by one it rejects. Before the fix this forced the
+ * matcher to explore every division of the run.
+ */
+ @Test(timeout = RESPONSE_TIMEOUT_MS)
+ public void testMaliciousProfileIdIsValidatedInBoundedTime() throws Exception {
+ final StringBuilder maliciousId = new StringBuilder();
+ for (int i = 0; i < 55; i++) {
+ maliciousId.append('_');
+ }
+ maliciousId.append('!');
+
+ final String payload = "{\"profileId\":\"" + maliciousId + "\",\"sessionId\":\"regex-safety-session\","
+ + "\"requiredProfileProperties\":[\"*\"],\"events\":[]}";
+
+ assertReturnsPromptly(payload);
+ }
+
+ /** The same shape supplied as a sessionId, which is validated by the same pattern. */
+ @Test(timeout = RESPONSE_TIMEOUT_MS)
+ public void testMaliciousSessionIdIsValidatedInBoundedTime() throws Exception {
+ final StringBuilder maliciousId = new StringBuilder();
+ for (int i = 0; i < 55; i++) {
+ maliciousId.append('_');
+ }
+ maliciousId.append('!');
+
+ final String payload = "{\"sessionId\":\"" + maliciousId + "\",\"requiredProfileProperties\":[\"*\"],"
+ + "\"events\":[]}";
+
+ assertReturnsPromptly(payload);
+ }
+
+ /**
+ * Sends the payload to the public context endpoint and requires an answer. Any HTTP status is
+ * acceptable — rejecting the input and processing it are both fine outcomes. What is not acceptable
+ * is the request not completing, which is exactly what an unbounded evaluation causes.
+ */
+ private void assertReturnsPromptly(final String payload) throws Exception {
+ final HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL));
+ // No credential: on this line the context endpoint is public, which is the surface the
+ // unbounded evaluation was reachable from.
+ request.setEntity(new StringEntity(payload, ContentType.APPLICATION_JSON));
+
+ final long startedAt = System.currentTimeMillis();
+ try (CloseableHttpResponse response = httpClient.execute(request)) {
+ final long elapsed = System.currentTimeMillis() - startedAt;
+ Assert.assertNotNull("The public endpoint must answer rather than hang", response);
+ Assert.assertTrue("The public endpoint answered, but took " + elapsed
+ + "ms, which suggests the evaluation is no longer bounded", elapsed < RESPONSE_TIMEOUT_MS);
+ }
+ }
+}
diff --git a/rest/pom.xml b/rest/pom.xml
index 9c90d7f1b..4e6ea0cb3 100644
--- a/rest/pom.xml
+++ b/rest/pom.xml
@@ -168,6 +168,11 @@
<artifactId>slf4j-api</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
diff --git a/rest/src/main/resources/META-INF/cxs/schemas/rest/requestIds.json b/rest/src/main/resources/META-INF/cxs/schemas/rest/requestIds.json
index 366a79524..ec1cf3808 100644
--- a/rest/src/main/resources/META-INF/cxs/schemas/rest/requestIds.json
+++ b/rest/src/main/resources/META-INF/cxs/schemas/rest/requestIds.json
@@ -12,15 +12,15 @@
"properties": {
"sessionId": {
"type": ["null", "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\w@.-]{0,60}$"
},
"profileId": {
"type": ["null", "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\w@.-]{0,60}$"
},
"personaId" : {
"type" : ["null", "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\w@.-]{0,60}$"
}
}
}
diff --git a/rest/src/test/java/org/apache/unomi/rest/schema/RequestIdsSchemaPatternSafetyTest.java b/rest/src/test/java/org/apache/unomi/rest/schema/RequestIdsSchemaPatternSafetyTest.java
new file mode 100644
index 000000000..645a71de6
--- /dev/null
+++ b/rest/src/test/java/org/apache/unomi/rest/schema/RequestIdsSchemaPatternSafetyTest.java
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.rest.schema;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.Test;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Guards the identifier-validation patterns of the requestIds schema against a linear-time
+ * regression.
+ *
+ * The companion test in the json-schema services module covers event.json and item.json, which ship
+ * from that module. requestIds.json ships from this one, and it is the schema a public
+ * {@code POST /cxs/context.json} applies to its sessionId, profileId and personaId, so it needs its
+ * own guard here.
+ *
+ * A pattern with overlapping quantified alternatives (for example {@code ^(\w|[-_@\.]){0,60}$},
+ * where {@code _} matches both branches) degrades to exponential backtracking on the
+ * java.util.regex engine, so a bounded identifier can pin a validation thread.
+ */
+public class RequestIdsSchemaPatternSafetyTest {
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ private static final String SCHEMA_RESOURCE = "/META-INF/cxs/schemas/rest/requestIds.json";
+
+ /** sessionId, profileId and personaId. A dropped guard on any of the three is a live defect. */
+ private static final int EXPECTED_PATTERN_COUNT = 3;
+
+ @Test(timeout = 5000)
+ public void idPatternsAreLinearTimeAndKeepSemantics() throws Exception {
+ // The classic exponential-backtracking probe: a run of characters that are valid under both
+ // alternatives of the old pattern, followed by one invalid character forcing full backtracking.
+ StringBuilder probe = new StringBuilder();
+ for (int i = 0; i < 59; i++) {
+ probe.append('_');
+ }
+ probe.append('!');
+
+ List<String> patterns = collectPatterns();
+ assertEquals("every identifier of the requestIds schema must carry a pattern",
+ EXPECTED_PATTERN_COUNT, patterns.size());
+
+ for (String patternString : patterns) {
+ Pattern pattern = Pattern.compile(patternString);
+
+ // Must finish effectively instantly; the test-level timeout catches exponential backtracking.
+ assertFalse("Probe input must be rejected by " + patternString,
+ pattern.matcher(probe).matches());
+
+ // Previously accepted identifiers must still be accepted.
+ assertTrue(pattern.matcher("user_name-123@example.com").matches());
+ assertTrue(pattern.matcher("").matches());
+ assertTrue(pattern.matcher("a.b-c_d@e").matches());
+
+ // Previously rejected identifiers must still be rejected.
+ assertFalse(pattern.matcher("white space").matches());
+ assertFalse(pattern.matcher("slash/id").matches());
+ StringBuilder tooLong = new StringBuilder();
+ for (int i = 0; i < 61; i++) {
+ tooLong.append('a');
+ }
+ assertFalse(pattern.matcher(tooLong).matches());
+ }
+ }
+
+ private List<String> collectPatterns() throws Exception {
+ try (InputStream is = getClass().getResourceAsStream(SCHEMA_RESOURCE)) {
+ assertTrue("Missing shipped schema resource " + SCHEMA_RESOURCE, is != null);
+ List<String> patterns = new ArrayList<>();
+ collectPatterns(MAPPER.readTree(is), patterns);
+ return patterns;
+ }
+ }
+
+ private void collectPatterns(JsonNode node, List<String> patterns) {
+ if (node.isObject()) {
+ for (Iterator<Map.Entry<String, JsonNode>> it = node.fields(); it.hasNext(); ) {
+ Map.Entry<String, JsonNode> field = it.next();
+ if ("pattern".equals(field.getKey()) && field.getValue().isTextual()) {
+ patterns.add(field.getValue().asText());
+ } else {
+ collectPatterns(field.getValue(), patterns);
+ }
+ }
+ } else if (node.isArray()) {
+ for (JsonNode child : node) {
+ collectPatterns(child, patterns);
+ }
+ }
+ }
+}