Author: bentmann Date: Sun Jul 18 14:23:06 2010 New Revision: 965233 URL: http://svn.apache.org/viewvc?rev=965233&view=rev Log: [MNG-4717] Repository Ids containing ":" will lead to checksum errors on Windows machines
Added: maven/maven-3/trunk/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml (with props) Modified: maven/maven-3/trunk/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java maven/maven-3/trunk/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java maven/maven-3/trunk/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java maven/maven-3/trunk/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java Modified: maven/maven-3/trunk/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java URL: http://svn.apache.org/viewvc/maven/maven-3/trunk/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java?rev=965233&r1=965232&r2=965233&view=diff ============================================================================== --- maven/maven-3/trunk/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java (original) +++ maven/maven-3/trunk/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java Sun Jul 18 14:23:06 2010 @@ -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 @@ public class DefaultModelValidator { 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(), Modified: maven/maven-3/trunk/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java URL: http://svn.apache.org/viewvc/maven/maven-3/trunk/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java?rev=965233&r1=965232&r2=965233&view=diff ============================================================================== --- maven/maven-3/trunk/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java (original) +++ maven/maven-3/trunk/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java Sun Jul 18 14:23:06 2010 @@ -518,4 +518,21 @@ public class DefaultModelValidatorTest 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" ); + } + } Added: maven/maven-3/trunk/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml URL: http://svn.apache.org/viewvc/maven/maven-3/trunk/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml?rev=965233&view=auto ============================================================================== --- maven/maven-3/trunk/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml (added) +++ maven/maven-3/trunk/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml Sun Jul 18 14:23:06 2010 @@ -0,0 +1,50 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +<project> + <modelVersion>4.0.0</modelVersion> + + <groupId>gid</groupId> + <artifactId>aid</artifactId> + <version>1.0</version> + + <repositories> + <repository> + <id>this/is\bad</id> + <url>http://localhost</url> + </repository> + </repositories> + <pluginRepositories> + <pluginRepository> + <id>this/is\bad</id> + <url>http://localhost</url> + </pluginRepository> + </pluginRepositories> + + <distributionManagement> + <repository> + <id>this/is\bad</id> + <url>http://localhost</url> + </repository> + <snapshotRepository> + <id>this/is\bad</id> + <url>http://localhost</url> + </snapshotRepository> + </distributionManagement> +</project> Propchange: maven/maven-3/trunk/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml ------------------------------------------------------------------------------ svn:eol-style = native Propchange: maven/maven-3/trunk/maven-model-builder/src/test/resources/poms/validation/bad-repository-id.xml ------------------------------------------------------------------------------ svn:keywords = Author Date Id Revision Modified: maven/maven-3/trunk/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java URL: http://svn.apache.org/viewvc/maven/maven-3/trunk/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java?rev=965233&r1=965232&r2=965233&view=diff ============================================================================== --- maven/maven-3/trunk/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java (original) +++ maven/maven-3/trunk/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java Sun Jul 18 14:23:06 2010 @@ -27,6 +27,7 @@ import org.apache.maven.settings.Reposit 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<String> pluginGroups = settings.getPluginGroups(); @@ -58,12 +63,13 @@ public class DefaultSettingsValidator 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 @@ public class DefaultSettingsValidator { 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 @@ public class DefaultSettingsValidator { validateStringNotEmpty( problems, prefix + ".id", repository.getId(), repository.getUrl() ); + validateBannedCharacters( problems, prefix + ".id", Severity.WARNING, repository.getId(), null, + ILLEGAL_REPO_ID_CHARS ); + if ( "local".equals( repository.getId() ) ) { - addWarn( problems, "'" + prefix + ".id' must not be 'local'" + addViolation( problems, Severity.WARNING, 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." ); } @@ -131,8 +143,8 @@ public class DefaultSettingsValidator if ( "legacy".equals( repository.getLayout() ) ) { - addWarn( problems, "'" + prefix + ".layout' for " + repository.getId() - + " uses the deprecated value 'legacy'." ); + addViolation( problems, Severity.WARNING, prefix + ".layout", repository.getId(), + "uses the unsupported value 'legacy', artifact resolution might fail." ); } } } @@ -162,16 +174,7 @@ public class DefaultSettingsValidator return true; } - String msg; - if ( sourceHint != null ) - { - msg = "'" + fieldName + "' is missing for " + sourceHint; - } - else - { - msg = "'" + fieldName + "' is missing."; - } - addError( problems, msg ); + addViolation( problems, Severity.ERROR, fieldName, sourceHint, "is missing" ); return false; } @@ -191,28 +194,45 @@ public class DefaultSettingsValidator return true; } - String msg; - if ( sourceHint != null ) - { - msg = "'" + fieldName + "' is missing for " + sourceHint; - } - else - { - msg = "'" + fieldName + "' is missing."; - } - addError( problems, msg ); + addViolation( problems, Severity.ERROR, fieldName, sourceHint, "is missing" ); return false; } - private void addError( SettingsProblemCollector problems, String msg ) + private boolean validateBannedCharacters( SettingsProblemCollector problems, String fieldName, Severity severity, + String string, String sourceHint, String banned ) { - problems.add( SettingsProblem.Severity.ERROR, msg, -1, -1, null ); + if ( string != null ) + { + for ( int i = string.length() - 1; i >= 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 ); } } Modified: maven/maven-3/trunk/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java URL: http://svn.apache.org/viewvc/maven/maven-3/trunk/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java?rev=965233&r1=965232&r2=965233&view=diff ============================================================================== --- maven/maven-3/trunk/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java (original) +++ maven/maven-3/trunk/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java Sun Jul 18 14:23:06 2010 @@ -56,6 +56,11 @@ public class DefaultSettingsValidatorTes 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 class DefaultSettingsValidatorTes 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