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


Reply via email to