From 94b01a216c5eca2d4898b097180b876cdedd5ebd Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 5 Jun 2024 14:35:42 +0200 Subject: [PATCH 1/3] [MNG-8142] Hidden bug: JDK profile activator throww NumberFormatEx 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 properrty activation. Note: Maven allows `-Djava.version` override, this is exactly what IT MNG-3746 exactly does, but the NumberFormatEx went unnoticed (was swallowed). --- https://issues.apache.org/jira/browse/MNG-8142 --- .../maven/model/profile/DefaultProfileSelector.java | 3 ++- .../activation/JdkVersionProfileActivator.java | 11 ++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) 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 2d713b563d77..786efad4d824 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 @@ -103,7 +103,8 @@ private boolean isActive(Profile profile, ProfileActivationContext context, Mode } } 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 e2a5fe2d62d0..1d92c7d1a7d7 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 @@ -74,7 +74,16 @@ public boolean isActive(Profile profile, ProfileActivationContext context, Model if (jdk.startsWith("!")) { return !version.startsWith(jdk.substring(1)); } else if (isRange(jdk)) { - return isInRange(version, getRange(jdk)); + try { + return isInRange(version, getRange(jdk)); + } 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(profile.getLocation("")) + .setException(e)); + return false; + } } else { return version.startsWith(jdk); } From 08806f2b338706029afa474cd106ade083d6d201 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 6 Jun 2024 11:45:36 +0200 Subject: [PATCH 2/3] Add UT --- .../JdkVersionProfileActivatorTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) 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 fd7730ae3695..766fe43e0e96 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,6 +22,8 @@ import org.apache.maven.model.Activation; import org.apache.maven.model.Profile; +import org.apache.maven.model.building.SimpleProblemCollector; +import org.apache.maven.model.profile.ProfileActivationContext; /** * Tests {@link JdkVersionProfileActivator}. @@ -161,4 +163,25 @@ public void testVersionRangeExclusiveUpperBound() throws Exception { assertActivation(false, profile, newContext(null, newProperties("1.6.0_09"))); assertActivation(false, profile, newContext(null, newProperties("1.6.0_09-b03"))); } + + public 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(); + + assertEquals(false, activator.isActive(profile, context, problems)); + + assertEquals(problems.getErrors().toString(), 0, problems.getErrors().size()); + assertEquals( + problems.getWarnings().toString(), 1, problems.getWarnings().size()); + assertTrue(problems.getWarnings().get(0), problems.getWarnings().get(0).contains(warningContains)); + } } From 78bfbd6b127f41ac02a033a491ebddfddebb3f11 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 6 Jun 2024 11:58:28 +0200 Subject: [PATCH 3/3] Add Selector UT --- .../profile/DefaultProfileSelectorTest.java | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 maven-model-builder/src/test/java/org/apache/maven/model/profile/DefaultProfileSelectorTest.java 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 000000000000..51de2aeb91ff --- /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.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.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 + public 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 profiles = Collections.singletonList(newProfile("one")); + DefaultProfileActivationContext context = new DefaultProfileActivationContext(); + SimpleProblemCollector problems = new SimpleProblemCollector(); + List 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)); + } +}