gnodet commented on code in PR #11770:
URL: https://github.com/apache/maven/pull/11770#discussion_r3646437422
##########
maven-core/src/main/java/org/apache/maven/toolchain/RequirementMatcherFactory.java:
##########
@@ -79,6 +81,29 @@ public boolean matches(String requirement) {
}
}
+ private VersionRange convertRequirementToVersionRange(String
requirement)
+ throws InvalidVersionSpecificationException {
+ // Specific for Version _requirement_ matching;
+ // If the version is a simple integer (like "25")
+ // then treat this as the requirement "the major version is 25"
+ if (Pattern.matches("^[0-9]+$", requirement)) {
+ int majorVersion = Integer.parseInt(requirement);
+ return VersionRange.createFromVersionSpec("[" + majorVersion +
"," + (majorVersion + 1) + ")");
+ }
+
+ // If the version is a major.minor (like "1.5")
+ // then treat this as the requirement "the major version is 1 and
the minor is 5"
+ if (Pattern.matches("^[0-9]\\.[0-9]+$", requirement)) {
Review Comment:
Bug: `[0-9]` matches only a single digit for the major version. Requirements
like `"21.3"`, `"17.0"`, or `"11.0"` will not match this pattern and will fall
through to exact-match behavior — exactly the most common modern JDK versions.
The major-only pattern on line 89 correctly uses `[0-9]+`.
```suggestion
if (Pattern.matches("^[0-9]+\\.[0-9]+$", requirement)) {
```
##########
maven-core/src/test/java/org/apache/maven/toolchain/RequirementMatcherFactoryTest.java:
##########
@@ -50,11 +50,18 @@ public void testCreateExactMatcher() {
public void testCreateVersionMatcher() {
RequirementMatcher matcher;
matcher = RequirementMatcherFactory.createVersionMatcher("1.5.2");
- assertFalse(matcher.matches("1.5"));
- assertTrue(matcher.matches("1.5.2"));
+ assertTrue(matcher.matches("1")); // Major matches
+ assertTrue(matcher.matches("1.5")); // Major.Minor matches
+ assertTrue(matcher.matches("1.5.2")); // Full match
Review Comment:
Consider adding test cases with multi-digit major versions to cover the
regex bug above. For example:
```java
matcher = RequirementMatcherFactory.createVersionMatcher("21.0.2");
assertTrue(matcher.matches("21")); // Major-only
assertTrue(matcher.matches("21.0")); // Major.minor
assertFalse(matcher.matches("17")); // Wrong major
```
This would immediately expose the single-digit regex limitation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
