From 5d385b6605c379f840718d40f0e23288f36c627f Mon Sep 17 00:00:00 2001
From: Elliotte Rusty Harold <elharo@ibiblio.org>
Date: Wed, 22 Jul 2026 11:34:55 +0000
Subject: [PATCH] Fix version comparator: use numeric comparison instead of
 lexicographic string comparison (#166)

The version() comparator in ToolchainDiscoverer used String.compareTo()
for version segment comparison, which produces incorrect ordering for
versions with different digit counts (e.g. '8' was sorted before '11'
and '17' because '8' > '1' lexicographically, and the reversed order
incorrectly promoted JDK 8 over JDK 11/17).

Fix by parsing each segment as an integer and using Integer.compare().

Also add unit test that verifies correct descending version sorting:
17 > 11 > 8.
---
 .../toolchain/jdk/ToolchainDiscoverer.java    | 25 +++++++-------
 .../jdk/ToolchainDiscovererTest.java          | 33 +++++++++++++++++++
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java b/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java
index fa9455d..3b671cc 100644
--- a/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java
+++ b/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java
@@ -370,24 +370,25 @@ Comparator<ToolchainModel> version() {
                     String[] b = v2.split("\\.");
                     int length = Math.min(a.length, b.length);
                     for (int i = 0; i < length; i++) {
-                        String oa = a[i];
-                        String ob = b[i];
-                        if (!Objects.equals(oa, ob)) {
-                            // A null element is less than a non-null element
-                            if (oa == null || ob == null) {
-                                return oa == null ? -1 : 1;
-                            }
-                            int v = oa.compareTo(ob);
-                            if (v != 0) {
-                                return v;
-                            }
+                        int oa = parseInt(a[i]);
+                        int ob = parseInt(b[i]);
+                        if (oa != ob) {
+                            return Integer.compare(oa, ob);
                         }
                     }
-                    return a.length - b.length;
+                    return Integer.compare(a.length, b.length);
                 })
                 .reversed();
     }
 
+    private static int parseInt(String s) {
+        try {
+            return Integer.parseInt(s);
+        } catch (NumberFormatException e) {
+            return 0;
+        }
+    }
+
     private Set<Path> findJdks() {
         if (foundJdks == null) {
             synchronized (this) {
diff --git a/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java b/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java
index 11657c0..2ab09d6 100644
--- a/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java
+++ b/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java
@@ -18,7 +18,11 @@
  */
 package org.apache.maven.plugins.toolchain.jdk;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import org.apache.maven.toolchain.model.PersistedToolchains;
+import org.apache.maven.toolchain.model.ToolchainModel;
 import org.codehaus.plexus.util.xml.Xpp3Dom;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledOnJre;
@@ -27,6 +31,7 @@
 import org.slf4j.LoggerFactory;
 
 import static org.apache.maven.plugins.toolchain.jdk.ToolchainDiscoverer.CURRENT;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -51,4 +56,32 @@ void testDiscovery() {
         assertTrue(persistedToolchains.getToolchains().stream()
                 .anyMatch(tc -> tc.getProvides().containsKey(CURRENT)));
     }
+
+    @Test
+    void testVersionComparator() {
+        ToolchainDiscoverer discoverer = new ToolchainDiscoverer();
+
+        ToolchainModel jdk8 = new ToolchainModel();
+        jdk8.setType("jdk");
+        jdk8.addProvide("version", "8");
+
+        ToolchainModel jdk11 = new ToolchainModel();
+        jdk11.setType("jdk");
+        jdk11.addProvide("version", "11");
+
+        ToolchainModel jdk17 = new ToolchainModel();
+        jdk17.setType("jdk");
+        jdk17.addProvide("version", "17");
+
+        List<ToolchainModel> list = new ArrayList<>();
+        list.add(jdk8);
+        list.add(jdk17);
+        list.add(jdk11);
+
+        list.sort(discoverer.version());
+
+        assertEquals("17", list.get(0).getProvides().getProperty("version"));
+        assertEquals("11", list.get(1).getProvides().getProperty("version"));
+        assertEquals("8", list.get(2).getProvides().getProperty("version"));
+    }
 }