This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new a8833e7f5 GH-1609 Bad metadata naming (#1610)
a8833e7f5 is described below
commit a8833e7f541b8984d774cfcded64fe0cf646a7bc
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Tue Oct 7 10:52:44 2025 +0200
GH-1609 Bad metadata naming (#1610)
It caused in certain cases that lock names for different metadata collapsed
onto same name, and hence, congestion with possible timeouts on resource
happened. In case of metadata, `Metadata#getType` was totally ignored, which
turns out was wrong, as today we have more types than the simple
`maven-metadata.xml`.
Fix: We already had means to make sure "string is valid for path segment",
but it was buried in `RepositoryIdHelper`. Pull it out, and make it reusable,
as this is essentially very same use case. Also, this extra work really
"punishes" only non maven metadata, and that is okay (right now we have only
repository prefix and archetype catalog files in play).
Fixes #1609
---
.../impl/synccontext/named/GAVNameMapper.java | 10 +++
...Test.java => BasedirHashingNameMapperTest.java} | 43 +++++++++-
.../synccontext/named/BasedirNameMapperTest.java | 94 +++++++++++++++++-----
.../impl/synccontext/named/GAVNameMapperTest.java | 30 +++++++
.../java/org/eclipse/aether/util/PathUtils.java | 75 +++++++++++++++++
.../aether/util/repository/RepositoryIdHelper.java | 35 +-------
.../org/eclipse/aether/util/PathUtilsTest.java | 50 ++++++++++++
.../util/repository/RepositoryIdHelperTest.java | 27 -------
8 files changed, 282 insertions(+), 82 deletions(-)
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapper.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapper.java
index 532526547..dc21b4438 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapper.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapper.java
@@ -26,6 +26,7 @@ import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.metadata.Metadata;
import org.eclipse.aether.named.NamedLockKey;
+import org.eclipse.aether.util.PathUtils;
import static java.util.Objects.requireNonNull;
@@ -103,6 +104,8 @@ public class GAVNameMapper implements NameMapper {
+ suffix;
}
+ private static final String MAVEN_METADATA = "maven-metadata.xml";
+
private static String getMetadataName(Metadata metadata, String prefix,
String separator, String suffix) {
String name = prefix;
if (!metadata.getGroupId().isEmpty()) {
@@ -113,6 +116,13 @@ public class GAVNameMapper implements NameMapper {
name += separator + metadata.getVersion();
}
}
+ if (!MAVEN_METADATA.equals(metadata.getType())) {
+ name += separator +
PathUtils.stringToPathSegment(metadata.getType());
+ }
+ } else {
+ if (!MAVEN_METADATA.equals(metadata.getType())) {
+ name += PathUtils.stringToPathSegment(metadata.getType());
+ }
}
return name + suffix;
}
diff --git
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirHashingNameMapperTest.java
similarity index 75%
copy from
maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
copy to
maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirHashingNameMapperTest.java
index 1eaf5549b..feea3b1ea 100644
---
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
+++
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirHashingNameMapperTest.java
@@ -31,9 +31,10 @@ import org.junit.jupiter.api.Test;
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
-public class BasedirNameMapperTest extends NameMapperTestSupport {
+public class BasedirHashingNameMapperTest extends NameMapperTestSupport {
private final String PS = "/"; // we work with URIs now, not OS file paths
BasedirNameMapper mapper = new BasedirNameMapper(new
HashingNameMapper(GAVNameMapper.gav()));
@@ -118,6 +119,44 @@ public class BasedirNameMapperTest extends
NameMapperTestSupport {
basedir.toUri() + PS + ".locks" + PS +
"293b3990971f4b4b02b220620d2538eaac5f221b");
}
+ @Test
+ void prefixMetadata() {
+ configProperties.put("aether.syncContext.named.hashing.depth", "0");
+
+ DefaultMetadata metadata =
+ new DefaultMetadata("", "", ".meta/prefixes-central.txt",
Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
+ assertEquals(1, names.size());
+ assertEquals(
+ names.iterator().next().name(),
+ basedir.toUri() + PS + ".locks" + PS +
"520e2ba3a365db8cd804bcc40df38e1a52987e0f");
+ }
+
+ @Test
+ void rootSomeMetadata() {
+ configProperties.put("aether.syncContext.named.hashing.depth", "0");
+
+ DefaultMetadata metadata = new DefaultMetadata("", "",
"something.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
+ assertEquals(1, names.size());
+ assertEquals(
+ names.iterator().next().name(),
+ basedir.toUri() + PS + ".locks" + PS +
"e75ca04110613537eeb805ac3cc3a3bb4b3b999a");
+ }
+
+ @Test
+ void nonRootSomeMetadata() {
+ configProperties.put("aether.syncContext.named.hashing.depth", "0");
+
+ DefaultMetadata metadata =
+ new DefaultMetadata("groupId", "artifactId", "something.xml",
Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
+ assertEquals(1, names.size());
+ assertEquals(
+ names.iterator().next().name(),
+ basedir.toUri() + PS + ".locks" + PS +
"3ebd7051578a145a78fc67adeaccdcdc5a914b28");
+ }
+
@Test
void oneAndOne() {
configProperties.put("aether.syncContext.named.hashing.depth", "0");
diff --git
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
index 1eaf5549b..e322ccf14 100644
---
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
+++
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
@@ -20,6 +20,7 @@ package org.eclipse.aether.internal.impl.synccontext.named;
import java.io.File;
import java.io.IOException;
+import java.nio.file.Path;
import java.util.Collection;
import java.util.Iterator;
@@ -27,6 +28,7 @@ import org.eclipse.aether.artifact.DefaultArtifact;
import org.eclipse.aether.metadata.DefaultMetadata;
import org.eclipse.aether.metadata.Metadata;
import org.eclipse.aether.named.NamedLockKey;
+import org.eclipse.aether.util.DirectoryUtils;
import org.junit.jupiter.api.Test;
import static java.util.Collections.emptyList;
@@ -34,9 +36,17 @@ import static java.util.Collections.singletonList;
import static org.junit.jupiter.api.Assertions.*;
public class BasedirNameMapperTest extends NameMapperTestSupport {
- private final String PS = "/"; // we work with URIs now, not OS file paths
-
- BasedirNameMapper mapper = new BasedirNameMapper(new
HashingNameMapper(GAVNameMapper.gav()));
+ private final BasedirNameMapper mapper = new
BasedirNameMapper(GAVNameMapper.fileGav());
+
+ private String getPrefix() throws IOException {
+ Path basedir = DirectoryUtils.resolveDirectory(
+ session, BasedirNameMapper.DEFAULT_LOCKS_DIR,
BasedirNameMapper.CONFIG_PROP_LOCKS_DIR, false);
+ String basedirPath = basedir.toAbsolutePath().toUri().toASCIIString();
+ if (!basedirPath.endsWith("/")) {
+ basedirPath = basedirPath + "/";
+ }
+ return basedirPath;
+ }
@Test
void nullsAndEmptyInputs() {
@@ -56,27 +66,27 @@ public class BasedirNameMapperTest extends
NameMapperTestSupport {
}
@Test
- void defaultLocksDir() {
+ void defaultLocksDir() throws IOException {
configProperties.put("aether.syncContext.named.hashing.depth", "0");
configProperties.put("aether.syncContext.named.basedir.locksDir",
null);
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
Collection<NamedLockKey> names = mapper.nameLocks(session,
singletonList(artifact), null);
assertEquals(1, names.size());
assertEquals(
- names.iterator().next().name(),
- basedir.toUri() + PS + ".locks" + PS +
"46e98183d232f1e16f863025080c7f2b9797fd10");
+ getPrefix() + "artifact~group~artifact~1.0.lock",
+ names.iterator().next().name());
}
@Test
- void relativeLocksDir() {
+ void relativeLocksDir() throws IOException {
configProperties.put("aether.syncContext.named.hashing.depth", "0");
configProperties.put("aether.syncContext.named.basedir.locksDir",
"my/locks");
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
Collection<NamedLockKey> names = mapper.nameLocks(session,
singletonList(artifact), null);
assertEquals(1, names.size());
assertEquals(
- names.iterator().next().name(),
- basedir.toUri() + PS + "my" + PS + "locks" + PS +
"46e98183d232f1e16f863025080c7f2b9797fd10");
+ getPrefix() + "artifact~group~artifact~1.0.lock",
+ names.iterator().next().name());
}
@Test
@@ -90,23 +100,25 @@ public class BasedirNameMapperTest extends
NameMapperTestSupport {
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
Collection<NamedLockKey> names = mapper.nameLocks(session,
singletonList(artifact), null);
assertEquals(1, names.size());
- assertEquals(names.iterator().next().name(), customBaseDir + PS +
"46e98183d232f1e16f863025080c7f2b9797fd10");
+ assertEquals(
+ getPrefix() + "artifact~group~artifact~1.0.lock",
+ names.iterator().next().name());
}
@Test
- void singleArtifact() {
+ void singleArtifact() throws IOException {
configProperties.put("aether.syncContext.named.hashing.depth", "0");
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
Collection<NamedLockKey> names = mapper.nameLocks(session,
singletonList(artifact), null);
assertEquals(1, names.size());
assertEquals(
- names.iterator().next().name(),
- basedir.toUri() + PS + ".locks" + PS +
"46e98183d232f1e16f863025080c7f2b9797fd10");
+ getPrefix() + "artifact~group~artifact~1.0.lock",
+ names.iterator().next().name());
}
@Test
- void singleMetadata() {
+ void singleMetadata() throws IOException {
configProperties.put("aether.syncContext.named.hashing.depth", "0");
DefaultMetadata metadata =
@@ -114,12 +126,50 @@ public class BasedirNameMapperTest extends
NameMapperTestSupport {
Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
assertEquals(1, names.size());
assertEquals(
- names.iterator().next().name(),
- basedir.toUri() + PS + ".locks" + PS +
"293b3990971f4b4b02b220620d2538eaac5f221b");
+ getPrefix() + "metadata~group~artifact.lock",
+ names.iterator().next().name());
+ }
+
+ @Test
+ void prefixMetadata() throws IOException {
+ configProperties.put("aether.syncContext.named.hashing.depth", "0");
+
+ DefaultMetadata metadata =
+ new DefaultMetadata("", "", ".meta/prefixes-central.txt",
Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
+ assertEquals(1, names.size());
+ assertEquals(
+ getPrefix() + "metadata~.meta-SLASH-prefixes-central.txt.lock",
+ names.iterator().next().name());
+ }
+
+ @Test
+ void rootSomeMetadata() throws IOException {
+ configProperties.put("aether.syncContext.named.hashing.depth", "0");
+
+ DefaultMetadata metadata = new DefaultMetadata("", "",
"something.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
+ assertEquals(1, names.size());
+ assertEquals(
+ getPrefix() + "metadata~something.xml.lock",
+ names.iterator().next().name());
+ }
+
+ @Test
+ void nonRootSomeMetadata() throws IOException {
+ configProperties.put("aether.syncContext.named.hashing.depth", "0");
+
+ DefaultMetadata metadata =
+ new DefaultMetadata("groupId", "artifactId", "something.xml",
Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
+ assertEquals(1, names.size());
+ assertEquals(
+ getPrefix() + "metadata~groupId~artifactId~something.xml.lock",
+ names.iterator().next().name());
}
@Test
- void oneAndOne() {
+ void oneAndOne() throws IOException {
configProperties.put("aether.syncContext.named.hashing.depth", "0");
DefaultArtifact artifact = new DefaultArtifact("agroup:artifact:1.0");
@@ -127,15 +177,15 @@ public class BasedirNameMapperTest extends
NameMapperTestSupport {
new DefaultMetadata("bgroup", "artifact",
"maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
Collection<NamedLockKey> names = mapper.nameLocks(session,
singletonList(artifact), singletonList(metadata));
- assertEquals(names.size(), 2);
+ assertEquals(2, names.size());
Iterator<NamedLockKey> namesIterator = names.iterator();
// they are sorted as well
assertEquals(
- namesIterator.next().name(),
- basedir.toUri() + PS + ".locks" + PS +
"d36504431d00d1c6e4d1c34258f2bf0a004de085");
+ getPrefix() + "artifact~agroup~artifact~1.0.lock",
+ namesIterator.next().name());
assertEquals(
- namesIterator.next().name(),
- basedir.toUri() + PS + ".locks" + PS +
"fbcebba60d7eb931eca634f6ca494a8a1701b638");
+ getPrefix() + "metadata~bgroup~artifact.lock",
+ namesIterator.next().name());
}
}
diff --git
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapperTest.java
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapperTest.java
index 27a65d349..308828766 100644
---
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapperTest.java
+++
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapperTest.java
@@ -69,6 +69,36 @@ public class GAVNameMapperTest extends NameMapperTestSupport
{
assertEquals("metadata~group~artifact.lock",
names.iterator().next().name());
}
+ @Test
+ void prefixMetadata() {
+ DefaultMetadata metadata =
+ new DefaultMetadata("", "", ".meta/prefixes-central.txt",
Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
+ assertEquals(1, names.size());
+ assertEquals(
+ "metadata~.meta-SLASH-prefixes-central.txt.lock",
+ names.iterator().next().name());
+ }
+
+ @Test
+ void rootSomeMetadata() {
+ DefaultMetadata metadata = new DefaultMetadata("", "",
"something.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
+ assertEquals(1, names.size());
+ assertEquals("metadata~something.xml.lock",
names.iterator().next().name());
+ }
+
+ @Test
+ void nonRootSomeMetadata() {
+ DefaultMetadata metadata =
+ new DefaultMetadata("groupId", "artifactId", "something.xml",
Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null,
singletonList(metadata));
+ assertEquals(1, names.size());
+ assertEquals(
+ "metadata~groupId~artifactId~something.xml.lock",
+ names.iterator().next().name());
+ }
+
@Test
void oneAndOne() {
DefaultArtifact artifact = new DefaultArtifact("agroup:artifact:1.0");
diff --git
a/maven-resolver-util/src/main/java/org/eclipse/aether/util/PathUtils.java
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/PathUtils.java
new file mode 100644
index 000000000..a1480cd2c
--- /dev/null
+++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/PathUtils.java
@@ -0,0 +1,75 @@
+/*
+ * 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.eclipse.aether.util;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A reusable utility class for file paths.
+ *
+ * @since 2.0.13
+ */
+public final class PathUtils {
+ private PathUtils() {
+ // hide constructor
+ }
+
+ private static final Map<String, String> ILLEGAL_PATH_SEGMENT_REPLACEMENTS;
+
+ static {
+ HashMap<String, String> illegalPathSegmentReplacements = new
HashMap<>();
+ illegalPathSegmentReplacements.put("\\", "-BACKSLASH-");
+ illegalPathSegmentReplacements.put("/", "-SLASH-");
+ illegalPathSegmentReplacements.put(":", "-COLON-");
+ illegalPathSegmentReplacements.put("\"", "-QUOTE-");
+ illegalPathSegmentReplacements.put("<", "-LT-");
+ illegalPathSegmentReplacements.put(">", "-GT-");
+ illegalPathSegmentReplacements.put("|", "-PIPE-");
+ illegalPathSegmentReplacements.put("?", "-QMARK-");
+ illegalPathSegmentReplacements.put("*", "-ASTERISK-");
+ ILLEGAL_PATH_SEGMENT_REPLACEMENTS =
Collections.unmodifiableMap(illegalPathSegmentReplacements);
+ }
+
+ /**
+ * Method that makes sure that passed in string is valid "path segment"
string. It achieves it by potentially
+ * changing it, replacing illegal characters in it with legal ones.
+ * <p>
+ * Note: this method considers empty string as "valid path segment", it is
caller duty to ensure empty string
+ * is not used as path segment alone.
+ * <p>
+ * This method is simplistic on purpose, and if frequently used, best if
results are cached (per session)
+ */
+ public static String stringToPathSegment(String string) {
+ requireNonNull(string);
+ StringBuilder result = new StringBuilder(string);
+ for (Map.Entry<String, String> entry :
ILLEGAL_PATH_SEGMENT_REPLACEMENTS.entrySet()) {
+ String illegal = entry.getKey();
+ int pos = result.indexOf(illegal);
+ while (pos >= 0) {
+ result.replace(pos, pos + illegal.length(), entry.getValue());
+ pos = result.indexOf(illegal);
+ }
+ }
+ return result.toString();
+ }
+}
diff --git
a/maven-resolver-util/src/main/java/org/eclipse/aether/util/repository/RepositoryIdHelper.java
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/repository/RepositoryIdHelper.java
index 1c5563a2f..edcf42e8e 100644
---
a/maven-resolver-util/src/main/java/org/eclipse/aether/util/repository/RepositoryIdHelper.java
+++
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/repository/RepositoryIdHelper.java
@@ -21,9 +21,7 @@ package org.eclipse.aether.util.repository;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
-import java.util.HashMap;
import java.util.Locale;
-import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import java.util.function.Predicate;
@@ -31,6 +29,7 @@ import java.util.function.Predicate;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.repository.ArtifactRepository;
import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.util.PathUtils;
import org.eclipse.aether.util.StringDigestUtil;
import static java.util.Objects.requireNonNull;
@@ -41,27 +40,12 @@ import static java.util.Objects.requireNonNull;
* POMs out there that somehow define repositories with unsafe characters in
their id. The problem affects mostly
* {@link RemoteRepository} instances, as all other implementations have fixed
ids that are path safe.
*
+ * @see PathUtils
* @since 2.0.11
*/
public final class RepositoryIdHelper {
private RepositoryIdHelper() {}
- private static final Map<String, String> ILLEGAL_REPO_ID_REPLACEMENTS;
-
- static {
- HashMap<String, String> illegalRepoIdReplacements = new HashMap<>();
- illegalRepoIdReplacements.put("\\", "-BACKSLASH-");
- illegalRepoIdReplacements.put("/", "-SLASH-");
- illegalRepoIdReplacements.put(":", "-COLON-");
- illegalRepoIdReplacements.put("\"", "-QUOTE-");
- illegalRepoIdReplacements.put("<", "-LT-");
- illegalRepoIdReplacements.put(">", "-GT-");
- illegalRepoIdReplacements.put("|", "-PIPE-");
- illegalRepoIdReplacements.put("?", "-QMARK-");
- illegalRepoIdReplacements.put("*", "-ASTERISK-");
- ILLEGAL_REPO_ID_REPLACEMENTS =
Collections.unmodifiableMap(illegalRepoIdReplacements);
- }
-
private static final String CENTRAL_REPOSITORY_ID = "central";
private static final Collection<String> CENTRAL_URLS =
Collections.unmodifiableList(Arrays.asList(
"https://repo.maven.apache.org/maven2";,
@@ -164,23 +148,12 @@ public final class RepositoryIdHelper {
* <p>
* This method is simplistic on purpose, and if frequently used, best if
results are cached (per session),
* see {@link #cachedIdToPathSegment(RepositorySystemSession)} method.
- * <p>
- * This method is visible for testing only, should not be used in any
other scenarios.
*
* @see #cachedIdToPathSegment(RepositorySystemSession)
*/
- static String idToPathSegment(ArtifactRepository repository) {
+ private static String idToPathSegment(ArtifactRepository repository) {
if (repository instanceof RemoteRepository) {
- StringBuilder result = new StringBuilder(repository.getId());
- for (Map.Entry<String, String> entry :
ILLEGAL_REPO_ID_REPLACEMENTS.entrySet()) {
- String illegal = entry.getKey();
- int pos = result.indexOf(illegal);
- while (pos >= 0) {
- result.replace(pos, pos + illegal.length(),
entry.getValue());
- pos = result.indexOf(illegal);
- }
- }
- return result.toString();
+ return PathUtils.stringToPathSegment(repository.getId());
} else {
return repository.getId();
}
diff --git
a/maven-resolver-util/src/test/java/org/eclipse/aether/util/PathUtilsTest.java
b/maven-resolver-util/src/test/java/org/eclipse/aether/util/PathUtilsTest.java
new file mode 100644
index 000000000..97c1b4563
--- /dev/null
+++
b/maven-resolver-util/src/test/java/org/eclipse/aether/util/PathUtilsTest.java
@@ -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.
+ */
+package org.eclipse.aether.util;
+
+import java.util.function.UnaryOperator;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+
+public class PathUtilsTest {
+ @Test
+ void stringToPathSegment_fixes() {
+ UnaryOperator<String> safeId = PathUtils::stringToPathSegment;
+ String good = "good";
+ String bad = "bad:id";
+
+ String goodFixedId = safeId.apply(good);
+ assertEquals(good, goodFixedId);
+
+ String badFixedId = safeId.apply(bad);
+ assertNotEquals(bad, badFixedId);
+ assertEquals("bad-COLON-id", badFixedId);
+ }
+
+ @Test
+ void stringToPathSegment_allCharsBad() {
+ String veryBad = "\\/:\"<>|?*";
+ String badFixedId = PathUtils.stringToPathSegment(veryBad);
+ assertNotEquals(veryBad, badFixedId);
+
assertEquals("-BACKSLASH--SLASH--COLON--QUOTE--LT--GT--PIPE--QMARK--ASTERISK-",
badFixedId);
+ }
+}
diff --git
a/maven-resolver-util/src/test/java/org/eclipse/aether/util/repository/RepositoryIdHelperTest.java
b/maven-resolver-util/src/test/java/org/eclipse/aether/util/repository/RepositoryIdHelperTest.java
index 7d80c98fa..bdbc15c1e 100644
---
a/maven-resolver-util/src/test/java/org/eclipse/aether/util/repository/RepositoryIdHelperTest.java
+++
b/maven-resolver-util/src/test/java/org/eclipse/aether/util/repository/RepositoryIdHelperTest.java
@@ -32,33 +32,6 @@ import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertSame;
public class RepositoryIdHelperTest {
- @Test
- void fixes() {
- Function<RemoteRepository, String> safeId =
RepositoryIdHelper::idToPathSegment;
- RemoteRepository good = new RemoteRepository.Builder("good",
"default", "https://somewhere.com";).build();
- RemoteRepository bad = new RemoteRepository.Builder("bad:id",
"default", "https://somewhere.com";).build();
-
- String goodId = good.getId();
- String goodFixedId = safeId.apply(good);
- assertEquals(goodId, goodFixedId);
-
- String badId = bad.getId();
- String badFixedId = safeId.apply(bad);
- assertNotEquals(badId, badFixedId);
- assertEquals("bad-COLON-id", badFixedId);
- }
-
- @Test
- void allCharsBad() {
- // hopefully we have no such IDs
- RemoteRepository veryBad =
- new RemoteRepository.Builder("\\/:\"<>|?*", "default",
"https://somewhere.com";).build();
- String badId = veryBad.getId();
- String badFixedId = RepositoryIdHelper.idToPathSegment(veryBad);
- assertNotEquals(badId, badFixedId);
-
assertEquals("-BACKSLASH--SLASH--COLON--QUOTE--LT--GT--PIPE--QMARK--ASTERISK-",
badFixedId);
- }
-
@Test
void caching() {
DefaultRepositorySystemSession session = new
DefaultRepositorySystemSession(s -> false);
