[MNG-3133] DefaultModelInheritence::appendPath assumes it is operating on interpolated/literal paths

git-svn-id: https://svn.apache.org/repos/asf/maven/maven-3/trunk@930411 13f79535-47bb-0310-9956-ffa450edef68

maven-core/src/test/java/org/apache/maven/project/PomConstructionTest.java

@@ -600,7 +600,7 @@ public class PomConstructionTest
     {
         PomTestWrapper pom = buildPom( "url-inheritance/sub" );
         assertEquals( "http://parent.url/child", pom.getValue( "url" ) );
-        assertEquals( "http://parent.url/org/", pom.getValue( "organization/url" ) );
+        assertEquals( "http://parent.url/org", pom.getValue( "organization/url" ) );
         assertEquals( "http://parent.url/license.txt", pom.getValue( "licenses[1]/url" ) );
         assertEquals( "http://parent.url/viewvc/child", pom.getValue( "scm/url" ) );
         assertEquals( "http://parent.url/scm/child", pom.getValue( "scm/connection" ) );
@@ -619,7 +619,7 @@ public class PomConstructionTest
     {
         PomTestWrapper pom = buildPom( "url-inheritance/another-parent/sub" );
         assertEquals( "http://parent.url/ap/child", pom.getValue( "url" ) );
-        assertEquals( "http://parent.url/org/", pom.getValue( "organization/url" ) );
+        assertEquals( "http://parent.url/org", pom.getValue( "organization/url" ) );
         assertEquals( "http://parent.url/license.txt", pom.getValue( "licenses[1]/url" ) );
         assertEquals( "http://parent.url/viewvc/ap/child", pom.getValue( "scm/url" ) );
         assertEquals( "http://parent.url/scm/ap/child", pom.getValue( "scm/connection" ) );

maven-core/src/test/resources-project-builder/unc-path/pom.xml

@@ -35,7 +35,7 @@
   <distributionManagement>
     <site>
       <id>site</id>
-      <url>file:////host/site/</url>
+      <url>file:////host/site</url>
     </site>
   </distributionManagement>
 </project>

maven-core/src/test/resources-project-builder/url-inheritance/pom.xml

@@ -32,10 +32,10 @@ under the License.
     Test that inheritance of certain URLs automatically appends the child's artifact id.
   </description>
 
-  <url>http://parent.url/</url>
+  <url>http://parent.url</url>
   <organization>
     <name>parent-org</name>
-    <url>http://parent.url/org/</url>
+    <url>http://parent.url/org</url>
   </organization>
   <licenses>
     <license>

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java

@@ -45,6 +45,7 @@ import org.apache.maven.model.management.DependencyManagementInjector;
 import org.apache.maven.model.management.PluginManagementInjector;
 import org.apache.maven.model.normalization.ModelNormalizer;
 import org.apache.maven.model.path.ModelPathTranslator;
+import org.apache.maven.model.path.ModelUrlNormalizer;
 import org.apache.maven.model.plugin.LifecycleBindingsInjector;
 import org.apache.maven.model.plugin.PluginConfigurationExpander;
 import org.apache.maven.model.plugin.ReportConfigurationExpander;
@@ -83,6 +84,9 @@ public class DefaultModelBuilder
     @Requirement
     private ModelPathTranslator modelPathTranslator;
 
+    @Requirement
+    private ModelUrlNormalizer modelUrlNormalizer;
+
     @Requirement
     private SuperPomProvider superPomProvider;
 
@@ -225,6 +229,8 @@ public class DefaultModelBuilder
         resultModel = interpolateModel( resultModel, request, problems );
         resultData.setModel( resultModel );
 
+        modelUrlNormalizer.normalize( resultModel, request );
+
         resultData.setGroupId( resultModel.getGroupId() );
         resultData.setArtifactId( resultModel.getArtifactId() );
         resultData.setVersion( resultModel.getVersion() );

maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java

@@ -24,6 +24,7 @@ import org.apache.maven.model.building.ModelBuildingRequest;
 import org.apache.maven.model.building.ModelProblemCollector;
 import org.apache.maven.model.building.ModelProblem.Severity;
 import org.apache.maven.model.path.PathTranslator;
+import org.apache.maven.model.path.UrlNormalizer;
 import org.codehaus.plexus.component.annotations.Requirement;
 import org.codehaus.plexus.interpolation.AbstractValueSource;
 import org.codehaus.plexus.interpolation.InterpolationException;
@@ -95,6 +96,9 @@ public abstract class AbstractStringBasedModelInterpolator
     @Requirement
     private PathTranslator pathTranslator;
 
+    @Requirement
+    private UrlNormalizer urlNormalizer;
+
     private Interpolator interpolator;
 
     private RecursionInterceptor recursionInterceptor;
@@ -187,16 +191,14 @@ public abstract class AbstractStringBasedModelInterpolator
                                                                                final File projectDir,
                                                                                final ModelBuildingRequest config )
     {
+        List<InterpolationPostProcessor> processors = new ArrayList<InterpolationPostProcessor>( 2 );
         if ( projectDir != null )
         {
-            return Collections.singletonList( new PathTranslatingPostProcessor( PROJECT_PREFIXES,
+            processors.add( new PathTranslatingPostProcessor( PROJECT_PREFIXES, TRANSLATED_PATH_EXPRESSIONS,
-                                                                                TRANSLATED_PATH_EXPRESSIONS,
                                                               projectDir, pathTranslator ) );
         }
-        else
+        processors.add( new UrlNormalizingPostProcessor( urlNormalizer ) );
-        {
+        return processors;
-            return Collections.emptyList();
-        }
     }
 
     protected String interpolateInternal( String src, List<? extends ValueSource> valueSources,

maven-model-builder/src/main/java/org/apache/maven/model/interpolation/PathTranslatingPostProcessor.java

@@ -60,7 +60,7 @@ class PathTranslatingPostProcessor
             }
         }
 
-        return value;
+        return null;
     }
 
 }

maven-model-builder/src/main/java/org/apache/maven/model/interpolation/UrlNormalizingPostProcessor.java

@@ -0,0 +1,68 @@
+package org.apache.maven.model.interpolation;
+
+/*
+ * 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.
+ */
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.maven.model.path.UrlNormalizer;
+import org.codehaus.plexus.interpolation.InterpolationPostProcessor;
+
+/**
+ * Ensures that expressions referring to URLs evaluate to normalized URLs.
+ * 
+ * @author Benjamin Bentmann
+ */
+class UrlNormalizingPostProcessor
+    implements InterpolationPostProcessor
+{
+
+    private static final Set<String> urlExpressions;
+
+    static
+    {
+        Set<String> expressions = new HashSet<String>();
+        expressions.add( "project.url" );
+        expressions.add( "project.scm.url" );
+        expressions.add( "project.scm.connection" );
+        expressions.add( "project.scm.developerConnection" );
+        expressions.add( "project.distributionManagement.site.url" );
+
+        urlExpressions = expressions;
+    }
+
+    private UrlNormalizer normalizer;
+
+    public UrlNormalizingPostProcessor( UrlNormalizer normalizer )
+    {
+        this.normalizer = normalizer;
+    }
+
+    public Object execute( String expression, Object value )
+    {
+        if ( value != null && urlExpressions.contains( expression ) )
+        {
+            return normalizer.normalize( value.toString() );
+        }
+
+        return null;
+    }
+
+}

maven-model-builder/src/main/java/org/apache/maven/model/merge/MavenModelMerger.java

@@ -22,11 +22,9 @@ package org.apache.maven.model.merge;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.StringTokenizer;
 
 import org.apache.maven.model.BuildBase;
 import org.apache.maven.model.CiManagement;
@@ -560,80 +558,39 @@ public class MavenModelMerger
 
     private String appendPath( String parentPath, String childPath, String pathAdjustment )
     {
-        String uncleanPath = parentPath;
+        String path = parentPath;
-
+        path = concatPath( path, pathAdjustment );
-        if ( pathAdjustment != null && pathAdjustment.length() > 0 )
+        path = concatPath( path, childPath );
-        {
+        return path;
-            uncleanPath += "/" + pathAdjustment;
     }
 
-        if ( childPath != null )
+    private String concatPath( String base, String path )
     {
-            uncleanPath += "/" + childPath;
+        String result = base;
+
+        if ( path != null && path.length() > 0 )
+        {
+            if ( ( result.endsWith( "/" ) && !path.startsWith( "/" ) )
+                || ( !result.endsWith( "/" ) && path.startsWith( "/" ) ) )
+            {
+                result += path;
             }
-
+            else if ( result.endsWith( "/" ) && path.startsWith( "/" ) )
-        String cleanedPath = "";
-
-        int protocolIdx = uncleanPath.indexOf( "://" );
-
-        if ( protocolIdx > -1 )
             {
-            cleanedPath = uncleanPath.substring( 0, protocolIdx + 3 );
+                result += path.substring( 1 );
-            uncleanPath = uncleanPath.substring( protocolIdx + 3 );
-        }
-
-        if ( uncleanPath.startsWith( "//" ) )
-        {
-            // preserve leading double slash for UNC paths like "file:////host/pom.xml"
-            cleanedPath += "//";
-        }
-        else if ( uncleanPath.startsWith( "/" ) )
-        {
-            cleanedPath += "/";
-        }
-
-        return cleanedPath + resolvePath( uncleanPath );
-    }
-
-    private String resolvePath( String uncleanPath )
-    {
-        LinkedList<String> pathElements = new LinkedList<String>();
-
-        StringTokenizer tokenizer = new StringTokenizer( uncleanPath, "/" );
-
-        while ( tokenizer.hasMoreTokens() )
-        {
-            String token = tokenizer.nextToken();
-
-            if ( token.equals( "" ) )
-            {
-                // Empty path entry ("...//.."), remove.
-            }
-            else if ( token.equals( ".." ) )
-            {
-                if ( !pathElements.isEmpty() )
-                {
-                    pathElements.removeLast();
-                }
             }
             else
             {
-                pathElements.addLast( token );
+                result += '/';
+                result += path;
             }
-        }
+            if ( base.endsWith( "/" ) && !result.endsWith( "/" ) )
-
-        StringBuilder cleanedPath = new StringBuilder( 128 );
-
-        while ( !pathElements.isEmpty() )
             {
-            cleanedPath.append( pathElements.removeFirst() );
+                result += '/';
-            if ( !pathElements.isEmpty() )
-            {
-                cleanedPath.append( '/' );
             }
         }
 
-        return cleanedPath.toString();
+        return result;
     }
 
 }

maven-model-builder/src/main/java/org/apache/maven/model/path/DefaultModelUrlNormalizer.java

@@ -0,0 +1,77 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.DistributionManagement;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.Scm;
+import org.apache.maven.model.Site;
+import org.apache.maven.model.building.ModelBuildingRequest;
+import org.codehaus.plexus.component.annotations.Component;
+import org.codehaus.plexus.component.annotations.Requirement;
+
+/**
+ * Normalizes URLs to remove the ugly parent references "../" that got potentially inserted by URL adjustment during
+ * model inheritance.
+ * 
+ * @author Benjamin Bentmann
+ */
+@Component( role = ModelUrlNormalizer.class )
+public class DefaultModelUrlNormalizer
+    implements ModelUrlNormalizer
+{
+
+    @Requirement
+    private UrlNormalizer urlNormalizer;
+
+    public void normalize( Model model, ModelBuildingRequest request )
+    {
+        if ( model == null )
+        {
+            return;
+        }
+
+        model.setUrl( normalize( model.getUrl() ) );
+
+        Scm scm = model.getScm();
+        if ( scm != null )
+        {
+            scm.setUrl( normalize( scm.getUrl() ) );
+            scm.setConnection( normalize( scm.getConnection() ) );
+            scm.setDeveloperConnection( normalize( scm.getDeveloperConnection() ) );
+        }
+
+        DistributionManagement dist = model.getDistributionManagement();
+        if ( dist != null )
+        {
+            Site site = dist.getSite();
+            if ( site != null )
+            {
+                site.setUrl( normalize( site.getUrl() ) );
+            }
+        }
+    }
+
+    private String normalize( String url )
+    {
+        return urlNormalizer.normalize( url );
+    }
+
+}

maven-model-builder/src/main/java/org/apache/maven/model/path/DefaultUrlNormalizer.java

@@ -0,0 +1,61 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+import org.codehaus.plexus.component.annotations.Component;
+
+/**
+ * Normalizes a URL.
+ * 
+ * @author Benjamin Bentmann
+ */
+@Component( role = UrlNormalizer.class )
+public class DefaultUrlNormalizer
+    implements UrlNormalizer
+{
+
+    public String normalize( String url )
+    {
+        String result = url;
+
+        if ( result != null )
+        {
+            result = result.replaceAll( "(?<![:/])/+", "/" );
+
+            while ( true )
+            {
+                int idx = result.indexOf( "/../" );
+                if ( idx <= 0 )
+                {
+                    break;
+                }
+                int parent = result.lastIndexOf( '/', idx - 1 );
+                if ( parent < 0 )
+                {
+                    break;
+                }
+                result = result.substring( 0, parent ) + result.substring( idx + 3 );
+            }
+        }
+
+        return result;
+    }
+
+}

maven-model-builder/src/main/java/org/apache/maven/model/path/ModelUrlNormalizer.java

@@ -0,0 +1,42 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.model.Model;
+import org.apache.maven.model.building.ModelBuildingRequest;
+
+/**
+ * Normalizes URLs to remove the ugly parent references "../" that got potentially inserted by URL adjustment during
+ * model inheritance.
+ * 
+ * @author Benjamin Bentmann
+ */
+public interface ModelUrlNormalizer
+{
+
+    /**
+     * Normalizes the well-known URLs of the specified model.
+     * 
+     * @param model The model whose URLs should be normalized, may be {@code null}.
+     * @param request The model building request that holds further settings, must not be {@code null}.
+     */
+    void normalize( Model model, ModelBuildingRequest request );
+
+}

maven-model-builder/src/main/java/org/apache/maven/model/path/UrlNormalizer.java

@@ -0,0 +1,39 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+/**
+ * Normalizes a URL to remove the ugly parent references "../" that got potentially inserted by URL adjustment during
+ * model inheritance.
+ * 
+ * @author Benjamin Bentmann
+ */
+public interface UrlNormalizer
+{
+
+    /**
+     * Normalizes the specified URL.
+     * 
+     * @param url The URL to normalize, may be {@code null}.
+     * @return The normalized URL or {@code null} if the input was {@code null}.
+     */
+    String normalize( String url );
+
+}

maven-model-builder/src/test/java/org/apache/maven/model/path/DefaultUrlNormalizerTest.java

@@ -0,0 +1,84 @@
+package org.apache.maven.model.path;
+
+/*
+ * 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.
+ */
+
+import junit.framework.TestCase;
+
+/**
+ * @author Benjamin Bentmann
+ */
+public class DefaultUrlNormalizerTest
+    extends TestCase
+{
+
+    private UrlNormalizer normalizer;
+
+    @Override
+    protected void setUp()
+        throws Exception
+    {
+        super.setUp();
+
+        normalizer = new DefaultUrlNormalizer();
+    }
+
+    @Override
+    protected void tearDown()
+        throws Exception
+    {
+        normalizer = null;
+
+        super.tearDown();
+    }
+
+    private String normalize( String url )
+    {
+        return normalizer.normalize( url );
+    }
+
+    public void testNullSafe()
+    {
+        assertNull( normalize( null ) );
+    }
+
+    public void testTrailingSlash()
+    {
+        assertEquals( "", normalize( "" ) );
+        assertEquals( "http://server.org/dir", normalize( "http://server.org/dir" ) );
+        assertEquals( "http://server.org/dir/", normalize( "http://server.org/dir/" ) );
+    }
+
+    public void testRemovalOfParentRefs()
+    {
+        assertEquals( "http://server.org/child", normalize( "http://server.org/parent/../child" ) );
+        assertEquals( "http://server.org/child", normalize( "http://server.org/grand/parent/../../child" ) );
+    }
+
+    public void testRemovalOfDoubleSlashes()
+    {
+        assertEquals( "http://server.org/dir/", normalize( "http://server.org/dir//" ) );
+        assertEquals( "http://server.org/parent/child", normalize( "http://server.org/parent//child" ) );
+
+        assertEquals( "file:////UNC/server", normalize( "file:////UNC/server" ) );
+        assertEquals( "[fetch=]http://server.org/[push=]ssh://server.org/",
+                      normalize( "[fetch=]http://server.org/[push=]ssh://server.org/" ) );
+    }
+
+}