[MNG-8192] Consistently throw InvalidArtifactRTException for invalid

coordinates This fixes throwing NPE for version being null.
apache · Aug 9, 2024 · b2f0924 · b2f0924
1 parent f8aaf28
commit b2f0924
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 4 deletions.
diff --git a/maven-artifact/pom.xml b/maven-artifact/pom.xml
@@ -36,6 +36,11 @@ under the License.
       <artifactId>junit-jupiter-api</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-params</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/DefaultArtifact.java b/maven-artifact/src/main/java/org/apache/maven/artifact/DefaultArtifact.java
@@ -168,22 +168,22 @@ private void validateIdentity() {
                     groupId, artifactId, getVersion(), type, "The groupId cannot be empty.");
         }
 
-        if (artifactId == null) {
+        if (empty(artifactId)) {
             throw new InvalidArtifactRTException(
                     groupId, artifactId, getVersion(), type, "The artifactId cannot be empty.");
         }
 
-        if (type == null) {
+        if (empty(type)) {
             throw new InvalidArtifactRTException(groupId, artifactId, getVersion(), type, "The type cannot be empty.");
         }
 
-        if ((version == null) && (versionRange == null)) {
+        if ((empty(version)) && (versionRange == null)) {
             throw new InvalidArtifactRTException(
                     groupId, artifactId, getVersion(), type, "The version cannot be empty.");
         }
     }
 
-    private boolean empty(String value) {
+    public static boolean empty(String value) {
         return (value == null) || (value.trim().length() < 1);
     }
 

diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java
@@ -27,6 +27,7 @@
 import java.util.WeakHashMap;
 
 import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.DefaultArtifact;
 
 /**
  * Construct a version range from a specification.
@@ -202,6 +203,9 @@ private static Restriction parseRestriction(String spec) throws InvalidVersionSp
     }
 
     public static VersionRange createFromVersion(String version) {
+        if (DefaultArtifact.empty(version)) {
+            return null;
+        }
         VersionRange cached = CACHE_VERSION.get(version);
         if (cached == null) {
             List<Restriction> restrictions = Collections.emptyList();

diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/DefaultArtifactTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/DefaultArtifactTest.java
@@ -18,13 +18,19 @@
  */
 package org.apache.maven.artifact;
 
+import java.util.stream.Stream;
+
 import org.apache.maven.artifact.handler.ArtifactHandlerMock;
 import org.apache.maven.artifact.versioning.VersionRange;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class DefaultArtifactTest {
@@ -152,4 +158,31 @@ void testMNG7780() throws Exception {
                 new DefaultArtifact(groupId, artifactId, vr, scope, type, null, artifactHandler);
         assertEquals(artifact, nullVersionArtifact);
     }
+
+    @ParameterizedTest
+    @MethodSource("invalidMavenCoordinates")
+    void testIllegalCoordinatesInConstructor(String groupId, String artifactId, String version) {
+        assertThrows(
+                InvalidArtifactRTException.class,
+                () -> new DefaultArtifact(
+                        groupId, artifactId, version, scope, type, classifier, artifactHandler, false));
+        if (version == null) {
+            assertThrows(
+                    InvalidArtifactRTException.class,
+                    () -> new DefaultArtifact(
+                            groupId, artifactId, (VersionRange) null, scope, type, classifier, artifactHandler, false));
+        }
+    }
+
+    static Stream<Arguments> invalidMavenCoordinates() {
+        return Stream.of(
+                Arguments.of(null, null, null),
+                Arguments.of("", "", ""),
+                Arguments.of(null, "artifactId", "1.0"),
+                Arguments.of("", "artifactId", "1.0"),
+                Arguments.of("groupId", null, "1.0"),
+                Arguments.of("groupId", "", "1.0"),
+                Arguments.of("groupId", "artifactId", null),
+                Arguments.of("groupId", "artifactId", ""));
+    }
 }