You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by cs...@apache.org on 2024/06/06 16:51:55 UTC
(maven) branch master updated: [MNG-8142] Hidden bug: JDK profile activator throw NumberFormatEx (#1557)
This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new 9db546ddb9 [MNG-8142] Hidden bug: JDK profile activator throw NumberFormatEx (#1557)
9db546ddb9 is described below
commit 9db546ddb905392c6c4df9484895bf11182ab303
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Thu Jun 6 18:51:39 2024 +0200
[MNG-8142] Hidden bug: JDK profile activator throw NumberFormatEx (#1557)
If property `java.version` is in unexpected format, the activator throws `NumberFormatEx`, that in turn, is caught and reported by `DefaultProfileSelector` w/o any cause.
These should be cleanly reported instead: report that `java.version` property is in "unexpected format", and also report why was there are failure to evaluate a property activation.
Note 1: Maven allows `-Djava.version` override (!!!), this is exactly what IT MNG-3746 does, but the `NumberFormatEx` went unnoticed, was swallowed, no cause reported.
Note 2: This bug was revealed by #1555 as it reported the issue, and later "asserted error free log" which was not error-free. Hence, this bug was simply revealed by improved logging on unrelated issue.
---
https://issues.apache.org/jira/browse/MNG-8142
---
.../impl/model/DefaultProfileSelector.java | 2 +-
.../model/profile/JdkVersionProfileActivator.java | 12 +++-
.../model/profile/DefaultProfileSelector.java | 3 +-
.../activation/JdkVersionProfileActivator.java | 10 ++-
.../model/profile/DefaultProfileSelectorTest.java | 72 ++++++++++++++++++++++
.../activation/JdkVersionProfileActivatorTest.java | 25 ++++++++
6 files changed, 120 insertions(+), 4 deletions(-)
diff --git a/maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileSelector.java b/maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileSelector.java
index e451956e50..c15513e1ec 100644
--- a/maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileSelector.java
+++ b/maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultProfileSelector.java
@@ -106,7 +106,7 @@ public class DefaultProfileSelector implements ProfileSelector {
problems.add(
Severity.ERROR,
Version.BASE,
- "Failed to determine activation for profile " + profile.getId(),
+ "Failed to determine activation for profile " + profile.getId() + ": " + e.getMessage(),
profile.getLocation(""),
e);
return false;
diff --git a/maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/profile/JdkVersionProfileActivator.java b/maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/profile/JdkVersionProfileActivator.java
index 62e288e294..2a417a3bc6 100644
--- a/maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/profile/JdkVersionProfileActivator.java
+++ b/maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/profile/JdkVersionProfileActivator.java
@@ -70,7 +70,17 @@ public class JdkVersionProfileActivator implements ProfileActivator {
activation.getLocation("jdk"));
return false;
}
- return isJavaVersionCompatible(jdk, version);
+ try {
+ return isJavaVersionCompatible(jdk, version);
+ } catch (NumberFormatException e) {
+ problems.add(
+ BuilderProblem.Severity.WARNING,
+ ModelProblem.Version.BASE,
+ "Failed to determine JDK activation for profile " + profile.getId() + " due invalid JDK version: '"
+ + version + "'",
+ profile.getLocation("jdk"));
+ return false;
+ }
}
public static boolean isJavaVersionCompatible(String requiredJdkRange, String currentJavaVersion) {
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileSelector.java b/maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileSelector.java
index b2d0e9a801..3ccfb261c7 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileSelector.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileSelector.java
@@ -119,7 +119,8 @@ public class DefaultProfileSelector implements ProfileSelector {
}
} catch (RuntimeException e) {
problems.add(new ModelProblemCollectorRequest(Severity.ERROR, Version.BASE)
- .setMessage("Failed to determine activation for profile " + profile.getId())
+ .setMessage("Failed to determine activation for profile " + profile.getId() + ": "
+ + e.getMessage())
.setLocation(profile.getLocation(""))
.setException(e));
return false;
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/profile/activation/JdkVersionProfileActivator.java b/maven-model-builder/src/main/java/org/apache/maven/model/profile/activation/JdkVersionProfileActivator.java
index 23f4320866..378550fc56 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/profile/activation/JdkVersionProfileActivator.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/profile/activation/JdkVersionProfileActivator.java
@@ -69,7 +69,15 @@ public class JdkVersionProfileActivator implements ProfileActivator {
.setLocation(activation.getLocation("jdk")));
return false;
}
- return isJavaVersionCompatible(jdk, version);
+ try {
+ return isJavaVersionCompatible(jdk, version);
+ } catch (NumberFormatException e) {
+ problems.add(new ModelProblemCollectorRequest(Severity.WARNING, Version.BASE)
+ .setMessage("Failed to determine JDK activation for profile " + profile.getId()
+ + " due invalid JDK version: '" + version + "'")
+ .setLocation(activation.getLocation("jdk")));
+ return false;
+ }
}
public static boolean isJavaVersionCompatible(String requiredJdkRange, String currentJavaVersion) {
diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/profile/DefaultProfileSelectorTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/profile/DefaultProfileSelectorTest.java
new file mode 100644
index 0000000000..9b86ef8401
--- /dev/null
+++ b/maven-model-builder/src/test/java/org/apache/maven/model/profile/DefaultProfileSelectorTest.java
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.model.profile;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.maven.model.Activation;
+import org.apache.maven.model.Profile;
+import org.apache.maven.model.building.ModelProblemCollector;
+import org.apache.maven.model.building.SimpleProblemCollector;
+import org.apache.maven.model.profile.activation.ProfileActivator;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests {@link DefaultProfileSelector}.
+ */
+public class DefaultProfileSelectorTest {
+ private Profile newProfile(String id) {
+ Activation activation = new Activation();
+ Profile profile = new Profile();
+ profile.setId(id);
+ profile.setActivation(activation);
+ return profile;
+ }
+
+ @Test
+ void testThrowingActivator() {
+ DefaultProfileSelector selector = new DefaultProfileSelector();
+ selector.addProfileActivator(new ProfileActivator() {
+ @Override
+ public boolean isActive(Profile profile, ProfileActivationContext context, ModelProblemCollector problems) {
+ throw new RuntimeException("BOOM");
+ }
+
+ @Override
+ public boolean presentInConfig(
+ Profile profile, ProfileActivationContext context, ModelProblemCollector problems) {
+ return true;
+ }
+ });
+
+ List<Profile> profiles = Collections.singletonList(newProfile("one"));
+ DefaultProfileActivationContext context = new DefaultProfileActivationContext();
+ SimpleProblemCollector problems = new SimpleProblemCollector();
+ List<Profile> active = selector.getActiveProfiles(profiles, context, problems);
+ assertTrue(active.isEmpty());
+ assertEquals(1, problems.getErrors().size());
+ assertEquals(
+ "Failed to determine activation for profile one: BOOM",
+ problems.getErrors().get(0));
+ }
+}
diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/profile/activation/JdkVersionProfileActivatorTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/profile/activation/JdkVersionProfileActivatorTest.java
index 74d6f19478..5d982478dd 100644
--- a/maven-model-builder/src/test/java/org/apache/maven/model/profile/activation/JdkVersionProfileActivatorTest.java
+++ b/maven-model-builder/src/test/java/org/apache/maven/model/profile/activation/JdkVersionProfileActivatorTest.java
@@ -22,9 +22,13 @@ import java.util.Properties;
import org.apache.maven.api.model.Activation;
import org.apache.maven.api.model.Profile;
+import org.apache.maven.model.building.SimpleProblemCollector;
+import org.apache.maven.model.profile.ProfileActivationContext;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+
/**
* Tests {@link JdkVersionProfileActivator}.
*
@@ -169,4 +173,25 @@ class JdkVersionProfileActivatorTest extends AbstractProfileActivatorTest<JdkVer
assertActivation(false, profile, newContext(null, newProperties("1.6.0_09")));
assertActivation(false, profile, newContext(null, newProperties("1.6.0_09-b03")));
}
+
+ @Test
+ void testRubbishJavaVersion() {
+ Profile profile = newProfile("[1.8,)");
+
+ assertActivationWithProblems(profile, newContext(null, newProperties("Pūteketeke")), "invalid JDK version");
+ assertActivationWithProblems(profile, newContext(null, newProperties("rubbish")), "invalid JDK version");
+ assertActivationWithProblems(profile, newContext(null, newProperties("1.a.0_09")), "invalid JDK version");
+ assertActivationWithProblems(profile, newContext(null, newProperties("1.a.2.b")), "invalid JDK version");
+ }
+
+ private void assertActivationWithProblems(
+ Profile profile, ProfileActivationContext context, String warningContains) {
+ SimpleProblemCollector problems = new SimpleProblemCollector();
+
+ assertFalse(activator.isActive(new org.apache.maven.model.Profile(profile), context, problems));
+
+ assertEquals(0, problems.getErrors().size());
+ assertEquals(1, problems.getWarnings().size());
+ assertTrue(problems.getWarnings().get(0).contains(warningContains));
+ }
}