This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/circular-reference-no-id in repository https://gitbox.apache.org/repos/asf/ws-neethi.git
commit 39870cf545820905939359d9ae8017b00d524e0c Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Thu Jul 9 16:59:06 2026 +0100 Detect cycles when there is no Policy Id --- .../org/apache/neethi/AbstractPolicyOperator.java | 20 +++++--- .../org/apache/neethi/PolicyReferenceTest.java | 56 +++++++++++++++++----- 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/apache/neethi/AbstractPolicyOperator.java b/src/main/java/org/apache/neethi/AbstractPolicyOperator.java index bdac65a..b4f920c 100644 --- a/src/main/java/org/apache/neethi/AbstractPolicyOperator.java +++ b/src/main/java/org/apache/neethi/AbstractPolicyOperator.java @@ -138,6 +138,9 @@ public abstract class AbstractPolicyOperator implements PolicyOperator { } } else if (policyComponent.getType() == Constants.TYPE_POLICY_REF) { String uri = ((PolicyReference) policyComponent).getURI(); + if (uri == null || uri.length() == 0) { + throw new RuntimeException("PolicyReference URI is null or empty"); + } policyComponent = reg == null ? null : reg.lookup(uri); if (policyComponent == null && uri.charAt(0) == '#') { String id = uri.substring(1); @@ -155,14 +158,17 @@ public abstract class AbstractPolicyOperator implements PolicyOperator { throw new RuntimeException(uri + " can't be resolved"); } String resolvedId = ((Policy) policyComponent).getId(); - if (resolvedId != null && !resolving.add(resolvedId)) { - throw new RuntimeException("Circular PolicyReference detected: " + resolvedId); + String resolvingToken = resolvedId != null && resolvedId.length() > 0 + ? "id:" + resolvedId : "uri:" + uri; + if (!resolving.add(resolvingToken)) { + throw new RuntimeException("Circular PolicyReference detected: " + resolvingToken); } - All all = new All(); - all.addPolicyComponents(((Policy) policyComponent).getPolicyComponents()); - childComponentsList.add(AbstractPolicyOperator.normalizeOperator(policy, all, reg, deep, resolving)); - if (resolvedId != null) { - resolving.remove(resolvedId); + try { + All all = new All(); + all.addPolicyComponents(((Policy) policyComponent).getPolicyComponents()); + childComponentsList.add(AbstractPolicyOperator.normalizeOperator(policy, all, reg, deep, resolving)); + } finally { + resolving.remove(resolvingToken); } } else if (policyComponent.getType() == Constants.TYPE_POLICY) { diff --git a/src/test/java/org/apache/neethi/PolicyReferenceTest.java b/src/test/java/org/apache/neethi/PolicyReferenceTest.java index e0d0c46..b5b99a8 100644 --- a/src/test/java/org/apache/neethi/PolicyReferenceTest.java +++ b/src/test/java/org/apache/neethi/PolicyReferenceTest.java @@ -164,19 +164,10 @@ public class PolicyReferenceTest extends PolicyTestCase { } /** - * Test two policies that mutually reference each other cause unbounded recursion in - * AbstractPolicyOperator.normalizeOperator() when normalize(true) is called. - * - * Policy A contains a PolicyReference to "B". - * Policy B contains a PolicyReference to "A". - * Both are registered in the PolicyRegistry before normalization begins. - * - * normalizeOperator() resolves each PolicyReference via reg.lookup() and - * recurses into the resolved policy's components with no cycle detection, - * ultimately throwing StackOverflowError. + * Ensures circular references are detected for policies with explicit Id values. */ @Test - public void testCircularPolicyReferenceThrowsStackOverflow() { + public void testCircularPolicyReferenceWithIdsIsDetected() { // Build Policy A: ExactlyOne > All > PolicyReference("B") PolicyReference refToB = new PolicyReference(); refToB.setURI("B"); @@ -204,7 +195,48 @@ public class PolicyReferenceTest extends PolicyTestCase { try { policyA.normalize(registry, true); - fail("Expected StackOverflowError or RuntimeException due to circular reference"); + fail("Expected RuntimeException due to circular reference"); + } catch (RuntimeException e) { + assertTrue( + "Expected a cycle-detection message but got: " + e.getMessage(), + e.getMessage() != null && e.getMessage().toLowerCase().contains("circular")); + } + } + + /** + * Ensures circular references are detected even when referenced policies have no Id. + * + * Cycle detection must use a stable non-null resolution token so that + * no-Id references cannot bypass the resolving set. + */ + @Test + public void testCircularPolicyReferenceWithoutIdsIsDetected() { + // Policy A references registry key "B" but has no policy Id. + PolicyReference refToB = new PolicyReference(); + refToB.setURI("B"); + All allA = new All(); + allA.addPolicyComponent(refToB); + ExactlyOne eoA = new ExactlyOne(); + eoA.addPolicyComponent(allA); + Policy policyA = new Policy(registry); + policyA.addPolicyComponent(eoA); + + // Policy B references registry key "A" and also has no policy Id. + PolicyReference refToA = new PolicyReference(); + refToA.setURI("A"); + All allB = new All(); + allB.addPolicyComponent(refToA); + ExactlyOne eoB = new ExactlyOne(); + eoB.addPolicyComponent(allB); + Policy policyB = new Policy(registry); + policyB.addPolicyComponent(eoB); + + registry.register("A", policyA); + registry.register("B", policyB); + + try { + policyA.normalize(registry, true); + fail("Expected RuntimeException due to circular reference"); } catch (RuntimeException e) { assertTrue( "Expected a cycle-detection message but got: " + e.getMessage(),
