From 85ef5598599491fe07872257a001f08a0dde3ce8 Mon Sep 17 00:00:00 2001 From: Benjamin Bentmann Date: Sun, 18 Jul 2010 14:23:06 +0000 Subject: [PATCH] [MNG-4717] Repository Ids containing ":" will lead to checksum errors on Windows machines git-svn-id: https://svn.apache.org/repos/asf/maven/maven-3/trunk@965233 13f79535-47bb-0310-9956-ffa450edef68 --- .../validation/DefaultModelValidator.java | 13 ++- .../validation/DefaultModelValidatorTest.java | 17 ++++ .../poms/validation/bad-repository-id.xml | 50 +++++++++++ .../validation/DefaultSettingsValidator.java | 84 ++++++++++++------- .../DefaultSettingsValidatorTest.java | 37 +++++--- 5 files changed, 155 insertions(+), 46 deletions(-) create mode 100644 maven-model-builder/src/test/resources/poms/validation/bad-repository-id.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 e30cb9cb08..2568482c6e 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 @@ -61,7 +61,11 @@ public class DefaultModelValidator private static final String ID_REGEX = "[A-Za-z0-9_\\-.]+"; - private static final String ILLEGAL_VERSION_CHARS = "\\/:\"<>|?*"; + private static final String ILLEGAL_FS_CHARS = "\\/:\"<>|?*"; + + private static final String ILLEGAL_VERSION_CHARS = ILLEGAL_FS_CHARS; + + private static final String ILLEGAL_REPO_ID_CHARS = ILLEGAL_FS_CHARS; public void validateRawModel( Model model, ModelBuildingRequest request, ModelProblemCollector problems ) { @@ -524,13 +528,18 @@ private void validateRepository( ModelProblemCollector problems, Repository repo { if ( repository != null ) { + Severity errOn31 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 ); + + validateBannedCharacters( prefix + ".id", problems, errOn31, repository.getId(), null, repository, + ILLEGAL_REPO_ID_CHARS ); + if ( "local".equals( repository.getId() ) ) { - Severity errOn31 = getSeverity( request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1 ); addViolation( problems, errOn31, prefix + ".id", null, "must not be 'local'" + ", this identifier is reserved for the local repository" + ", using it for other repositories will corrupt your repository metadata.", repository ); } + if ( "legacy".equals( repository.getLayout() ) ) { addViolation( problems, Severity.WARNING, prefix + ".layout", repository.getId(), 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 e8d9c42807..f857093ef1 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 @@ -518,4 +518,21 @@ public void testBadVersion() assertContains( result.getWarnings().get( 0 ), "'version' must not contain any of these characters" ); } + public void testBadRepositoryId() + throws Exception + { + SimpleProblemCollector result = validate( "bad-repository-id.xml" ); + + assertViolations( result, 0, 0, 4 ); + + assertContains( result.getWarnings().get( 0 ), + "'repositories.repository.id' must not contain any of these characters" ); + assertContains( result.getWarnings().get( 1 ), + "'pluginRepositories.pluginRepository.id' must not contain any of these characters" ); + assertContains( result.getWarnings().get( 2 ), + "'distributionManagement.repository.id' must not contain any of these characters" ); + assertContains( result.getWarnings().get( 3 ), + "'distributionManagement.snapshotRepository.id' must not contain any of these characters" ); + } + } diff --git a/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml b/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml new file mode 100644 index 0000000000..70553eb3fd --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml @@ -0,0 +1,50 @@ + + + + 4.0.0 + + gid + aid + 1.0 + + + + this/is\bad + http://localhost + + + + + this/is\bad + http://localhost + + + + + + this/is\bad + http://localhost + + + this/is\bad + http://localhost + + + diff --git a/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java b/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java index 3c4b88581e..27c4308432 100644 --- a/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java +++ b/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java @@ -27,6 +27,7 @@ import org.apache.maven.settings.Server; import org.apache.maven.settings.Settings; import org.apache.maven.settings.building.SettingsProblem; +import org.apache.maven.settings.building.SettingsProblem.Severity; import org.apache.maven.settings.building.SettingsProblemCollector; import org.codehaus.plexus.component.annotations.Component; import org.codehaus.plexus.util.StringUtils; @@ -41,11 +42,15 @@ public class DefaultSettingsValidator private static final String ID_REGEX = "[A-Za-z0-9_\\-.]+"; + private static final String ILLEGAL_FS_CHARS = "\\/:\"<>|?*"; + + private static final String ILLEGAL_REPO_ID_CHARS = ILLEGAL_FS_CHARS; + public void validate( Settings settings, SettingsProblemCollector problems ) { if ( settings.isUsePluginRegistry() ) { - addWarn( problems, "'usePluginRegistry' is deprecated and has no effect." ); + addViolation( problems, Severity.WARNING, "usePluginRegistry", null, "is deprecated and has no effect." ); } List pluginGroups = settings.getPluginGroups(); @@ -58,12 +63,13 @@ public void validate( Settings settings, SettingsProblemCollector problems ) if ( StringUtils.isBlank( pluginGroup ) ) { - addError( problems, "'pluginGroups.pluginGroup[" + i + "]' must not be empty." ); + addViolation( problems, Severity.ERROR, "pluginGroups.pluginGroup[" + i + "]", null, + "must not be empty" ); } else if ( !pluginGroup.matches( ID_REGEX ) ) { - addError( problems, "'pluginGroups.pluginGroup[" + i - + "]' must denote a valid group id and match the pattern " + ID_REGEX ); + addViolation( problems, Severity.ERROR, "pluginGroups.pluginGroup[" + i + "]", null, + "must denote a valid group id and match the pattern " + ID_REGEX ); } } } @@ -88,9 +94,12 @@ else if ( !pluginGroup.matches( ID_REGEX ) ) { validateStringNotEmpty( problems, "mirrors.mirror.id", mirror.getId(), mirror.getUrl() ); + validateBannedCharacters( problems, "mirrors.mirror.id", Severity.WARNING, mirror.getId(), null, + ILLEGAL_REPO_ID_CHARS ); + if ( "local".equals( mirror.getId() ) ) { - addWarn( problems, "'mirrors.mirror.id' must not be 'local'" + addViolation( problems, Severity.WARNING, "mirrors.mirror.id", null, "must not be 'local'" + ", this identifier is reserved for the local repository" + ", using it for other repositories will corrupt your repository metadata." ); } @@ -120,9 +129,12 @@ private void validateRepositories( SettingsProblemCollector problems, List= 0; i-- ) + { + if ( banned.indexOf( string.charAt( i ) ) >= 0 ) + { + addViolation( problems, severity, fieldName, sourceHint, + "must not contain any of these characters " + banned + " but found " + + string.charAt( i ) ); + return false; + } + } + } + + return true; } - private void addWarn( SettingsProblemCollector problems, String msg ) + private void addViolation( SettingsProblemCollector problems, Severity severity, String fieldName, + String sourceHint, String message ) { - problems.add( SettingsProblem.Severity.WARNING, msg, -1, -1, null ); + StringBuilder buffer = new StringBuilder( 256 ); + buffer.append( '\'' ).append( fieldName ).append( '\'' ); + + if ( sourceHint != null ) + { + buffer.append( " for " ).append( sourceHint ); + } + + buffer.append( ' ' ).append( message ); + + problems.add( severity, buffer.toString(), -1, -1, null ); } } diff --git a/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java b/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java index d35ae418c6..073966c49a 100644 --- a/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java +++ b/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java @@ -56,6 +56,11 @@ protected void tearDown() super.tearDown(); } + private void assertContains( String msg, String substring ) + { + assertTrue( "\"" + substring + "\" was not found in: " + msg, msg.contains( substring ) ); + } + public void testValidate() { Settings model = new Settings(); @@ -86,37 +91,45 @@ public void testValidate() public void testValidateMirror() throws Exception { + Settings settings = new Settings(); Mirror mirror = new Mirror(); mirror.setId( "local" ); - Settings settings = new Settings(); + settings.addMirror( mirror ); + mirror = new Mirror(); + mirror.setId( "illegal\\:/chars" ); + mirror.setUrl( "http://void" ); + mirror.setMirrorOf( "void" ); settings.addMirror( mirror ); SimpleProblemCollector problems = new SimpleProblemCollector(); validator.validate( settings, problems ); - assertEquals( 3, problems.messages.size() ); - assertTrue( problems.messages.get( 0 ), problems.messages.get( 0 ).contains( "'mirrors.mirror.id' must not be 'local'" ) ); - assertTrue( problems.messages.get( 1 ), problems.messages.get( 1 ).contains( "'mirrors.mirror.url' is missing" ) ); - assertTrue( problems.messages.get( 2 ), - problems.messages.get( 2 ).contains( "'mirrors.mirror.mirrorOf' is missing" ) ); + assertEquals( 4, problems.messages.size() ); + assertContains( problems.messages.get( 0 ), "'mirrors.mirror.id' must not be 'local'" ); + assertContains( problems.messages.get( 1 ), "'mirrors.mirror.url' for local is missing" ); + assertContains( problems.messages.get( 2 ), "'mirrors.mirror.mirrorOf' for local is missing" ); + assertContains( problems.messages.get( 3 ), "'mirrors.mirror.id' must not contain any of these characters" ); } public void testValidateRepository() throws Exception { + Profile profile = new Profile(); Repository repo = new Repository(); repo.setId( "local" ); - Profile profile = new Profile(); + profile.addRepository( repo ); + repo = new Repository(); + repo.setId( "illegal\\:/chars" ); + repo.setUrl( "http://void" ); profile.addRepository( repo ); Settings settings = new Settings(); settings.addProfile( profile ); SimpleProblemCollector problems = new SimpleProblemCollector(); validator.validate( settings, problems ); - assertEquals( 2, problems.messages.size() ); - assertTrue( problems.messages.get( 0 ), - problems.messages.get( 0 ).contains( "'repositories.repository.id' must not be 'local'" ) ); - assertTrue( problems.messages.get( 1 ), - problems.messages.get( 1 ).contains( "'repositories.repository.url' is missing" ) ); + assertEquals( 3, problems.messages.size() ); + assertContains( problems.messages.get( 0 ), "'repositories.repository.id' must not be 'local'" ); + assertContains( problems.messages.get( 1 ), "'repositories.repository.url' for local is missing" ); + assertContains( problems.messages.get( 2 ), "'repositories.repository.id' must not contain any of these characters" ); } private static class SimpleProblemCollector