Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WFLY-4908] WriteAttributeHandler should better validate attribute na… #4158

Merged
merged 1 commit into from Apr 17, 2020
Merged

[WFLY-4908] WriteAttributeHandler should better validate attribute na… #4158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 52 additions & 4 deletions ...roller/src/main/java/org/jboss/as/controller/operations/global/EnhancedSyntaxSupport.java
View file
Open in desktop
Expand Up @@ -115,14 +115,25 @@ static ModelNode resolveEnhancedSyntax(final String attributeExpression, final M
return result;
}

static ModelNode updateWithEnhancedSyntax(final String attributeExpression, final ModelNode oldValue, final ModelNode newValue) throws OperationFailedException {
static ModelNode updateWithEnhancedSyntax(final String attributeExpression, final ModelNode oldValue, final ModelNode newValue,
AttributeDefinition attributeDefinition) throws OperationFailedException {
assert attributeExpression != null;
ModelNode result = oldValue;
for (String part : attributeExpression.split("\\.")) {
Matcher matcher = BRACKETS_PATTERN.matcher(part);
String[] parts = attributeExpression.split("\\.");
for (int i = 0; i < parts.length; i++) {
String part = parts[i];
Matcher matcher = BRACKETS_PATTERN.matcher(part);
if (matcher.matches()) {
if (attributeDefinition != null && attributeDefinition.getType() != ModelType.LIST) {
throw ControllerLogger.MGMT_OP_LOGGER.couldNotResolveExpressionList(attributeExpression);
}
String attribute = matcher.group(1);
int index = Integer.parseInt(matcher.group(2));
int index;
try {
index = Integer.parseInt(matcher.group(2));
} catch (NumberFormatException e) {
throw ControllerLogger.MGMT_OP_LOGGER.couldNotResolveExpression(attributeExpression);
}
ModelNode list = attribute.isEmpty() ? result : result.get(attribute); // in case we want to additionally resolve already loaded attribute, this usually applies to attributes that have custom read handlers
if (index < 0) {
result = list.add();
Expand All @@ -131,13 +142,50 @@ static ModelNode updateWithEnhancedSyntax(final String attributeExpression, fina
} else {
throw ControllerLogger.MGMT_OP_LOGGER.couldNotResolveExpressionList(attributeExpression);
}
if (i < parts.length - 1) {
// More parts implies this is a list of objects, so if the AD type allows it
// track the AD of the value type in order to validate later parts
attributeDefinition = attributeDefinition instanceof ObjectListAttributeDefinition
? ((ObjectListAttributeDefinition) attributeDefinition).getValueType()
: null; // oh well, AD impl won't let us validate further
}
} else if (attributeDefinition != null && !part.equals(attributeDefinition.getName())) {
// Unknown part
throw ControllerLogger.MGMT_OP_LOGGER.couldNotResolveExpression(attributeExpression);
} else if (attributeDefinition == null && part.contains("[")) {
// A valid attribute or field name wouldn't include an unmatched '['.
throw ControllerLogger.MGMT_OP_LOGGER.couldNotResolveExpression(attributeExpression);
} else {
if (!result.isDefined() || result.getType() == ModelType.OBJECT) {
result = result.get(part);
} else {
throw ControllerLogger.MGMT_OP_LOGGER.couldNotResolveExpression(attributeExpression);
}
}

if (i < parts.length - 1) {
// Try to find the AD for the next part to use in validating it
if (attributeDefinition instanceof ObjectTypeAttributeDefinition) {
String nextPart = parts[i + 1];
int bracket = nextPart.indexOf('[');
nextPart = bracket < 0 ? nextPart : nextPart.substring(0, bracket);
// See if the next part matches a field
AttributeDefinition[] fields = ((ObjectTypeAttributeDefinition) attributeDefinition).getValueTypes();
attributeDefinition = null;
for (AttributeDefinition field : fields) {
if (field.getName().equals(nextPart)) {
attributeDefinition = field;
break;
}
}
if (attributeDefinition == null) {
throw ControllerLogger.MGMT_OP_LOGGER.couldNotResolveExpression(attributeExpression);
}
} else {
// oh well, AD impl won't let us validate further
attributeDefinition = null;
}
}
}
result.set(newValue);
return oldValue;
Expand Down
13 changes: 9 additions & 4 deletions ...roller/src/main/java/org/jboss/as/controller/operations/global/WriteAttributeHandler.java
View file
Open in desktop
Expand Up @@ -31,6 +31,7 @@
import static org.jboss.as.controller.operations.global.GlobalOperationAttributes.NAME;
import static org.jboss.as.controller.operations.global.GlobalOperationAttributes.VALUE;

import org.jboss.as.controller.AttributeDefinition;
import org.jboss.as.controller.OperationContext;
import org.jboss.as.controller.OperationDefinition;
import org.jboss.as.controller.OperationFailedException;
Expand Down Expand Up @@ -170,7 +171,8 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
private void doExecuteInternal(OperationContext context, ModelNode operation, AttributeAccess attributeAccess, String attributeName, ModelNode currentValue, boolean useEnhancedSyntax, String attributeExpression) throws OperationFailedException {
if (useEnhancedSyntax){
if (attributeAccess.getStorageType() == AttributeAccess.Storage.CONFIGURATION) {
operation = getEnhancedSyntaxResolvedOperation(operation, currentValue, attributeName, attributeExpression);
operation = getEnhancedSyntaxResolvedOperation(operation, currentValue, attributeName, attributeExpression,
attributeAccess.getAttributeDefinition());
} else {
assert attributeAccess.getStorageType() == AttributeAccess.Storage.RUNTIME;

Expand All @@ -181,7 +183,8 @@ private void doExecuteInternal(OperationContext context, ModelNode operation, At
operation = resolvedOperation;

context.addStep((context1, operation1) -> {
ModelNode resolved = getEnhancedSyntaxResolvedOperation(originalOperation, currentValue, attributeName, attributeExpression);
ModelNode resolved = getEnhancedSyntaxResolvedOperation(originalOperation, currentValue, attributeName,
attributeExpression, attributeAccess.getAttributeDefinition());
resolvedOperation.get(ModelDescriptionConstants.NAME).set(resolved.get(ModelDescriptionConstants.NAME));
resolvedOperation.get(ModelDescriptionConstants.VALUE).set(resolved.get(ModelDescriptionConstants.VALUE));
}, OperationContext.Stage.RUNTIME);
Expand Down Expand Up @@ -209,12 +212,14 @@ private void emitAttributeValueWrittenNotification(OperationContext context, Pat
context.emit(notification);
}

private ModelNode getEnhancedSyntaxResolvedOperation(ModelNode originalOperation, ModelNode currentModel, String attributeName, String attributeExpression) throws OperationFailedException {
private ModelNode getEnhancedSyntaxResolvedOperation(ModelNode originalOperation, ModelNode currentModel,
String attributeName, String attributeExpression,
AttributeDefinition attributeDefinition) throws OperationFailedException {
ModelNode writeOp = originalOperation.clone();
ModelNode diffValue = originalOperation.get(ModelDescriptionConstants.VALUE);
ModelNode old = new ModelNode();
old.get(attributeName).set(currentModel);
ModelNode fullValue = EnhancedSyntaxSupport.updateWithEnhancedSyntax(attributeExpression, old, diffValue);
ModelNode fullValue = EnhancedSyntaxSupport.updateWithEnhancedSyntax(attributeExpression, old, diffValue, attributeDefinition);
writeOp.get(ModelDescriptionConstants.NAME).set(attributeName);
writeOp.get(ModelDescriptionConstants.VALUE).set(fullValue.get(attributeName));
return writeOp;
Expand Down
19 changes: 19 additions & 0 deletions ...roller/src/test/java/org/jboss/as/controller/operation/global/EnhancedSyntaxTestCase.java
View file
Open in desktop
Expand Up @@ -805,4 +805,23 @@ private void performCompositeMapWrite(String attrName) throws Exception {
Assert.assertEquals("map-value2", result.get("map-key2").asString());
}

@Test
public void testBadIndex() throws Exception {
//test set
ModelNode op = createOperation("write-attribute", TEST_ADDRESS);
op.get("name").set(OBJECT_LIST.getName() + "[zero]");
op.get("value").set(ModelNode.ZERO);
ModelNode resp = executeCheckForFailure(op);
Assert.assertTrue(resp.toString(), resp.toString().contains("WFLYCTL0393"));
}

@Test
public void testInvalidDot() throws Exception {
//test set
ModelNode op = createOperation("write-attribute", TEST_ADDRESS);
op.get("name").set(OBJECT_LIST.getName() + "[1.1]");
op.get("value").set(ModelNode.ZERO);
ModelNode resp = executeCheckForFailure(op);
Assert.assertTrue(resp.toString(), resp.toString().contains("WFLYCTL0393"));
}
}