[MNG-8241] ComparableVersion incorrectly handles Unicode non-BMP characters

[jira-importer]

Matthew Donoughe opened MNG-8241 and commented

Java strings are (usually) Unicode, but Java chars are a subset of Unicode. ComparableVersion makes heavy use of String.charAt, which will return surrogate values instead of Unicode code points whenever a character takes more than 16 bits.

This leads to the following behavior:

 

java -jar ~/.m2/repository/org/apache/maven/maven-artifact/3.9.4/maven-artifact-3.9.4.jar 1 𝟤
Display parameters as parsed by Maven (in canonical form and as a list of tokens) and comparison result:
1. 1 -> 1; tokens: [1]
   1 > 𝟤
2. 𝟤 -> 𝟤; tokens: [𝟤]

1 (DIGIT ONE) > 𝟤 (MATHEMATICAL SANS-SERIF DIGIT TWO) because ComparableVersion sees 𝟤 as two invalid characters and treats it as text.

 

 

java -jar ~/.m2/repository/org/apache/maven/maven-artifact/3.9.4/maven-artifact-3.9.4.jar 0 𝟤
Display parameters as parsed by Maven (in canonical form and as a list of tokens) and comparison result:
1. 0 -> ; tokens: []
   0 < 𝟤
2. 𝟤 -> 𝟤; tokens: [𝟤]

However, 0 (DIGIT 0) is still < 𝟤 (MATHEMATICAL SANS-SERIF DIGIT TWO). 0 < 𝟤 < 1 the same way 0 < a < 1.

 

It's unclear whether this should be considered to be a bug or whether it's just an undocumented limitation. String.charAt and String.length should be avoided unless you can be sure the characters are all BMP (Basic Multilingual Plane).

I was initially worried that 𝟣𝟣𝟣𝟣𝟣 (MATHEMATICAL SANS-SERIF DIGIT ONE) > 22222 (DIGIT TWO) because "𝟣𝟣𝟣𝟣𝟣".length is 10, greater than MAX_INTITEM_LENGTH, but that code doesn't even get hit because String.charAt is producing effectively "�����������". If the code is changed to identify non-BMP Nd class digits like 𝟣 as digits then the code that determines the required size of the data type needs to be updated to measure the length in code points instead of chars.

---

Remote Links:

• GitHub Pull Request #661
  
• GitHub Pull Request #2071
  
• GitHub Pull Request #2081
  

[jira-importer]

Guillaume Nodet commented

I think we should fix the code to support non-BMP characters, but I would not go as far as trying to understand all kind of digits. I just discovered the Character.getNumericValue() method which could be used, but it also supports latin digits or hexadecimal, so the results may quickly be unexpected.

[jira-importer]

Elliotte Rusty Harold commented

We need to check what Semantic Versioning spec suggests. We don't promise to follow it, and there are differences, but we shouldn't deviate from it accidentally.

Not a lot of help there it seems: "Identifiers MUST comprise only ASCII alphanumerics and hyphens [0-9A-Za-z-]."

[jira-importer]

Elliotte Rusty Harold commented

See

https://maven.apache.org/ref/3.9.9/maven-artifact/apidocs/org/apache/maven/artifact/versioning/ComparableVersion.html
https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning
https://maven.apache.org/pom.html#Version_Order_Specification

[jira-importer]

Tamas Cservenak commented

The maven-artifact is deprecated in Maven 4. Should be considered "frozen" in Maven 3 (as Maven 3 will soon become, or is already "bugfix only" mode).

The proper version spec and implementation is in Reaolver, see here:
https://github.com/apache/maven-resolver/blob/1c580b0165e73492e7cf23a88f548056e07e2559/maven-resolver-util/src/main/java/org/eclipse/aether/util/version/package-info.java

[jira-importer]

Elliotte Rusty Harold commented

This is a bug, so worthy of bug fix, and ComparableVersion is used beyond Maven core. There's a main method that calls it instead of Maven, and I seem to recall that I used it in Google's Java linkage checker enforcer rule. There might be other uses outside the core plugins as well.

[jira-importer]

Elliotte Rusty Harold commented

We do seem to treat non-ASCII digits as numbers currently. I'm not sure we should.