Check for invalid characters when adding an extension #50419

nderwin · 2025-10-05T23:52:28Z

Fixes Quarkus CLI does not check for special characters when adding an extension #49895
use a pattern matcher to validate the groupId and artifactId for a fully qualified extension name (group, artifact, and version)

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

nderwin · 2025-10-05T23:57:30Z

I'm not sure I'm fully happy with this as it still can lead to bad dependencies being added because when given a GAV, it just adds it, and you'll find out it's bad with your first Maven command.

Regardless, some testing scenarios...

java -jar quarkus-cli-999-SNAPSHOT-runner.jar ext add io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4
[ERROR] ❗  Nothing installed because keyword(s) 'io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4' were not matched in the catalog.

Note an invalid dependency, but it has valid characters in it:

java -jar quarkus-cli-999-SNAPSHOT-runner.jar ext add io.quarkus:quarkus-info-:3.20.1
[SUCCESS] ✅  Extension io.quarkus:quarkus-info-:3.20.1 has been installed

Note a really invalid dependency:

java -jar quarkus-cli-999-SNAPSHOT-runner.jar ext add group:artifact:version
[SUCCESS] ✅  Extension group:artifact:version has been installed

If you leave off the version, then the dependency is validated against the catalog, so it would have given an error for adding 'io.quarkiverse.businessscore:quarkus-business-score-health~

gsmet

I haven't checked the code but is there a reason why we don't send an explicit error?

gsmet · 2025-10-06T14:41:35Z

Thanks for having a look at this one btw :)

nderwin · 2025-10-06T15:35:07Z

I haven't checked the code but is there a reason why we don't send an explicit error?

Yeah, that would be nicer, right? Unfortunately, it goes back to ExtensionInstallPlan - it has a provision for just a collection of unmatched keys, no collection of errors.

I stopped short of adding that as it touches many more classes, but if you think it's worth it, I can take a look at that.

nderwin · 2025-10-06T17:22:04Z

@gsmet Meh, it was easier than I thought, so I just went ahead and added it. Example now:

java -jar quarkus-cli-999-SNAPSHOT-runner.jar ext add io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4
[FAILURE] ❌  Nothing installed because keyword(s) 'io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4' were invalid.

gastaldi · 2025-10-09T14:23:30Z

Can you include some tests to justify the change?

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

nderwin · 2025-10-09T15:02:17Z

Can you include some tests to justify the change?

Sure, just as soon as I figure out where to put 'em.

* Fixes quarkusio#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]>

nderwin · 2025-10-09T19:18:10Z

Ok @gastaldi I took a swipe at adding tests, maybe it's ok? Not really sure...

gastaldi

LGTM, thanks!

quarkus-bot · 2025-10-09T22:21:54Z

Status for workflow `Quarkus CI`

This is the status report for running Quarkus CI on commit 39f9848.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging/deployment

❌ io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector - History

Expecting actual: ["-4","-5","-6","-7","-8","-9","-10","-11"] to start with: ["-3", "-4", "-5", "-6"] - java.lang.AssertionError

java.lang.AssertionError: 

Expecting actual:
  ["-4","-5","-6","-7","-8","-9","-10","-11"]
to start with:
  ["-3", "-4", "-5", "-6"]

	at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:36)

⚙️ JVM Tests - JDK 21

📦 extensions/infinispan-cache/deployment

❌ io.quarkus.cache.infinispan.InfinispanCacheTest.testGetAsyncWithParallelCalls - History

expected: "thread1" but was: "thread2" - org.opentest4j.AssertionFailedError

org.opentest4j.AssertionFailedError: 

expected: "thread1"
 but was: "thread2"
	at io.quarkus.cache.infinispan.InfinispanCacheTest.testGetAsyncWithParallelCalls(InfinispanCacheTest.java:285)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:534)
	at io.quarkus.test.QuarkusUnitTest.interceptTestMethod(QuarkusUnitTest.java:448)

⚙️ JVM Tests - JDK 25

📦 extensions/smallrye-graphql/deployment

❌ io.quarkus.smallrye.graphql.deployment.CompletionStageTest.testSourcePost - History

1 expectation failed. Expected status code <200> but was <500>. - java.lang.AssertionError

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

	at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:483)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:73)

aloubyansky · 2025-10-13T07:47:33Z

@nderwin It's ok to split the task into validating the strings and where an artifact is actually resolvable.

I am not sure why

java -jar quarkus-cli-999-SNAPSHOT-runner.jar ext add io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4
[ERROR] ❗  Nothing installed because keyword(s) 'io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4' were not matched in the catalog.

is not reported as invalid in terms of pattern.

Is - at the end actually allowed? I don't see it being documented, e.g. here https://maven.apache.org/guides/mini/guide-naming-conventions.html, but i guess, it wouldn't be. The same for a . at the end of a groupId.

java -jar quarkus-cli-999-SNAPSHOT-runner.jar ext add io.quarkus:quarkus-info-:3.20.1
[SUCCESS] ✅  Extension io.quarkus:quarkus-info-:3.20.1 has been installed

Could we adjust the regex pattern to account for that? Thanks!

nderwin · 2025-10-13T11:17:50Z

@aloubyansky The issue with the first error you mention isn't the position, it's the ~ (tilde) - that's not a valid character for a Maven groupId or artifactId.

True, it's weird to have one of those values end with a - or ., but at least they are valid characters, and will be validated when you try to reference the artifact during a Maven lifecycle.

I'll do some further digging to see how Maven itself validates this, maybe there's something that can be shared there (if not code, then at least ideas).

aloubyansky · 2025-10-13T11:20:57Z

Is

java -jar quarkus-cli-999-SNAPSHOT-runner.jar ext add io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4
[ERROR] ❗ Nothing installed because keyword(s) 'io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4' were not matched in the catalog.

The expected outcome though?

nderwin · 2025-10-13T11:26:34Z

@aloubyansky No, this should be the expected output for that case:

java -jar quarkus-cli-999-SNAPSHOT-runner.jar ext add io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4
[FAILURE] ❌  Nothing installed because keyword(s) 'io.quarkiverse.businessscore:quarkus-business-score-health~:1.0.0.Alpha4' were invalid.

I had updated the PR from the comment by @gsmet wondering why it was mixed in with the other errors.

aloubyansky · 2025-10-13T11:33:04Z

Perfect, thanks!
Would you like to adjust the regex to make sure groupIds don't end with . and artifactIds don't end with -?

nderwin · 2025-10-13T11:42:17Z

@aloubyansky Well... I'm not sure we should? Based on what I found for how Maven does it's validation, it doesn't actually care, even though the documentation talks about the conventions.

https://github.com/apache/maven/blob/maven-3.9.11/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java

I do see in there that it's also validating some wildcard characters, ? and *, so maybe those do need to be added to the regex?

aloubyansky · 2025-10-13T12:09:38Z

Unless I'm mistaken, this is the check https://github.com/apache/maven/blob/maven-3.9.11/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java#L1158-L1170

    private boolean isValidId(String id) {
        for (int i = 0; i < id.length(); i++) {
            char c = id.charAt(i);
            if (!isValidIdCharacter(c)) {
                return false;
            }
        }
        return true;
    }

    private boolean isValidIdCharacter(char c) {
        return c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-' || c == '_' || c == '.';
    }

nderwin · 2025-10-13T12:18:27Z

Agreed, that's what I saw; I also saw this in there: https://github.com/apache/maven/blob/maven-3.9.11/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java#L1200-L1212

    private boolean isValidIdWithWildCards(String id) {
        for (int i = 0; i < id.length(); i++) {
            char c = id.charAt(i);
            if (!isValidIdWithWildCardCharacter(c)) {
                return false;
            }
        }
        return true;
    }


    private boolean isValidIdWithWildCardCharacter(char c) {
        return isValidIdCharacter(c) || c == '?' || c == '*';
    }

aloubyansky · 2025-10-13T13:31:04Z

Alright, let's get this in for now. Thanks a lot @nderwin !

nderwin · 2025-10-13T13:31:10Z

Though it looks like the wildcard validation is for dependency exclusions, which is not what the add ext command does, so maybe that can be ignored.

quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform labels Oct 5, 2025

gsmet reviewed Oct 6, 2025

View reviewed changes

nderwin force-pushed the bugfix/cli-special-character-check branch from f981842 to 7fcf461 Compare October 6, 2025 17:20

nderwin requested a review from gsmet October 6, 2025 17:22

nderwin force-pushed the bugfix/cli-special-character-check branch 2 times, most recently from 63aa9ad to cc50d1a Compare October 9, 2025 11:38

gastaldi reviewed Oct 9, 2025

View reviewed changes

...-common/src/main/java/io/quarkus/devtools/commands/handlers/AddExtensionsCommandHandler.java Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

quarkus-bot bot added the triage/flaky-test label Oct 9, 2025

nderwin force-pushed the bugfix/cli-special-character-check branch from cc50d1a to 39f9848 Compare October 9, 2025 19:17

nderwin requested a review from gastaldi October 9, 2025 19:17

gastaldi approved these changes Oct 9, 2025

View reviewed changes

gastaldi requested a review from aloubyansky October 9, 2025 22:44

aloubyansky merged commit dabb86d into quarkusio:main Oct 13, 2025
59 checks passed

quarkus-bot bot added this to the 3.29 - main milestone Oct 13, 2025

quarkus-bot bot added the kind/bugfix label Oct 13, 2025

nderwin deleted the bugfix/cli-special-character-check branch October 13, 2025 13:31

Check for invalid characters when adding an extension #50419

Check for invalid characters when adding an extension #50419

Conversation

nderwin commented Oct 5, 2025

Uh oh!

nderwin commented Oct 5, 2025

Uh oh!

gsmet left a comment

Choose a reason for hiding this comment

Uh oh!

gsmet commented Oct 6, 2025

Uh oh!

nderwin commented Oct 6, 2025

Uh oh!

nderwin commented Oct 6, 2025

Uh oh!

gastaldi commented Oct 9, 2025

Uh oh!

Uh oh!

nderwin commented Oct 9, 2025

Uh oh!

This comment has been minimized.

nderwin commented Oct 9, 2025

Uh oh!

gastaldi left a comment

Choose a reason for hiding this comment

Uh oh!

quarkus-bot bot commented Oct 9, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status for workflow Quarkus CI

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging/deployment

⚙️ JVM Tests - JDK 21

📦 extensions/infinispan-cache/deployment

⚙️ JVM Tests - JDK 25

📦 extensions/smallrye-graphql/deployment

Uh oh!

aloubyansky commented Oct 13, 2025

Uh oh!

nderwin commented Oct 13, 2025

Uh oh!

aloubyansky commented Oct 13, 2025

Uh oh!

nderwin commented Oct 13, 2025

Uh oh!

aloubyansky commented Oct 13, 2025

Uh oh!

nderwin commented Oct 13, 2025

Uh oh!

aloubyansky commented Oct 13, 2025

Uh oh!

nderwin commented Oct 13, 2025

Uh oh!

aloubyansky commented Oct 13, 2025

Uh oh!

nderwin commented Oct 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

quarkus-bot bot commented Oct 9, 2025 •

edited by github-actions bot

Loading

Status for workflow `Quarkus CI`