You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by be...@apache.org on 2010/07/18 16:23:06 UTC
svn commit: r965233 - in /maven/maven-3/trunk:
maven-model-builder/src/main/java/org/apache/maven/model/validation/
maven-model-builder/src/test/java/org/apache/maven/model/validation/
maven-model-builder/src/test/resources/poms/validation/ maven-setti...
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