This is an automated email from the ASF dual-hosted git repository.
sergehuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/master by this push:
new 7dab55a50 Simplify identifier validation patterns to a single
character class (#873)
7dab55a50 is described below
commit 7dab55a5068a0ba169367b73f8e24492ea4e8452
Author: Serge Huber <[email protected]>
AuthorDate: Thu Sep 24 19:54:47 2026 +0200
Simplify identifier validation patterns to a single character class (#873)
Co-authored-by: Claude Opus 4.8 <[email protected]>
---
.../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 | 104 ++++++++++++++++++
.../META-INF/cxs/schemas/rest/requestIds.json | 6 +-
6 files changed, 230 insertions(+), 9 deletions(-)
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 4f46c3bef..e98b21763 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 f13057981..19b07796b 100644
--- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java
+++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
@@ -57,6 +57,7 @@ import org.junit.runners.Suite.SuiteClasses;
PatchIT.class,
ContextServletIT.class,
ConditionRegexSafetyIT.class,
+ SchemaIdPatternSafetyIT.class,
ContextEndpointBaselineIT.class,
SecurityIT.class,
RuleServiceIT.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..e8ba6783e
--- /dev/null
+++ b/itests/src/test/java/org/apache/unomi/itests/SchemaIdPatternSafetyIT.java
@@ -0,0 +1,104 @@
+/*
+ * 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";
+
+ private final static String UNOMI_API_KEY_HTTP_HEADER_KEY =
"X-Unomi-Api-Key";
+
+ /** 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));
+ request.addHeader(UNOMI_API_KEY_HTTP_HEADER_KEY, testPublicKeyValue);
+ 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/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}$"
}
}
}
