This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch bound-matchesregex-condition-evaluation in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 7ec8eed1f6e88298c98b12ec20b097ec4f4b1d4a Author: Serge Huber <[email protected]> AuthorDate: Fri Aug 28 07:34:07 2026 +0200 Cover caller-supplied condition patterns with an integration test Sends personalization conditions carrying a caller-supplied regular expression to the public tracking endpoint: the catastrophic backtracking case, and a pattern far longer than any legitimate one. Each is bounded by a test timeout rather than an assertion on the response, because a regression here does not return a wrong answer: the request stops coming back at all. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../test/java/org/apache/unomi/itests/AllITs.java | 1 + .../unomi/itests/ConditionRegexSafetyIT.java | 110 +++++++++++++++++++++ 2 files changed, 111 insertions(+) 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 21511f8d8..f13057981 100644 --- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java +++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java @@ -56,6 +56,7 @@ import org.junit.runners.Suite.SuiteClasses; ModifyConsentIT.class, PatchIT.class, ContextServletIT.class, + ConditionRegexSafetyIT.class, ContextEndpointBaselineIT.class, SecurityIT.class, RuleServiceIT.class, diff --git a/itests/src/test/java/org/apache/unomi/itests/ConditionRegexSafetyIT.java b/itests/src/test/java/org/apache/unomi/itests/ConditionRegexSafetyIT.java new file mode 100644 index 000000000..c622413e8 --- /dev/null +++ b/itests/src/test/java/org/apache/unomi/itests/ConditionRegexSafetyIT.java @@ -0,0 +1,110 @@ +/* + * 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; + +/** + * A personalization condition on the public tracking endpoints lets the caller supply a regular + * expression outright, without administrator credentials. If its evaluation 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 ConditionRegexSafetyIT 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; + + /** + * A personalization filter carrying a caller-supplied pattern. {@code (a+)+$} against a long run of + * 'a' followed by a non-matching character is the textbook catastrophic case; the caller controls + * both the pattern and, through the identifier, much of the subject. + */ + @Test(timeout = RESPONSE_TIMEOUT_MS) + public void testCallerSuppliedRegexConditionIsEvaluatedInBoundedTime() throws Exception { + final StringBuilder subject = new StringBuilder(); + for (int i = 0; i < 60; i++) { + subject.append('a'); + } + + final String payload = "{\"sessionId\":\"regex-condition-session\",\"profileId\":\"" + subject + "\"," + + "\"requiredProfileProperties\":[\"*\"],\"events\":[]," + + "\"filters\":[{\"id\":\"regex-safety-filter\",\"filters\":[{\"condition\":{" + + "\"type\":\"profilePropertyCondition\",\"parameterValues\":{" + + "\"propertyName\":\"itemId\",\"comparisonOperator\":\"matchesRegex\"," + + "\"propertyValue\":\"(a+)+$\"}}}]}]}"; + + assertReturnsPromptly(payload); + } + + /** A pattern far longer than any legitimate one must be refused rather than compiled and run. */ + @Test(timeout = RESPONSE_TIMEOUT_MS) + public void testOversizedCallerSuppliedRegexIsRefused() throws Exception { + final StringBuilder hugePattern = new StringBuilder(); + for (int i = 0; i < 2000; i++) { + hugePattern.append("a?"); + } + + final String payload = "{\"sessionId\":\"regex-oversize-session\",\"requiredProfileProperties\":[\"*\"]," + + "\"events\":[]," + + "\"filters\":[{\"id\":\"regex-oversize-filter\",\"filters\":[{\"condition\":{" + + "\"type\":\"profilePropertyCondition\",\"parameterValues\":{" + + "\"propertyName\":\"itemId\",\"comparisonOperator\":\"matchesRegex\"," + + "\"propertyValue\":\"" + hugePattern + "\"}}}]}]}"; + + 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); + } + } +}
