[MNG-8180] Handle NPE due non-existent tags (#1639)

There was an NPE possibility when plugin.xml had no expected tags present. Also: maven-compat has plugin.xml (!) w/o "name" tag, it NPEd and failed build. This was NOT picked up by CI as "rebuild itself" step does not install (just verify). --- https://issues.apache.org/jira/browse/MNG-8180
apache · Aug 11, 2024 · da7c211 · da7c211
1 parent 182b87c
commit da7c211
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 12 deletions.
diff --git a/maven-api-impl/src/main/java/org/apache/maven/internal/impl/resolver/PluginsMetadata.java b/maven-api-impl/src/main/java/org/apache/maven/internal/impl/resolver/PluginsMetadata.java
@@ -24,6 +24,8 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
 import org.apache.maven.api.metadata.Metadata;
 import org.apache.maven.api.metadata.Plugin;
 
@@ -32,12 +34,16 @@
  */
 final class PluginsMetadata extends MavenMetadata {
     static final class PluginInfo {
+        @Nonnull
         final String groupId;
 
+        @Nonnull
         private final String artifactId;
 
+        @Nullable
         private final String goalPrefix;
 
+        @Nullable
         private final String name;
 
         PluginInfo(String groupId, String artifactId, String goalPrefix, String name) {

diff --git a/...-impl/src/main/java/org/apache/maven/internal/impl/resolver/PluginsMetadataGenerator.java b/...-impl/src/main/java/org/apache/maven/internal/impl/resolver/PluginsMetadataGenerator.java
@@ -129,13 +129,14 @@ private PluginInfo extractPluginInfo(Artifact artifact) {
                             // - maven-plugin-api (for model)
                             // - Plexus Container (for model supporting classes and exceptions)
                             XmlNode root = XmlNodeStaxBuilder.build(is, null);
-                            String groupId = root.getChild("groupId").getValue();
-                            String artifactId = root.getChild("artifactId").getValue();
-                            String goalPrefix = root.getChild("goalPrefix").getValue();
-                            String name = root.getChild("name").getValue();
+                            String groupId = mayGetChild(root, "groupId");
+                            String artifactId = mayGetChild(root, "artifactId");
+                            String goalPrefix = mayGetChild(root, "goalPrefix");
+                            String name = mayGetChild(root, "name");
                             // sanity check: plugin descriptor extracted from artifact must have same GA
                             if (Objects.equals(artifact.getGroupId(), groupId)
                                     && Objects.equals(artifact.getArtifactId(), artifactId)) {
+                                // here groupId and artifactId cannot be null
                                 return new PluginInfo(groupId, artifactId, goalPrefix, name);
                             } else {
                                 throw new InvalidArtifactPluginMetadataException(
@@ -148,16 +149,25 @@ private PluginInfo extractPluginInfo(Artifact artifact) {
                             }
                         }
                     }
-                } catch (RuntimeException e) {
-                    throw e;
                 } catch (Exception e) {
+                    if (e instanceof InvalidArtifactPluginMetadataException iapme) {
+                        throw iapme;
+                    }
                     // here we can have: IO. ZIP or Plexus Conf Ex: but we should not interfere with user intent
                 }
             }
         }
         return null;
     }
 
+    private static String mayGetChild(XmlNode node, String child) {
+        XmlNode c = node.getChild(child);
+        if (c != null) {
+            return c.getValue();
+        }
+        return null;
+    }
+
     public static final class InvalidArtifactPluginMetadataException extends IllegalArgumentException {
         InvalidArtifactPluginMetadataException(String s) {
             super(s);

diff --git a/...resolver-provider/src/main/java/org/apache/maven/repository/internal/PluginsMetadata.java b/...resolver-provider/src/main/java/org/apache/maven/repository/internal/PluginsMetadata.java
@@ -25,6 +25,8 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
 import org.apache.maven.artifact.repository.metadata.Metadata;
 import org.apache.maven.artifact.repository.metadata.Plugin;
 
@@ -33,12 +35,16 @@
  */
 final class PluginsMetadata extends MavenMetadata {
     static final class PluginInfo {
+        @Nonnull
         final String groupId;
 
+        @Nonnull
         private final String artifactId;
 
+        @Nullable
         private final String goalPrefix;
 
+        @Nullable
         private final String name;
 
         PluginInfo(String groupId, String artifactId, String goalPrefix, String name) {

diff --git a/...provider/src/main/java/org/apache/maven/repository/internal/PluginsMetadataGenerator.java b/...provider/src/main/java/org/apache/maven/repository/internal/PluginsMetadataGenerator.java
@@ -130,13 +130,14 @@ private PluginInfo extractPluginInfo(Artifact artifact) {
                             // - maven-plugin-api (for model)
                             // - Plexus Container (for model supporting classes and exceptions)
                             XmlNode root = XmlNodeBuilder.build(is, null);
-                            String groupId = root.getChild("groupId").getValue();
-                            String artifactId = root.getChild("artifactId").getValue();
-                            String goalPrefix = root.getChild("goalPrefix").getValue();
-                            String name = root.getChild("name").getValue();
+                            String groupId = mayGetChild(root, "groupId");
+                            String artifactId = mayGetChild(root, "artifactId");
+                            String goalPrefix = mayGetChild(root, "goalPrefix");
+                            String name = mayGetChild(root, "name");
                             // sanity check: plugin descriptor extracted from artifact must have same GA
                             if (Objects.equals(artifact.getGroupId(), groupId)
                                     && Objects.equals(artifact.getArtifactId(), artifactId)) {
+                                // here groupId and artifactId cannot be null
                                 return new PluginInfo(groupId, artifactId, goalPrefix, name);
                             } else {
                                 throw new InvalidArtifactPluginMetadataException(
@@ -149,16 +150,25 @@ private PluginInfo extractPluginInfo(Artifact artifact) {
                             }
                         }
                     }
-                } catch (RuntimeException e) {
-                    throw e;
                 } catch (Exception e) {
+                    if (e instanceof InvalidArtifactPluginMetadataException iapme) {
+                        throw iapme;
+                    }
                     // here we can have: IO. ZIP or Plexus Conf Ex: but we should not interfere with user intent
                 }
             }
         }
         return null;
     }
 
+    private static String mayGetChild(XmlNode node, String child) {
+        XmlNode c = node.getChild(child);
+        if (c != null) {
+            return c.getValue();
+        }
+        return null;
+    }
+
     public static final class InvalidArtifactPluginMetadataException extends IllegalArgumentException {
         InvalidArtifactPluginMetadataException(String s) {
             super(s);