You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by el...@apache.org on 2026/08/02 14:45:20 UTC
(maven-remote-resources-plugin) 01/01: Fix SCM path normalization edge cases in ModelInheritanceAssembler (#274)
This is an automated email from the ASF dual-hosted git repository.
elharo pushed a commit to branch fix/issue-274-scm-path-normalization
in repository https://gitbox.apache.org/repos/asf/maven-remote-resources-plugin.git
commit 9d8bc6a9623447ca41ca1057c2f301adb0531ebd
Author: Elliotte Rusty Harold <el...@ibiblio.org>
AuthorDate: Sun Aug 2 14:45:13 2026 +0000
Fix SCM path normalization edge cases in ModelInheritanceAssembler (#274)
---
.../remote/AbstractProcessRemoteResourcesMojo.java | 1 +
.../remote/ModelInheritanceAssembler.java | 90 +++++++------
.../remote/ModelInheritanceAssemblerTest.java | 149 +++++++++++++++++++++
3 files changed, 202 insertions(+), 38 deletions(-)
diff --git a/src/main/java/org/apache/maven/plugin/resources/remote/AbstractProcessRemoteResourcesMojo.java b/src/main/java/org/apache/maven/plugin/resources/remote/AbstractProcessRemoteResourcesMojo.java
index 15d3ce3..5731ed8 100644
--- a/src/main/java/org/apache/maven/plugin/resources/remote/AbstractProcessRemoteResourcesMojo.java
+++ b/src/main/java/org/apache/maven/plugin/resources/remote/AbstractProcessRemoteResourcesMojo.java
@@ -1031,6 +1031,7 @@ public abstract class AbstractProcessRemoteResourcesMojo extends AbstractMojo {
}
protected Model mergeModels(Model parent, Model child) {
+ inheritanceAssembler.setLog(getLog());
inheritanceAssembler.assembleModelInheritance(child, parent);
return child;
}
diff --git a/src/main/java/org/apache/maven/plugin/resources/remote/ModelInheritanceAssembler.java b/src/main/java/org/apache/maven/plugin/resources/remote/ModelInheritanceAssembler.java
index 1da5c2e..e59ba48 100644
--- a/src/main/java/org/apache/maven/plugin/resources/remote/ModelInheritanceAssembler.java
+++ b/src/main/java/org/apache/maven/plugin/resources/remote/ModelInheritanceAssembler.java
@@ -18,13 +18,13 @@
*/
package org.apache.maven.plugin.resources.remote;
+import java.nio.file.InvalidPathException;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.LinkedHashMap;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Properties;
-import java.util.StringTokenizer;
import java.util.TreeMap;
import org.apache.maven.model.Build;
@@ -41,6 +41,7 @@ import org.apache.maven.model.Reporting;
import org.apache.maven.model.Resource;
import org.apache.maven.model.Scm;
import org.apache.maven.model.Site;
+import org.apache.maven.plugin.logging.Log;
import org.codehaus.plexus.util.StringUtils;
import org.codehaus.plexus.util.xml.Xpp3Dom;
@@ -48,6 +49,12 @@ import org.codehaus.plexus.util.xml.Xpp3Dom;
* DefaultModelInheritanceAssembler
*/
public class ModelInheritanceAssembler {
+ private Log log;
+
+ public void setLog(Log log) {
+ this.log = log;
+ }
+
// TODO Remove this!
public void assembleBuildInheritance(Build childBuild, Build parentBuild, boolean handleAsInheritance) {
// The build has been set but we want to step in here and fill in
@@ -568,51 +575,58 @@ public class ModelInheritanceAssembler {
uncleanPath = uncleanPath.substring(protocolIdx + 3);
}
- if (uncleanPath.startsWith("/")) {
- cleanedPath += "/";
- }
-
return cleanedPath + resolvePath(uncleanPath);
}
- // TODO Move this to plexus-utils' PathTool.
- private static String resolvePath(String uncleanPath) {
- LinkedList<String> pathElements = new LinkedList<>();
-
- StringTokenizer tokenizer = new StringTokenizer(uncleanPath, "/");
-
- while (tokenizer.hasMoreTokens()) {
- String token = tokenizer.nextToken();
-
- switch (token) {
- case "":
- // Empty path entry ("...//.."), remove.
- break;
- case "..":
- if (pathElements.isEmpty()) {
- // FIXME: somehow report to the user
- // that there are too many '..' elements.
- // For now, ignore the extra '..'.
- } else {
- pathElements.removeLast();
- }
- break;
- default:
- pathElements.addLast(token);
- break;
- }
+ /**
+ * Normalizes the path part of an SCM URL using {@link java.nio.file.Path#normalize()}.
+ * <ul>
+ * <li>Trailing separators are significant (e.g. {@code http://host/repo/} is not
+ * {@code http://host/repo}) and are preserved.</li>
+ * <li>Redundant separators, {@code "."} and resolvable {@code ".."} segments are collapsed.</li>
+ * <li>Excess {@code ".."} segments that would climb above the path root are left to the
+ * normalizer and a warning is emitted instead of silently dropping path elements.</li>
+ * </ul>
+ */
+ private String resolvePath(String uncleanPath) {
+ boolean trailingSeparator = uncleanPath.endsWith("/");
+
+ String resolved;
+ try {
+ resolved = Paths.get(uncleanPath).normalize().toString();
+ } catch (InvalidPathException e) {
+ resolved = uncleanPath;
}
- StringBuilder cleanedPath = new StringBuilder();
+ if (trailingSeparator && !resolved.endsWith("/")) {
+ resolved += "/";
+ }
+
+ warnOnExcessParentDirectory(uncleanPath);
+
+ return resolved;
+ }
- while (!pathElements.isEmpty()) {
- cleanedPath.append(pathElements.removeFirst());
- if (!pathElements.isEmpty()) {
- cleanedPath.append('/');
+ private void warnOnExcessParentDirectory(String uncleanPath) {
+ int depth = 0;
+ int excess = 0;
+
+ for (String segment : uncleanPath.split("/")) {
+ if ("..".equals(segment)) {
+ if (depth == 0) {
+ excess++;
+ } else {
+ depth--;
+ }
+ } else if (!segment.isEmpty() && !".".equals(segment)) {
+ depth++;
}
}
- return cleanedPath.toString();
+ if ((excess > 0) && (log != null)) {
+ log.warn("Path '" + uncleanPath + "' contains " + excess + " excess '..' segment(s) pointing above "
+ + "the path root; the normalized path may not match the original.");
+ }
}
private static void mergeExtensionLists(Build childBuild, Build parentBuild) {
diff --git a/src/test/java/org/apache/maven/plugin/resources/remote/ModelInheritanceAssemblerTest.java b/src/test/java/org/apache/maven/plugin/resources/remote/ModelInheritanceAssemblerTest.java
new file mode 100644
index 0000000..6a73f13
--- /dev/null
+++ b/src/test/java/org/apache/maven/plugin/resources/remote/ModelInheritanceAssemblerTest.java
@@ -0,0 +1,149 @@
+/*
+ * 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.
+ */
+package org.apache.maven.plugin.resources.remote;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.maven.plugin.logging.Log;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Unit tests for the SCM path normalization in {@link ModelInheritanceAssembler}.
+ */
+public class ModelInheritanceAssemblerTest {
+
+ private final ModelInheritanceAssembler assembler = new ModelInheritanceAssembler();
+
+ @Test
+ public void appendPathPreservesTrailingSlash() {
+ assertEquals(
+ "http://svn.example.com/repo/",
+ assembler.appendPath("http://svn.example.com/repo/", null, null, false));
+ }
+
+ @Test
+ public void appendPathAppendsChild() {
+ assertEquals(
+ "http://svn.example.com/repo/child",
+ assembler.appendPath("http://svn.example.com/repo", "child", null, true));
+ }
+
+ @Test
+ public void appendPathCollapsesDotSegment() {
+ assertEquals(
+ "http://svn.example.com/repo/child",
+ assembler.appendPath("http://svn.example.com/repo/./child", null, null, false));
+ }
+
+ @Test
+ public void appendPathResolvesParentDirectory() {
+ assertEquals(
+ "http://svn.example.com/repos",
+ assembler.appendPath("http://svn.example.com/repos/project/..", null, null, false));
+ }
+
+ @Test
+ public void appendPathWarnsOnExcessParentDirectory() {
+ RecordingLog log = new RecordingLog();
+ assembler.setLog(log);
+
+ assembler.appendPath("http://svn.example.com/../../../../repo", null, null, false);
+
+ assertTrue("Expected a warning about excess '..' segments", log.hasWarningContaining(".."));
+ }
+
+ /** Minimal {@link Log} implementation that records emitted warnings. */
+ private static class RecordingLog implements Log {
+
+ private final List<String> warnings = new ArrayList<>();
+
+ boolean hasWarningContaining(String text) {
+ for (String warning : warnings) {
+ if (warning.contains(text)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public boolean isDebugEnabled() {
+ return false;
+ }
+
+ @Override
+ public void debug(CharSequence content) {}
+
+ @Override
+ public void debug(CharSequence content, Throwable error) {}
+
+ @Override
+ public void debug(Throwable error) {}
+
+ @Override
+ public boolean isInfoEnabled() {
+ return false;
+ }
+
+ @Override
+ public void info(CharSequence content) {}
+
+ @Override
+ public void info(CharSequence content, Throwable error) {}
+
+ @Override
+ public void info(Throwable error) {}
+
+ @Override
+ public boolean isWarnEnabled() {
+ return true;
+ }
+
+ @Override
+ public void warn(CharSequence content) {
+ warnings.add(content.toString());
+ }
+
+ @Override
+ public void warn(CharSequence content, Throwable error) {
+ warnings.add(content.toString());
+ }
+
+ @Override
+ public void warn(Throwable error) {}
+
+ @Override
+ public boolean isErrorEnabled() {
+ return false;
+ }
+
+ @Override
+ public void error(CharSequence content) {}
+
+ @Override
+ public void error(CharSequence content, Throwable error) {}
+
+ @Override
+ public void error(Throwable error) {}
+ }
+}