Commit 5bcc320e authored by cdanger's avatar cdanger

- Added VariableId uniqueness conformance test

- Code enhancement: replaced Map#put(...) with Map#putIfAbsent(..)
introduced in Java 8
parent c240028e
......@@ -42,7 +42,7 @@
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>${artifactId.prefix}-core-pdp-api</artifactId>
<version>8.0.0</version>
<version>8.0.1-SNAPSHOT</version>
</dependency>
<!-- /Authzforce dependencies -->
......
......@@ -190,13 +190,13 @@ public final class CloseableAttributeProvider extends ModularAttributeProvider i
for (final AttributeDesignatorType attrDesignator : moduleAdapter.getProvidedAttributes())
{
final AttributeGUID attrGUID = new AttributeGUID(attrDesignator);
if (modulesByAttributeId.containsKey(attrGUID))
final AttributeProviderModule duplicate = modulesByAttributeId.putIfAbsent(attrGUID, moduleAdapter.getAdaptedModule());
if (duplicate != null)
{
moduleAdapter.close();
throw new IllegalArgumentException("Conflict: " + moduleAdapter + " providing the same AttributeDesignator (" + attrGUID + ") as another already registered.");
}
modulesByAttributeId.put(attrGUID, moduleAdapter.getAdaptedModule());
}
}
catch (final IllegalArgumentException e)
......
......@@ -137,11 +137,11 @@ public final class DefaultRequestFilter extends BaseRequestFilter
final XdmNode newContentNode = categorySpecificAttributes.getExtraContent();
if (newContentNode != null)
{
final XdmNode oldContentNode = extraContentsByCategory.put(categoryName, newContentNode);
final XdmNode duplicate = extraContentsByCategory.putIfAbsent(categoryName, newContentNode);
/*
* No support for Multiple Decision Profile -> no support for repeated categories as specified in Multiple Decision Profile. So we must check duplicate attribute categories.
*/
if (oldContentNode != null)
if (duplicate != null)
{
throw new IndeterminateEvaluationException("Unsupported repetition of Attributes[@Category='" + categoryName
+ "'] (feature 'urn:oasis:names:tc:xacml:3.0:profile:multiple:repeated-attribute-categories' is not supported)", StatusHelper.STATUS_SYNTAX_ERROR);
......
......@@ -161,21 +161,22 @@ public final class IndividualDecisionRequestContext implements EvaluationContext
@Override
public boolean putAttributeDesignatorResultIfAbsent(final AttributeGUID id, final Bag<?> result)
{
if (namedAttributes.containsKey(id))
final Bag<?> duplicate = namedAttributes.putIfAbsent(id, result);
if (duplicate != null)
{
/*
* This should never happen, as getAttributeDesignatorResult() should have been called first (for same id) and returned this oldResult, and no further call to
* putAttributeDesignatorResultIfAbsent() in this case. In any case, we do not support setting a different result for same id (but different datatype URI/datatype class) in the same
* context
*/
LOGGER.warn("Attempt to override value of AttributeDesignator {} already set in evaluation context. Overriding value: {}", id, result);
LOGGER.error("Attempt to override value of AttributeDesignator {} already set in evaluation context. Overriding value: {}", id, result);
return false;
}
/*
* Attribute value cannot change during evaluation context, so if old value already there, put it back
*/
return namedAttributes.put(id, result) == null;
return true;
}
/** {@inheritDoc} */
......@@ -210,13 +211,13 @@ public final class IndividualDecisionRequestContext implements EvaluationContext
@Override
public boolean putVariableIfAbsent(final String variableId, final Value value)
{
if (varValsById.containsKey(variableId))
if (varValsById.putIfAbsent(variableId, value) != null)
{
LOGGER.error("Attempt to override value of Variable '{}' already set in evaluation context. Overriding value: {}", variableId, value);
return false;
}
return varValsById.put(variableId, value) == null;
return true;
}
/** {@inheritDoc} */
......@@ -257,13 +258,13 @@ public final class IndividualDecisionRequestContext implements EvaluationContext
@Override
public boolean putAttributeSelectorResultIfAbsent(final AttributeSelectorId id, final Bag<?> result) throws IndeterminateEvaluationException
{
if (attributeSelectorResults.containsKey(id))
if (attributeSelectorResults.putIfAbsent(id, result) != null)
{
LOGGER.error("Attempt to override value of AttributeSelector {} already set in evaluation context. Overriding value: {}", id, result);
return false;
}
return attributeSelectorResults.put(id, result) == null;
return true;
}
/** {@inheritDoc} */
......
......@@ -109,8 +109,8 @@ public final class MutableIndividualDecisionRequest implements IndividualDecisio
final XdmNode newContentNode = categorySpecificAttributes.getExtraContent();
if (newContentNode != null)
{
final XdmNode oldContentNode = extraContentsByCategory.put(categoryName, newContentNode);
if (oldContentNode != null)
final XdmNode duplicate = extraContentsByCategory.putIfAbsent(categoryName, newContentNode);
if (duplicate != null)
{
throw new IllegalArgumentException("Duplicate Attributes[@Category] in Individual Decision Request (not allowed): " + categoryName);
}
......
......@@ -83,10 +83,10 @@ public final class PdpExtensionLoader
if (extension instanceof JaxbBoundPdpExtension<?>)
{
final JaxbBoundPdpExtension<?> jaxbBoundExt = (JaxbBoundPdpExtension<?>) extension;
final JaxbBoundPdpExtension<?> conflictingExt = mutableJaxbBoundExtMapByClass.put(jaxbBoundExt.getJaxbClass(), jaxbBoundExt);
if (conflictingExt != null)
final JaxbBoundPdpExtension<?> duplicate = mutableJaxbBoundExtMapByClass.putIfAbsent(jaxbBoundExt.getJaxbClass(), jaxbBoundExt);
if (duplicate != null)
{
throw new IllegalArgumentException("Extension " + jaxbBoundExt + " (" + jaxbBoundExt.getClass() + ") is conflicting with " + conflictingExt + "(" + conflictingExt.getClass()
throw new IllegalArgumentException("Extension " + jaxbBoundExt + " (" + jaxbBoundExt.getClass() + ") is conflicting with " + duplicate + "(" + duplicate.getClass()
+ ") for the same XML/JAXB configuration class: " + jaxbBoundExt.getJaxbClass());
}
......@@ -98,10 +98,10 @@ public final class PdpExtensionLoader
{
if (extClass.isInstance(extension))
{
final PdpExtension conflictingExt = mutableNonJaxbBoundExtMapByClassAndId.put(extClass, extension.getId(), extension);
if (conflictingExt != null)
final PdpExtension duplicate = mutableNonJaxbBoundExtMapByClassAndId.put(extClass, extension.getId(), extension);
if (duplicate != null)
{
throw new IllegalArgumentException("Extension " + extension + " is conflicting with " + conflictingExt + " registered with same ID: " + extension.getId());
throw new IllegalArgumentException("Extension " + extension + " is conflicting with " + duplicate + " registered with same ID: " + extension.getId());
}
isValidExt = true;
......
......@@ -422,7 +422,7 @@ public final class ExpressionFactoryImpl implements ExpressionFactory
}
final BaseVariableReference<?> var = newVariableReference(varId, varExpr, longestVarRefChainInCurrentVarExpression);
return idToVariableMap.put(varId, var);
return idToVariableMap.putIfAbsent(varId, var);
}
/** {@inheritDoc} */
......
......@@ -1006,7 +1006,7 @@ public final class PolicyEvaluators
{
assert staticRefPoliciesToUpdate != null && newRefPolicyId != null && newRefPolicyVersion != null;
final PolicyVersion otherVersion = staticRefPoliciesToUpdate.put(newRefPolicyId, newRefPolicyVersion);
final PolicyVersion otherVersion = staticRefPoliciesToUpdate.putIfAbsent(newRefPolicyId, newRefPolicyVersion);
if (otherVersion != null && !otherVersion.equals(newRefPolicyVersion))
{
throw new IllegalArgumentException(policyFriendlyId + ": policy references to same policy ID (" + newRefPolicyId + ") but different versions (" + otherVersion + " and "
......@@ -1631,7 +1631,7 @@ public final class PolicyEvaluators
throw new IllegalArgumentException(policyFriendlyId + ": Error parsing child #" + childIndex + " (Rule)", e);
}
final RuleEvaluator conflictingRuleEvaluator = ruleEvaluatorsByRuleIdInOrderOfDeclaration.put(ruleEvaluator.getRuleId(), ruleEvaluator);
final RuleEvaluator conflictingRuleEvaluator = ruleEvaluatorsByRuleIdInOrderOfDeclaration.putIfAbsent(ruleEvaluator.getRuleId(), ruleEvaluator);
if (conflictingRuleEvaluator != null)
{
/*
......@@ -2104,15 +2104,22 @@ public final class PolicyEvaluators
final IdReferenceType idRef = (IdReferenceType) jaxbElt.getValue();
final COMBINED_EVALUATOR childEvaluator = policyEvaluatorFactory.getChildPolicyRefEvaluator(childIndex, TopLevelPolicyElementType.POLICY, idRef, null);
combinedEvaluators.add(childEvaluator);
childPolicySetEvaluatorsByPolicySetId.put(childEvaluator.getPolicyId(), childEvaluator);
final COMBINED_EVALUATOR duplicate = childPolicySetEvaluatorsByPolicySetId.putIfAbsent(childEvaluator.getPolicyId(), childEvaluator);
if (duplicate != null)
{
throw new IllegalArgumentException("Duplicate PolicyIdReference's id = " + childEvaluator.getPolicyId());
}
}
else if (eltNameLocalPart.equals(XACMLNodeName.POLICYSET_ID_REFERENCE.value()))
{
final IdReferenceType idRef = (IdReferenceType) jaxbElt.getValue();
final COMBINED_EVALUATOR childEvaluator = policyEvaluatorFactory.getChildPolicyRefEvaluator(childIndex, TopLevelPolicyElementType.POLICY_SET, idRef, policySetRefChain);
combinedEvaluators.add(childEvaluator);
childPolicySetEvaluatorsByPolicySetId.put(childEvaluator.getPolicyId(), childEvaluator);
final COMBINED_EVALUATOR duplicate = childPolicySetEvaluatorsByPolicySetId.put(childEvaluator.getPolicyId(), childEvaluator);
if (duplicate != null)
{
throw new IllegalArgumentException("Duplicate PolicySetIdReference's id = " + childEvaluator.getPolicyId());
}
}
else if (eltNameLocalPart.equals(XACMLNodeName.COMBINER_PARAMETERS.value()))
{
......@@ -2149,7 +2156,11 @@ public final class PolicyEvaluators
final COMBINED_EVALUATOR childEvaluator = policyEvaluatorFactory.getChildPolicySetEvaluator(childIndex, childPolicy, nonNullParsedPolicyIds, updatableParsedPolicySetIds,
policySetRefChain);
combinedEvaluators.add(childEvaluator);
childPolicySetEvaluatorsByPolicySetId.put(childPolicyId, childEvaluator);
final COMBINED_EVALUATOR duplicate = childPolicySetEvaluatorsByPolicySetId.putIfAbsent(childPolicyId, childEvaluator);
if (duplicate != null)
{
throw new IllegalArgumentException("Duplicate PolicySetId = " + childPolicyId);
}
}
else if (policyChildElt instanceof Policy)
{
......@@ -2165,7 +2176,12 @@ public final class PolicyEvaluators
final COMBINED_EVALUATOR childEvaluator = policyEvaluatorFactory.getChildPolicyEvaluator(childIndex, childPolicy);
combinedEvaluators.add(childEvaluator);
childPolicyEvaluatorsByPolicyId.put(childPolicyId, childEvaluator);
final COMBINED_EVALUATOR duplicate = childPolicyEvaluatorsByPolicyId.putIfAbsent(childPolicyId, childEvaluator);
if (duplicate != null)
{
throw new IllegalArgumentException("Duplicate PolicyId = " + childPolicyId);
}
}
/*
......
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<Policy xmlns="urn:oasis:names:tc:xacml:3.0:core:schema:wd-17" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" PolicyId="policy-with-duplicate-RuleId"
RuleCombiningAlgId="urn:oasis:names:tc:xacml:3.0:rule-combining-algorithm:deny-overrides" Version="1.0">
<Description>
Purpose: Test detection of duplicate RuleId within Policy
</Description>
<Target />
<VariableDefinition VariableId="bartSimpsonAge">
<Apply FunctionId="urn:oasis:names:tc:xacml:1.0:function:integer-one-and-only">
<AttributeDesignator AttributeId="urn:oasis:names:tc:xacml:2.0:conformance-test:bart-simpson-age" Category="urn:oasis:names:tc:xacml:3.0:attribute-category:environment"
DataType="http://www.w3.org/2001/XMLSchema#integer" MustBePresent="false" />
</Apply>
</VariableDefinition>
<VariableDefinition VariableId="bartSimpsonAge">
<AttributeValue DataType="http://www.w3.org/2001/XMLSchema#integer">20</AttributeValue>
</VariableDefinition>
</Policy>
......@@ -41,7 +41,9 @@ For a description of the tests, see file `ConformanceTests.html` which is the or
1. IIIG302 does not exist, but IIIG300 does. Fix: re-numbered 301 to 302, and 300 to 301.
1. IIIG301 and IIIG302: tests for ReturnPolicyIdList: the XACML specification is ambiguous about what is considered an "applicable" policy, and therefore what should be included in the PolicyIdentifierList. See the discussion here for more info: https://lists.oasis-open.org/archives/xacml-comment/201605/msg00004.html. In our (Authzforce definition), for instance, even if a policy evaluates to Indeterminate, it may still be considered applicable, which makes it different from what was intended in the original conformance tests. More generally, we define here an "applicable" policy as follows: a policy is "applicable" if and only if its evaluation result is different from NotApplicable (not NotApplicable means Applicable, shouldn't it?), and one of these two conditions is met:
* The policy/policy reference has no enclosing policy, i.e. it is the root policy in PDP's evaluation.
* The policy has an enclosing policy and the enclosing policy is "applicable". (This definition is recursive.)
* The policy has an enclosing policy and the enclosing policy is "applicable". (This definition is recursive.)
1. These conformance tests do not include any test on VariableDefinitions/VariableReferences, such as VariableId uniqueness. We added our own in the parent directory.
1. These conformance tests do not include any test Policy(Set)Id/RuleId uniqueness. We added our own in the parent directory.
**WARNING**: There are conformance tests which are intentionally not supported (in `unsupported` directory):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment