Check for invalid characters when adding an extension

* Fixes #49895
* use a pattern matcher to validate the groupId and artifactId for a fully qualified extension name (group, artifact, and version)
* introduced a new collection of invalid keywords so they can be reported separately from those that are not in the catalog
* added some tests that try to mimic manual tests that have been used

Signed-off-by:Nathan Erwin <[email protected]>

Author: nderwin

independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/commands/handlers/AddExtensionsCommandHandler.java

@@ -10,6 +10,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
+import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.StringUtils;
 
@@ -34,6 +35,8 @@
  */
 public class AddExtensionsCommandHandler implements QuarkusCommandHandler {
 
+    private static final Pattern VALID_IDENTIFIER = Pattern.compile("[A-Za-z0-9_.-]+");
+
     @Override
     public QuarkusCommandOutcome execute(QuarkusCommandInvocation invocation) throws QuarkusCommandException {
         final Set<String> extensionsQuery = invocation.getValue(AddExtensions.EXTENSIONS, Collections.emptySet());
@@ -65,11 +68,21 @@ public QuarkusCommandOutcome execute(QuarkusCommandInvocation invocation) throws
                                 .info(MessageIcons.NOOP_ICON + " Extension " + a.getGroupId() + ":" + a.getArtifactId()
                                         + " was already installed"));
                 return QuarkusCommandOutcome.success().setValue(AddExtensions.OUTCOME_UPDATED, result.isSourceUpdated());
-            } else if (!extensionInstallPlan.getUnmatchedKeywords().isEmpty()) {
-                invocation.log()
-                        .info(ERROR_ICON + " Nothing installed because keyword(s) '"
-                                + String.join("', '", extensionInstallPlan.getUnmatchedKeywords())
-                                + "' were not matched in the catalog.");
+            } else if (!extensionInstallPlan.getUnmatchedKeywords().isEmpty()
+                    || !extensionInstallPlan.getInvalidKeywords().isEmpty()) {
+                if (!extensionInstallPlan.getUnmatchedKeywords().isEmpty()) {
+                    invocation.log()
+                            .info(ERROR_ICON + " Nothing installed because keyword(s) '"
+                                    + String.join("', '", extensionInstallPlan.getUnmatchedKeywords())
+                                    + "' were not matched in the catalog.");
+                }
+
+                if (!extensionInstallPlan.getInvalidKeywords().isEmpty()) {
+                    invocation.log()
+                            .info(FAILURE_ICON + " Nothing installed because keyword(s) '"
+                                    + String.join("', '", extensionInstallPlan.getInvalidKeywords())
+                                    + "' were invalid.");
+                }
             } else {
                 invocation.log()
                         .info(FAILURE_ICON + " The provided keyword(s) did not match any extension from the catalog.");
@@ -107,7 +120,13 @@ public ExtensionInstallPlan planInstallation(QuarkusCommandInvocation invocation
             int countColons = StringUtils.countMatches(keyword, ":");
             if (countColons > 1) {
                 // it's a gav
-                builder.addIndependentExtension(ArtifactCoords.fromString(keyword));
+                ArtifactCoords coords = ArtifactCoords.fromString(keyword);
+                if (VALID_IDENTIFIER.matcher(coords.getGroupId()).matches()
+                        && VALID_IDENTIFIER.matcher(coords.getArtifactId()).matches()) {
+                    builder.addIndependentExtension(coords);
+                } else {
+                    builder.addInvalidKeyword(keyword);
+                }
                 continue;
             }
             List<Extension> listed = List.of();

independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/project/extensions/ExtensionInstallPlan.java

@@ -14,21 +14,25 @@ public class ExtensionInstallPlan {
             Collections.emptySet(),
             Collections.emptySet(),
             Collections.emptySet(),
+            Collections.emptySet(),
             Collections.emptySet());
 
     private final Set<ArtifactCoords> platforms;
     private final Set<ArtifactCoords> managedExtensions;
     private final Set<ArtifactCoords> independentExtensions;
     private final Collection<String> unmatchedKeywords;
+    private final Collection<String> invalidKeywords;
 
     private ExtensionInstallPlan(Set<ArtifactCoords> platforms,
             Set<ArtifactCoords> managedExtensions,
             Set<ArtifactCoords> independentExtensions,
-            Collection<String> unmatchedKeywords) {
+            Collection<String> unmatchedKeywords,
+            Collection<String> invalidKeywords) {
         this.platforms = platforms;
         this.managedExtensions = managedExtensions;
         this.independentExtensions = independentExtensions;
         this.unmatchedKeywords = unmatchedKeywords;
+        this.invalidKeywords = invalidKeywords;
     }
 
     public boolean isNotEmpty() {
@@ -77,13 +81,18 @@ public Collection<String> getUnmatchedKeywords() {
         return unmatchedKeywords;
     }
 
+    public Collection<String> getInvalidKeywords() {
+        return invalidKeywords;
+    }
+
     @Override
     public String toString() {
         return "InstallRequest{" +
                 "platforms=" + platforms +
                 ", managedExtensions=" + managedExtensions +
                 ", independentExtensions=" + independentExtensions +
                 ", unmatchedKeywords=" + unmatchedKeywords +
+                ", invalidKeywords=" + invalidKeywords +
                 '}';
     }
 
@@ -97,9 +106,11 @@ public static class Builder {
         private final Set<ArtifactCoords> extensionsInPlatforms = new LinkedHashSet<>();
         private final Set<ArtifactCoords> independentExtensions = new LinkedHashSet<>();
         private final Collection<String> unmatchedKeywords = new ArrayList<>();
+        private final Collection<String> invalidKeywords = new ArrayList<>();
 
         public ExtensionInstallPlan build() {
-            return new ExtensionInstallPlan(platforms, extensionsInPlatforms, independentExtensions, unmatchedKeywords);
+            return new ExtensionInstallPlan(platforms, extensionsInPlatforms, independentExtensions, unmatchedKeywords,
+                    invalidKeywords);
         }
 
         public Builder addIndependentExtension(ArtifactCoords artifactCoords) {
@@ -122,6 +133,11 @@ public Builder addUnmatchedKeyword(String unmatchedKeyword) {
             return this;
         }
 
+        public Builder addInvalidKeyword(String invalidKeyword) {
+            this.invalidKeywords.add(invalidKeyword);
+            return this;
+        }
+
         public boolean hasExtensionInPlatform() {
             return !this.extensionsInPlatforms.isEmpty();
         }

independent-projects/tools/devtools-testing/src/test/java/io/quarkus/devtools/project/create/MavenProjectAddExtensionTest.java

@@ -0,0 +1,72 @@
+package io.quarkus.devtools.project.create;
+
+import static io.quarkus.devtools.project.create.MultiplePlatformBomsTestBase.enableRegistryClient;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.nio.file.Path;
+import java.util.List;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import io.quarkus.devtools.commands.data.QuarkusCommandOutcome;
+import io.quarkus.devtools.testing.registry.client.TestRegistryClientBuilder;
+import io.quarkus.registry.catalog.PlatformStreamCoords;
+
+public class MavenProjectAddExtensionTest extends MultiplePlatformBomsTestBase {
+
+    private static final String PLATFORM_KEY = "io.test.platform";
+
+    @BeforeAll
+    public static void setup() throws Exception {
+        TestRegistryClientBuilder.newInstance()
+                .baseDir(configDir())
+                .newRegistry("registry.test.io")
+                .newPlatform(PLATFORM_KEY)
+                .newStream("1.0")
+                .newRelease("1.1.1")
+                .quarkusVersion("1.1.1")
+                // default bom including quarkus-core + essential metadata
+                .addCoreMember().release()
+                // foo platform member
+                .newMember("acme-a-bom").addExtension("ext-a").release()
+                .stream().platform()
+                .newArchivedStream("0.5")
+                .newArchivedRelease("0.5.1")
+                .quarkusVersion("0.5.1")
+                // default bom including quarkus-core + essential metadata
+                .addCoreMember().release()
+                // foo platform member
+                .newMember("acme-a-bom").addExtension("ext-a").release()
+                .registry()
+                .clientBuilder()
+                .build();
+
+        enableRegistryClient();
+    }
+
+    @Override
+    protected String getMainPlatformKey() {
+        return PLATFORM_KEY;
+    }
+
+    @Test
+    public void test() throws Exception {
+        final Path projectDir = newProjectDir("existing-project");
+        createProject(projectDir, new PlatformStreamCoords(null, "0.5"), List.of("ext-a"));
+
+        // valid characters, existing dependency
+        QuarkusCommandOutcome outcome = addExtensions(projectDir, List.of("io.quarkus:quarkus-info:3.20.1"));
+        assertTrue(outcome.isSuccess());
+
+        // invalid characters (tilde in artifactId)
+        outcome = addExtensions(projectDir,
+                List.of("io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4"));
+        assertFalse(outcome.isSuccess());
+
+        // valid characters, questionable dependency
+        outcome = addExtensions(projectDir, List.of("group:artifact:version"));
+        assertTrue(outcome.isSuccess());
+    }
+}