This is an automated email from the ASF dual-hosted git repository. hboutemy pushed a commit to branch MNG-7107 in repository https://gitbox.apache.org/repos/asf/maven.git
commit 2b919c9bcd8881eefefe98071b28c01404929904 Author: Hervé Boutemy <hbout...@apache.org> AuthorDate: Fri Feb 26 08:25:22 2021 +0100 [MNG-7107] relax profile id validation, different from coordinate id --- .../model/validation/DefaultModelValidator.java | 90 ++++++++++++++++------ .../validation/DefaultModelValidatorTest.java | 18 +++-- ...-ids-pom.xml => invalid-coordinate-ids-pom.xml} | 0 ...alid-profile-id.xml => invalid-profile-ids.xml} | 15 ++++ 4 files changed, 92 insertions(+), 31 deletions(-) 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 ba812b5..51c7e57 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 @@ -85,7 +85,9 @@ public class DefaultModelValidator private static final String EMPTY = ""; - private final Set<String> validIds = new HashSet<>(); + private final Set<String> validCoordinateIds = new HashSet<>(); + + private final Set<String> validProfileIds = new HashSet<>(); @Override public void validateFileModel( Model m, ModelBuildingRequest request, ModelProblemCollector problems ) @@ -190,7 +192,7 @@ public class DefaultModelValidator { String prefix = "profiles.profile[" + profile.getId() + "]."; - validateId( prefix, "id", problems, Severity.ERROR, Version.V40, profile.getId(), null, m ); + validateProfileId( prefix, "id", problems, Severity.ERROR, Version.V40, profile.getId(), null, m ); if ( !profileIds.add( profile.getId() ) ) { @@ -358,9 +360,9 @@ public class DefaultModelValidator { validateStringNotEmpty( "modelVersion", problems, Severity.ERROR, Version.BASE, m.getModelVersion(), m ); - validateId( "groupId", problems, m.getGroupId(), m ); + validateCoordinateId( "groupId", problems, m.getGroupId(), m ); - validateId( "artifactId", problems, m.getArtifactId(), m ); + validateCoordinateId( "artifactId", problems, m.getArtifactId(), m ); validateStringNotEmpty( "packaging", problems, Severity.ERROR, Version.BASE, m.getPackaging(), m ); @@ -668,10 +670,10 @@ public class DefaultModelValidator private void validateEffectiveDependency( ModelProblemCollector problems, Dependency d, boolean management, String prefix, ModelBuildingRequest request ) { - validateId( prefix, "artifactId", problems, Severity.ERROR, Version.BASE, d.getArtifactId(), + validateCoordinateId( prefix, "artifactId", problems, Severity.ERROR, Version.BASE, d.getArtifactId(), d.getManagementKey(), d ); - validateId( prefix, "groupId", problems, Severity.ERROR, Version.BASE, d.getGroupId(), + validateCoordinateId( prefix, "groupId", problems, Severity.ERROR, Version.BASE, d.getGroupId(), d.getManagementKey(), d ); if ( !management ) @@ -727,11 +729,11 @@ public class DefaultModelValidator { if ( request.getValidationLevel() < ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0 ) { - validateId( prefix, "exclusions.exclusion.groupId", problems, Severity.WARNING, Version.V20, - exclusion.getGroupId(), d.getManagementKey(), exclusion ); + validateCoordinateId( prefix, "exclusions.exclusion.groupId", problems, Severity.WARNING, + Version.V20, exclusion.getGroupId(), d.getManagementKey(), exclusion ); - validateId( prefix, "exclusions.exclusion.artifactId", problems, Severity.WARNING, Version.V20, - exclusion.getArtifactId(), d.getManagementKey(), exclusion ); + validateCoordinateId( prefix, "exclusions.exclusion.artifactId", problems, Severity.WARNING, + Version.V20, exclusion.getArtifactId(), d.getManagementKey(), exclusion ); } else { @@ -830,17 +832,18 @@ public class DefaultModelValidator // Field validation // ---------------------------------------------------------------------- - private boolean validateId( String fieldName, ModelProblemCollector problems, String id, - InputLocationTracker tracker ) + private boolean validateCoordinateId( String fieldName, ModelProblemCollector problems, String id, + InputLocationTracker tracker ) { - return validateId( EMPTY, fieldName, problems, Severity.ERROR, Version.BASE, id, null, tracker ); + return validateCoordinateId( EMPTY, fieldName, problems, Severity.ERROR, Version.BASE, id, null, tracker ); } @SuppressWarnings( "checkstyle:parameternumber" ) - private boolean validateId( String prefix, String fieldName, ModelProblemCollector problems, Severity severity, - Version version, String id, String sourceHint, InputLocationTracker tracker ) + private boolean validateCoordinateId( String prefix, String fieldName, ModelProblemCollector problems, + Severity severity, Version version, String id, String sourceHint, + InputLocationTracker tracker ) { - if ( validIds.contains( id ) ) + if ( validCoordinateIds.contains( id ) ) { return true; } @@ -850,23 +853,23 @@ public class DefaultModelValidator } else { - if ( !isValidId( id ) ) + if ( !isValidCoordinateId( id ) ) { addViolation( problems, severity, version, prefix + fieldName, sourceHint, - "with value '" + id + "' does not match a valid id pattern.", tracker ); + "with value '" + id + "' does not match a valid coordinate id pattern.", tracker ); return false; } - validIds.add( id ); + validCoordinateIds.add( id ); return true; } } - private boolean isValidId( String id ) + private boolean isValidCoordinateId( String id ) { for ( int i = 0; i < id.length(); i++ ) { char c = id.charAt( i ); - if ( !isValidIdCharacter( c ) ) + if ( !isValidCoordinateIdCharacter( c ) ) { return false; } @@ -874,13 +877,52 @@ public class DefaultModelValidator return true; } - - private boolean isValidIdCharacter( char c ) + private boolean isValidCoordinateIdCharacter( char c ) { return c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-' || c == '_' || c == '.'; } @SuppressWarnings( "checkstyle:parameternumber" ) + private boolean validateProfileId( String prefix, String fieldName, ModelProblemCollector problems, + Severity severity, Version version, String id, String sourceHint, + InputLocationTracker tracker ) + { + if ( validProfileIds.contains( id ) ) + { + return true; + } + if ( !validateStringNotEmpty( prefix, fieldName, problems, severity, version, id, sourceHint, tracker ) ) + { + return false; + } + else + { + if ( !isValidProfileId( id ) ) + { + addViolation( problems, severity, version, prefix + fieldName, sourceHint, + "with value '" + id + "' does not match a valid profile id pattern.", tracker ); + return false; + } + validProfileIds.add( id ); + return true; + } + } + + private boolean isValidProfileId( String id ) + { + switch ( id.charAt( 0 ) ) + { // avoid first character that has special CLI meaning in "mvn -P xxx" + case '+': // activate + case '-': // deactivate + case '!': // deactivate + case '?': // optional + return false; + default: + } + return true; + } + + @SuppressWarnings( "checkstyle:parameternumber" ) private boolean validateIdWithWildcards( String prefix, String fieldName, ModelProblemCollector problems, Severity severity, Version version, String id, String sourceHint, InputLocationTracker tracker ) @@ -916,7 +958,7 @@ public class DefaultModelValidator private boolean isValidIdWithWildCardCharacter( char c ) { - return isValidIdCharacter( c ) || c == '?' || c == '*'; + return isValidCoordinateIdCharacter( c ) || c == '?' || c == '*'; } private boolean validateStringNoExpression( String fieldName, ModelProblemCollector problems, Severity severity, 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 d73dcb1..5de48c4 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 @@ -166,16 +166,17 @@ public class DefaultModelValidatorTest } @Test - public void testInvalidIds() + public void testInvalidCoordinateIds() throws Exception { - SimpleProblemCollector result = validate( "invalid-ids-pom.xml" ); + SimpleProblemCollector result = validate( "invalid-coordinate-ids-pom.xml" ); assertViolations( result, 0, 2, 0 ); - assertEquals( "'groupId' with value 'o/a/m' does not match a valid id pattern.", result.getErrors().get( 0 ) ); + assertEquals( "'groupId' with value 'o/a/m' does not match a valid coordinate id pattern.", + result.getErrors().get( 0 ) ); - assertEquals( "'artifactId' with value 'm$-do$' does not match a valid id pattern.", + assertEquals( "'artifactId' with value 'm$-do$' does not match a valid coordinate id pattern.", result.getErrors().get( 1 ) ); } @@ -406,11 +407,14 @@ public class DefaultModelValidatorTest public void testInvalidProfileId() throws Exception { - SimpleProblemCollector result = validateRaw( "invalid-profile-id.xml" ); + SimpleProblemCollector result = validateRaw( "invalid-profile-ids.xml" ); - assertViolations( result, 0, 1, 0 ); + assertViolations( result, 0, 4, 0 ); - assertTrue( result.getErrors().get( 0 ).contains( "?invalid-id" ) ); + assertTrue( result.getErrors().get( 0 ).contains( "+invalid-id" ) ); + assertTrue( result.getErrors().get( 1 ).contains( "-invalid-id" ) ); + assertTrue( result.getErrors().get( 2 ).contains( "!invalid-id" ) ); + assertTrue( result.getErrors().get( 3 ).contains( "?invalid-id" ) ); } public void testDuplicateProfileId() diff --git a/maven-model-builder/src/test/resources/poms/validation/invalid-ids-pom.xml b/maven-model-builder/src/test/resources/poms/validation/invalid-coordinate-ids-pom.xml similarity index 100% rename from maven-model-builder/src/test/resources/poms/validation/invalid-ids-pom.xml rename to maven-model-builder/src/test/resources/poms/validation/invalid-coordinate-ids-pom.xml diff --git a/maven-model-builder/src/test/resources/poms/validation/invalid-profile-id.xml b/maven-model-builder/src/test/resources/poms/validation/invalid-profile-ids.xml similarity index 75% rename from maven-model-builder/src/test/resources/poms/validation/invalid-profile-id.xml rename to maven-model-builder/src/test/resources/poms/validation/invalid-profile-ids.xml index 6e96026..74b670b 100644 --- a/maven-model-builder/src/test/resources/poms/validation/invalid-profile-id.xml +++ b/maven-model-builder/src/test/resources/poms/validation/invalid-profile-ids.xml @@ -26,7 +26,22 @@ under the License. <profiles> <profile> + <id>+invalid-id</id> + </profile> + <profile> + <id>-invalid-id</id> + </profile> + <profile> + <id>!invalid-id</id> + </profile> + <profile> <id>?invalid-id</id> </profile> + <profile> + <id>valid-id</id> + </profile> + <profile> + <id>valid?-jdk9+!</id> + </profile> </profiles> </project>