This is an automated email from the ASF dual-hosted git repository.
jsinovassin pushed a commit to branch unomi-3.0.x
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/unomi-3.0.x by this push:
new b11fba3bf Simplify identifier validation patterns to a single
character class (unomi-3.0.x) (#886)
b11fba3bf is described below
commit b11fba3bf23dc48cf2e905658543949f78610759
Author: Jonathan SINOVASSIN-NAIK <[email protected]>
AuthorDate: Mon Sep 28 15:58:49 2026 +0200
Simplify identifier validation patterns to a single character class
(unomi-3.0.x) (#886)
---
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" : "^[\\[email protected]]{0,60}$"
},
"profileId" : {
"type" : [ "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\[email protected]]{0,60}$"
},
"sessionId" : {
"type" : [ "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\[email protected]]{0,60}$"
},
"scope" : {
"type" : [ "string"],
"validateScope": true,
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\[email protected]]{0,60}$"
},
"sourceId" : {
"type" : [ "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\[email protected]]{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" : "^[\\[email protected]]{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("[email protected]").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" : "^[\\[email protected]]{0,60}$"
},
"profileId": {
"type": ["null", "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\[email protected]]{0,60}$"
},
"personaId" : {
"type" : ["null", "string"],
- "pattern" : "^(\\w|[-_@\\.]){0,60}$"
+ "pattern" : "^[\\[email protected]]{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("[email protected]").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);
+ }
+ }
+ }
+}
