From 1ef739e4093e92748ad2ef617232b6e1e08c5252 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 6 Jun 2024 14:32:40 +0200 Subject: [PATCH 1/3] Drop commons-lang For start, the distro zip lost almost 1 MB (was 10 MB now is 9MB). Second, am really unsure why it was introduced in the first place, as it merely caused confusion, over StringUtils for example. Finally, these "subtle distinctions" in input param validation (like notBlank throwing NPE for null string, and IAEx for blank string) just really makes no sense: a param is either valid (acceptable) or invalid (unacceptable) and failure should happen. --- maven-artifact/pom.xml | 4 --- .../apache/maven/artifact/ArtifactUtils.java | 5 +-- .../versioning/DefaultArtifactVersion.java | 14 ++++++-- maven-core/pom.xml | 4 --- .../DefaultBeanConfigurationRequest.java | 9 ++++-- .../internal/DefaultRuntimeInformation.java | 11 ++++--- .../DefaultRuntimeInformationTest.java | 4 ++- maven-embedder/pom.xml | 4 --- .../apache/maven/cli/CLIReportingUtils.java | 2 +- .../java/org/apache/maven/cli/MavenCli.java | 32 ++++++++----------- .../AbstractMavenTransferListener.java | 22 +++++++------ .../ConsoleMavenTransferListener.java | 18 ++++++++--- .../org/apache/maven/cli/MavenCliTest.java | 5 ++- pom.xml | 16 ---------- 14 files changed, 76 insertions(+), 74 deletions(-) diff --git a/maven-artifact/pom.xml b/maven-artifact/pom.xml index cf2f207d210f..3b43e7807207 100644 --- a/maven-artifact/pom.xml +++ b/maven-artifact/pom.xml @@ -35,10 +35,6 @@ under the License. org.codehaus.plexus plexus-utils - - org.apache.commons - commons-lang3 - diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/ArtifactUtils.java b/maven-artifact/src/main/java/org/apache/maven/artifact/ArtifactUtils.java index 61df919b60c8..f6e5e7d980db 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/ArtifactUtils.java +++ b/maven-artifact/src/main/java/org/apache/maven/artifact/ArtifactUtils.java @@ -25,7 +25,6 @@ import java.util.Map; import java.util.regex.Matcher; -import org.apache.commons.lang3.Validate; import org.apache.maven.artifact.versioning.VersionRange; /** @@ -91,7 +90,9 @@ public static String key(String groupId, String artifactId, String version) { private static void notBlank(String str, String message) { int c = str != null && str.length() > 0 ? str.charAt(0) : 0; if ((c < '0' || c > '9') && (c < 'a' || c > 'z')) { - Validate.notBlank(str, message); + if (str == null || str.trim().isEmpty()) { + throw new IllegalArgumentException(message); + } } } diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/DefaultArtifactVersion.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/DefaultArtifactVersion.java index 10da535c1530..b9f5c3bd06b1 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/DefaultArtifactVersion.java +++ b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/DefaultArtifactVersion.java @@ -20,8 +20,6 @@ import java.util.StringTokenizer; -import static org.apache.commons.lang3.math.NumberUtils.isDigits; - /** * Default implementation of artifact versioning. * @@ -176,6 +174,18 @@ private static Integer getNextIntegerToken(StringTokenizer tok) { return tryParseInt(s); } + private static boolean isDigits(String str) { + if (str == null || str.trim().isEmpty()) { + return false; + } + for (int i = 0; i < str.length(); i++) { + if (!Character.isDigit(str.charAt(i))) { + return false; + } + } + return true; + } + private static Integer tryParseInt(String s) { // for performance, check digits instead of relying later on catching NumberFormatException if (!isDigits(s)) { diff --git a/maven-core/pom.xml b/maven-core/pom.xml index 174a37ae9ef2..4d464c4301d3 100644 --- a/maven-core/pom.xml +++ b/maven-core/pom.xml @@ -131,10 +131,6 @@ under the License. org.codehaus.plexus plexus-component-annotations - - org.apache.commons - commons-lang3 - org.slf4j slf4j-api diff --git a/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java b/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java index 5e5291c1ff2e..a29c2b9d8de1 100644 --- a/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java +++ b/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java @@ -18,7 +18,6 @@ */ package org.apache.maven.configuration; -import org.apache.commons.lang3.Validate; import org.apache.maven.model.Build; import org.apache.maven.model.Model; import org.apache.maven.model.Plugin; @@ -104,8 +103,12 @@ public DefaultBeanConfigurationRequest setConfiguration( } private Plugin findPlugin(Model model, String groupId, String artifactId) { - Validate.notBlank(groupId, "groupId can neither be null, empty nor blank"); - Validate.notBlank(artifactId, "artifactId can neither be null, empty nor blank"); + if (groupId == null || groupId.trim().isEmpty()) { + throw new IllegalArgumentException("groupId can neither be null, empty nor blank"); + } + if (artifactId == null || artifactId.trim().isEmpty()) { + throw new IllegalArgumentException("artifactId can neither be null, empty nor blank"); + } if (model != null) { Build build = model.getBuild(); diff --git a/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java b/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java index f34bb73afa08..dbb04de91177 100644 --- a/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java +++ b/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java @@ -22,8 +22,6 @@ import java.io.InputStream; import java.util.Properties; -import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.Validate; import org.apache.maven.rtinfo.RuntimeInformation; import org.codehaus.plexus.component.annotations.Component; import org.codehaus.plexus.component.annotations.Requirement; @@ -80,10 +78,11 @@ public String getMavenVersion() { } public boolean isMavenVersion(String versionRange) { + if (versionRange == null || versionRange.trim().isEmpty()) { + throw new IllegalArgumentException("versionRange can neither be null, empty nor blank"); + } VersionScheme versionScheme = new GenericVersionScheme(); - Validate.notBlank(versionRange, "versionRange can neither be null, empty nor blank"); - VersionConstraint constraint; try { constraint = versionScheme.parseVersionConstraint(versionRange); @@ -94,7 +93,9 @@ public boolean isMavenVersion(String versionRange) { Version current; try { String mavenVersion = getMavenVersion(); - Validate.validState(StringUtils.isNotEmpty(mavenVersion), "Could not determine current Maven version"); + if (mavenVersion == null || mavenVersion.trim().isEmpty()) { + throw new IllegalStateException("Could not determine current Maven version"); + } current = versionScheme.parseVersion(mavenVersion); } catch (InvalidVersionSpecificationException e) { diff --git a/maven-core/src/test/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformationTest.java b/maven-core/src/test/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformationTest.java index 6a4086e47a7b..c9e43bdf86ba 100644 --- a/maven-core/src/test/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformationTest.java +++ b/maven-core/src/test/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformationTest.java @@ -57,7 +57,9 @@ public void testIsMavenVersion() throws Exception { try { rtInfo.isMavenVersion(null); fail("Bad version range wasn't rejected"); - } catch (NullPointerException e) { + } catch (IllegalArgumentException e) { + // distinction between IAEx and NPE makes no sense: argument is wrong + // moreover, the distinction does not give any benefit either assertTrue(true); } } diff --git a/maven-embedder/pom.xml b/maven-embedder/pom.xml index b3bd623f724f..066c0539e8a8 100644 --- a/maven-embedder/pom.xml +++ b/maven-embedder/pom.xml @@ -148,10 +148,6 @@ under the License. commons-io test - - org.apache.commons - commons-lang3 - org.mockito mockito-core diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java b/maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java index 33d94f370406..4bfd9b2512f5 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java @@ -25,8 +25,8 @@ import java.util.Locale; import java.util.Properties; -import org.apache.commons.lang3.StringUtils; import org.codehaus.plexus.util.Os; +import org.codehaus.plexus.util.StringUtils; import org.slf4j.Logger; import static org.apache.maven.shared.utils.logging.MessageUtils.buffer; diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java index 8a3878878b70..f04676d33aca 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java @@ -51,7 +51,6 @@ import org.apache.commons.cli.Option; import org.apache.commons.cli.ParseException; import org.apache.commons.cli.UnrecognizedOptionException; -import org.apache.commons.lang3.math.NumberUtils; import org.apache.maven.BuildAbort; import org.apache.maven.InternalErrorException; import org.apache.maven.Maven; @@ -1442,27 +1441,24 @@ int calculateDegreeOfConcurrency(String threadConfiguration) { if (threadConfiguration.endsWith("C")) { threadConfiguration = threadConfiguration.substring(0, threadConfiguration.length() - 1); - if (!NumberUtils.isParsable(threadConfiguration)) { - throw new IllegalArgumentException("Invalid threads core multiplier value: '" + threadConfiguration - + "C'. Supported are int and float values ending with C."); - } - - float coreMultiplier = Float.parseFloat(threadConfiguration); + try { + float coreMultiplier = Float.parseFloat(threadConfiguration); - if (coreMultiplier <= 0.0f) { - throw new IllegalArgumentException("Invalid threads core multiplier value: '" + threadConfiguration - + "C'. Value must be positive."); - } + if (coreMultiplier <= 0.0f) { + throw new IllegalArgumentException("Invalid threads core multiplier value: '" + threadConfiguration + + "C'. Value must be positive."); + } - int procs = Runtime.getRuntime().availableProcessors(); - int threads = (int) (coreMultiplier * procs); - return threads == 0 ? 1 : threads; - } else { - if (!NumberUtils.isParsable(threadConfiguration)) { + int procs = Runtime.getRuntime().availableProcessors(); + int threads = (int) (coreMultiplier * procs); + return threads == 0 ? 1 : threads; + } catch (NumberFormatException e) { throw new IllegalArgumentException( - "Invalid threads value: '" + threadConfiguration + "'. Supported are int values."); + "Invalid threads core multiplier value: '" + threadConfiguration + + "C'. Supported are int and float values ending with C.", + e); } - + } else { try { int threads = Integer.parseInt(threadConfiguration); diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/AbstractMavenTransferListener.java b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/AbstractMavenTransferListener.java index e91be5320943..23211917f95e 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/AbstractMavenTransferListener.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/AbstractMavenTransferListener.java @@ -23,7 +23,6 @@ import java.text.DecimalFormatSymbols; import java.util.Locale; -import org.apache.commons.lang3.Validate; import org.apache.maven.shared.utils.logging.MessageUtils; import org.eclipse.aether.transfer.AbstractTransferListener; import org.eclipse.aether.transfer.TransferCancelledException; @@ -105,7 +104,9 @@ public String symbol() { public abstract String symbol(); public static ScaleUnit getScaleUnit(long size) { - Validate.isTrue(size >= 0L, "file size cannot be negative: %s", size); + if (size < 0L) { + throw new IllegalArgumentException("file size cannot be negative: " + size); + } if (size >= GIGABYTE.bytes()) { return GIGABYTE; @@ -137,7 +138,9 @@ public String format(long size, ScaleUnit unit) { @SuppressWarnings("checkstyle:magicnumber") public String format(long size, ScaleUnit unit, boolean omitSymbol) { - Validate.isTrue(size >= 0L, "file size cannot be negative: %s", size); + if (size < 0L) { + throw new IllegalArgumentException("file size cannot be negative: " + size); + } if (unit == null) { unit = ScaleUnit.getScaleUnit(size); @@ -162,12 +165,13 @@ public String format(long size, ScaleUnit unit, boolean omitSymbol) { } public String formatProgress(long progressedSize, long size) { - Validate.isTrue(progressedSize >= 0L, "progressed file size cannot be negative: %s", progressedSize); - Validate.isTrue( - size >= 0L && progressedSize <= size || size < 0L, - "progressed file size cannot be greater than size: %s > %s", - progressedSize, - size); + if (progressedSize < 0L) { + throw new IllegalArgumentException("progressed file size cannot be negative: " + progressedSize); + } + if (size >= 0L && progressedSize > size) { + throw new IllegalArgumentException( + "progressed file size cannot be greater than size: " + progressedSize + " > " + size); + } if (size >= 0L && progressedSize != size) { ScaleUnit unit = ScaleUnit.getScaleUnit(size); diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java index 93144b63b407..e867762ca182 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java @@ -25,7 +25,6 @@ import java.util.Locale; import java.util.Map; -import org.apache.commons.lang3.StringUtils; import org.eclipse.aether.transfer.TransferCancelledException; import org.eclipse.aether.transfer.TransferEvent; import org.eclipse.aether.transfer.TransferResource; @@ -37,10 +36,10 @@ */ public class ConsoleMavenTransferListener extends AbstractMavenTransferListener { - private Map transfers = + private final Map transfers = Collections.synchronizedMap(new LinkedHashMap()); - private boolean printResourceNames; + private final boolean printResourceNames; private int lastLength; public ConsoleMavenTransferListener(PrintStream out, boolean printResourceNames) { @@ -97,7 +96,7 @@ private String getStatus(String resourceName, long complete, long total) { StringBuilder status = new StringBuilder(); if (printResourceNames) { - status.append(StringUtils.substringAfterLast(resourceName, "/")); + status.append(resourceName(resourceName)); status.append(" ("); } @@ -110,6 +109,17 @@ private String getStatus(String resourceName, long complete, long total) { return status.toString(); } + private String resourceName(String resourceName) { + if (resourceName == null || resourceName.trim().isEmpty()) { + return ""; + } + final int pos = resourceName.lastIndexOf("/"); + if (pos == -1 || pos == resourceName.length() - 1) { + return ""; + } + return resourceName.substring(pos + 1); + } + private void pad(StringBuilder buffer, int spaces) { String block = " "; while (spaces > 0) { diff --git a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java index ddeceba5d6db..7c8df4fdf2d9 100644 --- a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java +++ b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java @@ -89,7 +89,10 @@ public void testCalculateDegreeOfConcurrency() { int cpus = Runtime.getRuntime().availableProcessors(); assertEquals((int) (cpus * 2.2), cli.calculateDegreeOfConcurrency("2.2C")); assertEquals(1, cli.calculateDegreeOfConcurrency("0.0001C")); - assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("2.C")); + // Note: this assertion below makes no sense, first Float.parseFloat("2.") DOES parse the string in 2.0. + // Second, the limitation to reject string "2." was actually commons-lang3 NumberUtils.isParsable limitation + // We should not "skew" our input validation by some utility, while using Java classes to perform actual parsing + // assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("2.C")); assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("-2.2C")); assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("0C")); } diff --git a/pom.xml b/pom.xml index 71d0688ccff4..403e2635ede6 100644 --- a/pom.xml +++ b/pom.xml @@ -129,7 +129,6 @@ under the License. 2.8.0 1.8.0 2.16.1 - 3.14.0 4.13.2 2.2 4.11.0 @@ -442,16 +441,6 @@ under the License. commons-cli commons-cli ${commonsCliVersion} - - - commons-lang - commons-lang - - - commons-logging - commons-logging - - commons-io @@ -463,11 +452,6 @@ under the License. commons-jxpath ${jxpathVersion} - - org.apache.commons - commons-lang3 - ${commonsLangVersion} - org.codehaus.plexus plexus-sec-dispatcher From ab5223d5b532bd953d7c70083c40eb4171b24458 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 6 Jun 2024 16:06:48 +0200 Subject: [PATCH 2/3] Do throw NPE to fly --- .../java/org/apache/maven/artifact/ArtifactUtils.java | 5 +++-- .../configuration/DefaultBeanConfigurationRequest.java | 10 ++++++++-- .../rtinfo/internal/DefaultRuntimeInformation.java | 5 ++++- .../rtinfo/internal/DefaultRuntimeInformationTest.java | 4 +--- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/ArtifactUtils.java b/maven-artifact/src/main/java/org/apache/maven/artifact/ArtifactUtils.java index f6e5e7d980db..1ba88a549d8e 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/ArtifactUtils.java +++ b/maven-artifact/src/main/java/org/apache/maven/artifact/ArtifactUtils.java @@ -23,6 +23,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.regex.Matcher; import org.apache.maven.artifact.versioning.VersionRange; @@ -88,9 +89,9 @@ public static String key(String groupId, String artifactId, String version) { } private static void notBlank(String str, String message) { - int c = str != null && str.length() > 0 ? str.charAt(0) : 0; + int c = str != null && !str.isEmpty() ? str.charAt(0) : 0; if ((c < '0' || c > '9') && (c < 'a' || c > 'z')) { - if (str == null || str.trim().isEmpty()) { + if (Objects.requireNonNull(str, message).trim().isEmpty()) { throw new IllegalArgumentException(message); } } diff --git a/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java b/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java index a29c2b9d8de1..e054c8d15840 100644 --- a/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java +++ b/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java @@ -18,6 +18,8 @@ */ package org.apache.maven.configuration; +import java.util.Objects; + import org.apache.maven.model.Build; import org.apache.maven.model.Model; import org.apache.maven.model.Plugin; @@ -103,10 +105,14 @@ public DefaultBeanConfigurationRequest setConfiguration( } private Plugin findPlugin(Model model, String groupId, String artifactId) { - if (groupId == null || groupId.trim().isEmpty()) { + if (Objects.requireNonNull(groupId, "groupId can neither be null, empty nor blank") + .trim() + .isEmpty()) { throw new IllegalArgumentException("groupId can neither be null, empty nor blank"); } - if (artifactId == null || artifactId.trim().isEmpty()) { + if (Objects.requireNonNull(artifactId, "artifactId can neither be null, empty nor blank") + .trim() + .isEmpty()) { throw new IllegalArgumentException("artifactId can neither be null, empty nor blank"); } diff --git a/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java b/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java index dbb04de91177..44a62e5318d5 100644 --- a/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java +++ b/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.io.InputStream; +import java.util.Objects; import java.util.Properties; import org.apache.maven.rtinfo.RuntimeInformation; @@ -78,7 +79,9 @@ public String getMavenVersion() { } public boolean isMavenVersion(String versionRange) { - if (versionRange == null || versionRange.trim().isEmpty()) { + if (Objects.requireNonNull(versionRange, "versionRange can neither be null, empty nor blank") + .trim() + .isEmpty()) { throw new IllegalArgumentException("versionRange can neither be null, empty nor blank"); } VersionScheme versionScheme = new GenericVersionScheme(); diff --git a/maven-core/src/test/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformationTest.java b/maven-core/src/test/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformationTest.java index c9e43bdf86ba..6a4086e47a7b 100644 --- a/maven-core/src/test/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformationTest.java +++ b/maven-core/src/test/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformationTest.java @@ -57,9 +57,7 @@ public void testIsMavenVersion() throws Exception { try { rtInfo.isMavenVersion(null); fail("Bad version range wasn't rejected"); - } catch (IllegalArgumentException e) { - // distinction between IAEx and NPE makes no sense: argument is wrong - // moreover, the distinction does not give any benefit either + } catch (NullPointerException e) { assertTrue(true); } } From 967e88176f2700e596ae208b4c4ee3a2fb277160 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 6 Jun 2024 19:19:44 +0200 Subject: [PATCH 3/3] PR comments --- .../versioning/DefaultArtifactVersion.java | 3 ++- .../versioning/DefaultArtifactVersionTest.java | 9 +++++++++ .../DefaultBeanConfigurationRequest.java | 15 +++++++-------- .../internal/DefaultRuntimeInformation.java | 6 ++++-- .../java/org/apache/maven/cli/MavenCliTest.java | 4 ---- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/DefaultArtifactVersion.java b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/DefaultArtifactVersion.java index b9f5c3bd06b1..d336625023d8 100644 --- a/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/DefaultArtifactVersion.java +++ b/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/DefaultArtifactVersion.java @@ -179,7 +179,8 @@ private static boolean isDigits(String str) { return false; } for (int i = 0; i < str.length(); i++) { - if (!Character.isDigit(str.charAt(i))) { + char c = str.charAt(i); + if (!(c >= '0' && c <= '9')) { return false; } } diff --git a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java index a1b74124139a..368db34f6028 100644 --- a/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java +++ b/maven-artifact/src/test/java/org/apache/maven/artifact/versioning/DefaultArtifactVersionTest.java @@ -80,6 +80,15 @@ public void testVersionParsing() { checkVersionParsing("1.2.3-200705301630", 1, 2, 3, 0, "200705301630"); } + public void testVersionParsingNot09() { + String ver = "१.२.३"; + assertTrue(Character.isDigit(ver.charAt(0))); + assertTrue(Character.isDigit(ver.charAt(2))); + assertTrue(Character.isDigit(ver.charAt(4))); + ArtifactVersion version = newArtifactVersion(ver); + assertEquals(ver, version.getQualifier()); + } + public void testVersionComparing() { assertVersionEqual("1", "1"); assertVersionOlder("1", "2"); diff --git a/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java b/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java index e054c8d15840..9584cd2e2388 100644 --- a/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java +++ b/maven-core/src/main/java/org/apache/maven/configuration/DefaultBeanConfigurationRequest.java @@ -104,16 +104,15 @@ public DefaultBeanConfigurationRequest setConfiguration( return this; } + private static final String GROUP_ID_ERROR_MESSAGE = "groupId can neither be null, empty, nor blank"; + private static final String ARTIFACT_ID_ERROR_MESSAGE = "artifactId can neither be null, empty, nor blank"; + private Plugin findPlugin(Model model, String groupId, String artifactId) { - if (Objects.requireNonNull(groupId, "groupId can neither be null, empty nor blank") - .trim() - .isEmpty()) { - throw new IllegalArgumentException("groupId can neither be null, empty nor blank"); + if (Objects.requireNonNull(groupId, GROUP_ID_ERROR_MESSAGE).trim().isEmpty()) { + throw new IllegalArgumentException(GROUP_ID_ERROR_MESSAGE); } - if (Objects.requireNonNull(artifactId, "artifactId can neither be null, empty nor blank") - .trim() - .isEmpty()) { - throw new IllegalArgumentException("artifactId can neither be null, empty nor blank"); + if (Objects.requireNonNull(artifactId, ARTIFACT_ID_ERROR_MESSAGE).trim().isEmpty()) { + throw new IllegalArgumentException(ARTIFACT_ID_ERROR_MESSAGE); } if (model != null) { diff --git a/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java b/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java index 44a62e5318d5..1a9da2f6992e 100644 --- a/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java +++ b/maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java @@ -78,11 +78,13 @@ public String getMavenVersion() { return mavenVersion; } + private static final String VERSION_RANGE_ERROR_MESSAGE = "versionRange can neither be null, empty, nor blank"; + public boolean isMavenVersion(String versionRange) { - if (Objects.requireNonNull(versionRange, "versionRange can neither be null, empty nor blank") + if (Objects.requireNonNull(versionRange, VERSION_RANGE_ERROR_MESSAGE) .trim() .isEmpty()) { - throw new IllegalArgumentException("versionRange can neither be null, empty nor blank"); + throw new IllegalArgumentException(VERSION_RANGE_ERROR_MESSAGE); } VersionScheme versionScheme = new GenericVersionScheme(); diff --git a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java index 7c8df4fdf2d9..0005079fd2fe 100644 --- a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java +++ b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java @@ -89,10 +89,6 @@ public void testCalculateDegreeOfConcurrency() { int cpus = Runtime.getRuntime().availableProcessors(); assertEquals((int) (cpus * 2.2), cli.calculateDegreeOfConcurrency("2.2C")); assertEquals(1, cli.calculateDegreeOfConcurrency("0.0001C")); - // Note: this assertion below makes no sense, first Float.parseFloat("2.") DOES parse the string in 2.0. - // Second, the limitation to reject string "2." was actually commons-lang3 NumberUtils.isParsable limitation - // We should not "skew" our input validation by some utility, while using Java classes to perform actual parsing - // assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("2.C")); assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("-2.2C")); assertThrows(IllegalArgumentException.class, new ConcurrencyCalculator("0C")); }