elharo commented on code in PR #12947:
URL: https://github.com/apache/maven/pull/12947#discussion_r3893987547
##########
compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -68,6 +68,16 @@ public class ComparableVersion implements
Comparable<ComparableVersion> {
private static final int MAX_LONGITEM_LENGTH = 18;
+ /**
+ * Maximum accepted length of a version string. Version strings routinely
come from external
+ * repository metadata; without a bound, every {@code -} separator nests
another list whose
Review Comment:
metadata; without --> metadata. Without
##########
compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -808,6 +835,53 @@ public int hashCode() {
return items.hashCode();
}
+ /**
+ * Returns a hash code consistent with the ordering defined by {@link
#compareTo(ComparableVersion)}:
+ * two versions that compare as equal get the same value even when their
parsed representations
+ * differ, e.g. {@code 1-ga} ({@code [1, [ga]]}) and {@code 1} ({@code
[1]}).
+ * <p>
+ * {@link #hashCode()} cannot provide this: it is structural, matching the
structural
+ * {@link #equals(Object)} of this class. This method exists for classes
such as
+ * {@link DefaultArtifactVersion} whose {@code equals} is defined as
{@code compareTo == 0} and
+ * whose {@code hashCode} must therefore follow ordering equality (two
equal objects must have
+ * equal hash codes).
+ *
+ * @return a hash code such that {@code a.compareTo(b) == 0} implies
{@code a.orderingHashCode() == b.orderingHashCode()}
+ */
+ int orderingHashCode() {
+ return orderingHash(items);
+ }
+
+ private static int orderingHash(Item item) {
+ return switch (item.getType()) {
+ case Item.LIST_ITEM -> {
+ ListItem list = (ListItem) item;
+ int end = list.size();
+ // trailing items that compare as equal to null do not affect
ordering: 1-ga == 1
+ while (end > 0 && list.get(end - 1).compareTo(null) == 0) {
+ end--;
+ }
+ int hash = 1;
+ for (int i = 0; i < end; i++) {
+ hash = 31 * hash + orderingHash(list.get(i));
+ }
+ yield hash;
+ }
+ // qualifiers that compare as equal ("ga", "final", "release" and
the empty qualifier) must hash alike
Review Comment:
good catch
##########
compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -450,7 +460,14 @@ private static class CombinationItem implements Item {
public int compareTo(Item item) {
if (item == null) {
// 1-rc1 < 1, 1-ga1 > 1
- return stringPart.compareTo(item);
+ int result = stringPart.compareTo(item);
+ if (result == 0) {
+ // the string part is equivalent to the release qualifier
("ga", "final", "release"),
Review Comment:
have we tested 1-ga2 vs 1-ga11?
##########
compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -68,6 +68,16 @@ public class ComparableVersion implements
Comparable<ComparableVersion> {
private static final int MAX_LONGITEM_LENGTH = 18;
+ /**
+ * Maximum accepted length of a version string. Version strings routinely
come from external
+ * repository metadata; without a bound, every {@code -} separator nests
another list whose
+ * comparison, equality, hash code and canonicalization recurse one frame
per level, and digit
+ * runs longer than {@value #MAX_LONGITEM_LENGTH} characters are parsed
into {@link BigInteger}
+ * at quadratic cost. 256 characters is far beyond any real-world version
identifier while
+ * keeping the nesting depth (at most about half the length) and numeric
items small.
+ */
+ private static final int MAX_VERSION_LENGTH = 256;
Review Comment:
I don't know. I'd probably take a 50-50 bet that there are longer version
strings out there. Would it be feasible to go to 1024? or higher? At what
length do we actually have problems?
##########
compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -68,6 +68,16 @@ public class ComparableVersion implements
Comparable<ComparableVersion> {
private static final int MAX_LONGITEM_LENGTH = 18;
+ /**
+ * Maximum accepted length of a version string. Version strings routinely
come from external
+ * repository metadata; without a bound, every {@code -} separator nests
another list whose
+ * comparison, equality, hash code and canonicalization recurse one frame
per level, and digit
Review Comment:
hash code, and
--
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]