From 3eb270549d258cfc58ffcd9407878cfef4976942 Mon Sep 17 00:00:00 2001 From: Matt Benson Date: Tue, 26 Mar 2024 18:14:03 -0500 Subject: [PATCH] validate all profile activation config strings --- .../validation/DefaultModelValidator.java | 249 ++++++++++++++++-- .../validation/DefaultModelValidatorTest.java | 37 ++- ...tivation-file-with-allowed-expressions.xml | 18 ++ ...tion-property-with-project-expressions.xml | 49 ++++ 4 files changed, 323 insertions(+), 30 deletions(-) create mode 100644 maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index d2553c455df5..39c4ecbb1707 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -25,17 +25,29 @@ import java.io.File; import java.util.Arrays; import java.util.Collections; +import java.util.Deque; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.function.UnaryOperator; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import org.apache.maven.api.model.Activation; +import org.apache.maven.api.model.Activation.Builder; import org.apache.maven.api.model.ActivationFile; +import org.apache.maven.api.model.ActivationOS; +import org.apache.maven.api.model.ActivationProperty; import org.apache.maven.api.model.Build; import org.apache.maven.api.model.BuildBase; import org.apache.maven.api.model.Dependency; @@ -61,6 +73,7 @@ import org.apache.maven.model.building.ModelProblemCollectorRequest; import org.apache.maven.model.interpolation.ModelVersionProcessor; import org.apache.maven.model.v4.MavenModelVersion; +import org.apache.maven.model.v4.MavenTransformer; /** */ @@ -82,6 +95,180 @@ public class DefaultModelValidator implements ModelValidator { private static final String EMPTY = ""; + private record ActivationFrame(String location, Optional parent) {} + + private static class ActivationWalker extends MavenTransformer { + + private final Deque stk; + + ActivationWalker(Deque stk, UnaryOperator transformer) { + super(transformer); + this.stk = stk; + } + + private ActivationFrame nextFrame(String property) { + return new ActivationFrame(property, Optional.empty()); + } + + private

ActivationFrame nextFrame(String property, Function child) { + @SuppressWarnings("unchecked") + final Optional

parent = (Optional

) stk.peek().parent; + return new ActivationFrame(property, parent.map(child)); + } + + @Override + public Activation transformActivation(Activation target) { + stk.push(new ActivationFrame("activation", Optional.of(target))); + try { + return super.transformActivation(target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivation_ActiveByDefault(Builder builder, Activation target) {} + + @Override + protected void transformActivation_File(Builder builder, Activation target) { + stk.push(nextFrame("file", Activation::getFile)); + Optional.ofNullable(target.getFile()); + try { + super.transformActivation_File(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivationFile_Exists( + org.apache.maven.api.model.ActivationFile.Builder builder, ActivationFile target) { + stk.push(nextFrame("exists")); + try { + super.transformActivationFile_Exists(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivationFile_Missing( + org.apache.maven.api.model.ActivationFile.Builder builder, ActivationFile target) { + stk.push(nextFrame("missing")); + try { + super.transformActivationFile_Missing(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivation_Jdk(Builder builder, Activation target) { + stk.push(nextFrame("jdk")); + try { + super.transformActivation_Jdk(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivation_Os(Builder builder, Activation target) { + stk.push(nextFrame("os", Activation::getOs)); + try { + super.transformActivation_Os(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivationOS_Arch( + org.apache.maven.api.model.ActivationOS.Builder builder, ActivationOS target) { + stk.push(nextFrame("arch")); + try { + super.transformActivationOS_Arch(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivationOS_Family( + org.apache.maven.api.model.ActivationOS.Builder builder, ActivationOS target) { + stk.push(nextFrame("family")); + try { + super.transformActivationOS_Family(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivationOS_Name( + org.apache.maven.api.model.ActivationOS.Builder builder, ActivationOS target) { + stk.push(nextFrame("name")); + try { + super.transformActivationOS_Name(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivationOS_Version( + org.apache.maven.api.model.ActivationOS.Builder builder, ActivationOS target) { + stk.push(nextFrame("version")); + try { + super.transformActivationOS_Version(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivation_Packaging(Builder builder, Activation target) { + stk.push(nextFrame("packaging")); + try { + super.transformActivation_Packaging(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivation_Property(Builder builder, Activation target) { + stk.push(nextFrame("property", Activation::getProperty)); + try { + super.transformActivation_Property(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivationProperty_Name( + org.apache.maven.api.model.ActivationProperty.Builder builder, ActivationProperty target) { + stk.push(nextFrame("name")); + try { + super.transformActivationProperty_Name(builder, target); + } finally { + stk.pop(); + } + } + + @Override + protected void transformActivationProperty_Value( + org.apache.maven.api.model.ActivationProperty.Builder builder, ActivationProperty target) { + stk.push(nextFrame("value")); + try { + super.transformActivationProperty_Value(builder, target); + } finally { + stk.pop(); + } + } + } + private final Set validCoordinateIds = new HashSet<>(); private final Set validProfileIds = new HashSet<>(); @@ -288,42 +475,52 @@ public void validateRawModel(Model ma, ModelBuildingRequest request, ModelProble } private void validate30RawProfileActivation(ModelProblemCollector problems, Activation activation, String prefix) { - if (activation == null || activation.getFile() == null) { + if (activation == null) { return; } - - ActivationFile file = activation.getFile(); - - String path; - String location; - - if (file.getExists() != null && !file.getExists().isEmpty()) { - path = file.getExists(); - location = "exists"; - } else if (file.getMissing() != null && !file.getMissing().isEmpty()) { - path = file.getMissing(); - location = "missing"; - } else { - return; - } - - if (hasProjectExpression(path)) { - Matcher matcher = EXPRESSION_PROJECT_NAME_PATTERN.matcher(path); - while (matcher.find()) { - String propertyName = matcher.group(0); - if (!"${project.basedir}".equals(propertyName)) { + final Deque stk = new LinkedList<>(); + + final Supplier pathSupplier = () -> { + final boolean parallel = false; + return StreamSupport.stream(((Iterable) stk::descendingIterator).spliterator(), parallel) + .map(ActivationFrame::location) + .collect(Collectors.joining(".")); + }; + final Supplier locationSupplier = () -> { + if (stk.size() < 2) { + return null; + } + Iterator f = stk.iterator(); + + String location = f.next().location; + ActivationFrame parent = f.next(); + + return parent.parent.map(p -> p.getLocation(location)).orElse(null); + }; + final UnaryOperator transformer = s -> { + if (hasProjectExpression(s)) { + String path = pathSupplier.get(); + Matcher matcher = EXPRESSION_PROJECT_NAME_PATTERN.matcher(s); + while (matcher.find()) { + String propertyName = matcher.group(0); + + if (path.startsWith("activation.file.") && "${project.basedir}".equals(propertyName)) { + continue; + } addViolation( problems, Severity.WARNING, Version.V30, - prefix + "activation.file." + location, + prefix + path, null, - "Failed to interpolate file location " + path + ": " + propertyName + "Failed to interpolate profile activation property " + s + ": " + propertyName + " expressions are not supported during profile activation.", - file.getLocation(location)); + locationSupplier.get()); } } - } + return s; + }; + new ActivationWalker(stk, transformer).transformActivation(activation); } private void validate20RawPlugins( diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java index 6a5a029dbc71..0fd9041929d9 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java @@ -20,6 +20,7 @@ import java.io.InputStream; import java.util.List; +import java.util.Properties; import java.util.function.UnaryOperator; import org.apache.maven.model.Model; @@ -827,24 +828,52 @@ void repositoryWithBasedirExpression() throws Exception { @Test void profileActivationWithAllowedExpression() throws Exception { - SimpleProblemCollector result = validateRaw("raw-model/profile-activation-file-with-allowed-expressions.xml"); + SimpleProblemCollector result = validateRaw( + "raw-model/profile-activation-file-with-allowed-expressions.xml", + mbr -> mbr.setUserProperties(new Properties() { + private static final long serialVersionUID = 1L; + + { + setProperty("foo", "foo"); + setProperty("bar", "foo"); + } + })); assertViolations(result, 0, 0, 0); } @Test - void profileActivationWithProjectExpression() throws Exception { + void profileActivationFileWithProjectExpression() throws Exception { SimpleProblemCollector result = validateRaw("raw-model/profile-activation-file-with-project-expressions.xml"); assertViolations(result, 0, 0, 2); assertEquals( "'profiles.profile[exists-project-version].activation.file.exists' " - + "Failed to interpolate file location ${project.version}/test.txt: " + + "Failed to interpolate profile activation property ${project.version}/test.txt: " + "${project.version} expressions are not supported during profile activation.", result.getWarnings().get(0)); assertEquals( "'profiles.profile[missing-project-version].activation.file.missing' " - + "Failed to interpolate file location ${project.version}/test.txt: " + + "Failed to interpolate profile activation property ${project.version}/test.txt: " + + "${project.version} expressions are not supported during profile activation.", + result.getWarnings().get(1)); + } + + @Test + void profileActivationPropertyWithProjectExpression() throws Exception { + SimpleProblemCollector result = + validateRaw("raw-model/profile-activation-property-with-project-expressions.xml"); + assertViolations(result, 0, 0, 2); + + assertEquals( + "'profiles.profile[property-name-project-version].activation.property.name' " + + "Failed to interpolate profile activation property ${project.version}: " + + "${project.version} expressions are not supported during profile activation.", + result.getWarnings().get(0)); + + assertEquals( + "'profiles.profile[property-value-project-version].activation.property.value' " + + "Failed to interpolate profile activation property ${project.version}: " + "${project.version} expressions are not supported during profile activation.", result.getWarnings().get(1)); } diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml index a4beb6238f4a..72b6747f1835 100644 --- a/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml @@ -60,5 +60,23 @@ under the License. + + dynamic-property-available + + + ${activationProperty} + + + + + + matches-another-property + + + foo + ${bar} + + + diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml new file mode 100644 index 000000000000..8bcf89f66f72 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml @@ -0,0 +1,49 @@ + + + + 4.0.0 + aid + gid + 0.1 + pom + + + + + property-name-project-version + + + ${project.version} + + + + + property-value-project-version + + + project.version + ${project.version} + + + + + +