Skip to content

Commit b2f0924

Browse files
committed
[MNG-8192] Consistently throw InvalidArtifactRTException for invalid
coordinates This fixes throwing NPE for version being null.
1 parent f8aaf28 commit b2f0924

File tree

4 files changed

+46
-4
lines changed

4 files changed

+46
-4
lines changed

maven-artifact/pom.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ under the License.
3636
<artifactId>junit-jupiter-api</artifactId>
3737
<scope>test</scope>
3838
</dependency>
39+
<dependency>
40+
<groupId>org.junit.jupiter</groupId>
41+
<artifactId>junit-jupiter-params</artifactId>
42+
<scope>test</scope>
43+
</dependency>
3944
</dependencies>
4045

4146
<build>

maven-artifact/src/main/java/org/apache/maven/artifact/DefaultArtifact.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,22 +168,22 @@ private void validateIdentity() {
168168
groupId, artifactId, getVersion(), type, "The groupId cannot be empty.");
169169
}
170170

171-
if (artifactId == null) {
171+
if (empty(artifactId)) {
172172
throw new InvalidArtifactRTException(
173173
groupId, artifactId, getVersion(), type, "The artifactId cannot be empty.");
174174
}
175175

176-
if (type == null) {
176+
if (empty(type)) {
177177
throw new InvalidArtifactRTException(groupId, artifactId, getVersion(), type, "The type cannot be empty.");
178178
}
179179

180-
if ((version == null) && (versionRange == null)) {
180+
if ((empty(version)) && (versionRange == null)) {
181181
throw new InvalidArtifactRTException(
182182
groupId, artifactId, getVersion(), type, "The version cannot be empty.");
183183
}
184184
}
185185

186-
private boolean empty(String value) {
186+
public static boolean empty(String value) {
187187
return (value == null) || (value.trim().length() < 1);
188188
}
189189

maven-artifact/src/main/java/org/apache/maven/artifact/versioning/VersionRange.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.WeakHashMap;
2828

2929
import org.apache.maven.artifact.Artifact;
30+
import org.apache.maven.artifact.DefaultArtifact;
3031

3132
/**
3233
* Construct a version range from a specification.
@@ -202,6 +203,9 @@ private static Restriction parseRestriction(String spec) throws InvalidVersionSp
202203
}
203204

204205
public static VersionRange createFromVersion(String version) {
206+
if (DefaultArtifact.empty(version)) {
207+
return null;
208+
}
205209
VersionRange cached = CACHE_VERSION.get(version);
206210
if (cached == null) {
207211
List<Restriction> restrictions = Collections.emptyList();

maven-artifact/src/test/java/org/apache/maven/artifact/DefaultArtifactTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,19 @@
1818
*/
1919
package org.apache.maven.artifact;
2020

21+
import java.util.stream.Stream;
22+
2123
import org.apache.maven.artifact.handler.ArtifactHandlerMock;
2224
import org.apache.maven.artifact.versioning.VersionRange;
2325
import org.junit.jupiter.api.BeforeEach;
2426
import org.junit.jupiter.api.Test;
27+
import org.junit.jupiter.params.ParameterizedTest;
28+
import org.junit.jupiter.params.provider.Arguments;
29+
import org.junit.jupiter.params.provider.MethodSource;
2530

2631
import static org.junit.jupiter.api.Assertions.assertEquals;
2732
import static org.junit.jupiter.api.Assertions.assertNull;
33+
import static org.junit.jupiter.api.Assertions.assertThrows;
2834
import static org.junit.jupiter.api.Assertions.assertTrue;
2935

3036
class DefaultArtifactTest {
@@ -152,4 +158,31 @@ void testMNG7780() throws Exception {
152158
new DefaultArtifact(groupId, artifactId, vr, scope, type, null, artifactHandler);
153159
assertEquals(artifact, nullVersionArtifact);
154160
}
161+
162+
@ParameterizedTest
163+
@MethodSource("invalidMavenCoordinates")
164+
void testIllegalCoordinatesInConstructor(String groupId, String artifactId, String version) {
165+
assertThrows(
166+
InvalidArtifactRTException.class,
167+
() -> new DefaultArtifact(
168+
groupId, artifactId, version, scope, type, classifier, artifactHandler, false));
169+
if (version == null) {
170+
assertThrows(
171+
InvalidArtifactRTException.class,
172+
() -> new DefaultArtifact(
173+
groupId, artifactId, (VersionRange) null, scope, type, classifier, artifactHandler, false));
174+
}
175+
}
176+
177+
static Stream<Arguments> invalidMavenCoordinates() {
178+
return Stream.of(
179+
Arguments.of(null, null, null),
180+
Arguments.of("", "", ""),
181+
Arguments.of(null, "artifactId", "1.0"),
182+
Arguments.of("", "artifactId", "1.0"),
183+
Arguments.of("groupId", null, "1.0"),
184+
Arguments.of("groupId", "", "1.0"),
185+
Arguments.of("groupId", "artifactId", null),
186+
Arguments.of("groupId", "artifactId", ""));
187+
}
155188
}

0 commit comments

Comments
 (0)