fix(semver): handle unparseable version strings by returning UNKNOWN …

…instead of throwing (#515) * fix(semver): handle unparseable version strings by returning UNKNOWN instead of throwing * refactor * add test cases
cryostatio · Oct 17, 2024 · 52eda9b · 52eda9b
1 parent 98130d4
commit 52eda9b
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 23 deletions.
diff --git a/src/main/java/io/cryostat/agent/VersionInfo.java b/src/main/java/io/cryostat/agent/VersionInfo.java
@@ -21,9 +21,14 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import io.cryostat.agent.util.ResourcesUtil;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public class VersionInfo {
 
     private static final String RESOURCE_LOCATION = "versions.properties";
@@ -80,6 +85,13 @@ public boolean validateServerVersion(Semver actual) {
 
     public static class Semver implements Comparable<Semver> {
 
+        public static final Pattern VERSION_PATTERN =
+                Pattern.compile(
+                        "^v?(?<major>[\\d]+)\\.(?<minor>[\\d]+)\\.(?<patch>[\\d]+)(?:-[a-z0-9\\._-]*)?",
+                        Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
+
+        private static Logger log = LoggerFactory.getLogger(Semver.class);
+
         public static final Semver UNKNOWN =
                 new Semver(0, 0, 0) {
                     @Override
@@ -99,14 +111,20 @@ public Semver(int major, int minor, int patch) {
         }
 
         public static Semver fromString(String in) {
-            String[] parts = in.split("\\.");
-            if (parts.length != 3) {
-                throw new IllegalArgumentException();
+            try {
+                Matcher m = VERSION_PATTERN.matcher(in);
+                if (!m.matches()) {
+                    log.warn("Input version \"{}\" did not match expected semver pattern", in);
+                    return UNKNOWN;
+                }
+                return new Semver(
+                        Integer.parseInt(m.group("major")),
+                        Integer.parseInt(m.group("minor")),
+                        Integer.parseInt(m.group("patch")));
+            } catch (NumberFormatException nfe) {
+                log.error(String.format("Unable to parse input string \"%s\"", in), nfe);
+                return UNKNOWN;
             }
-            return new Semver(
-                    Integer.parseInt(parts[0]),
-                    Integer.parseInt(parts[1]),
-                    Integer.parseInt(parts[2]));
         }
 
         public int getMajor() {

diff --git a/src/main/java/io/cryostat/agent/model/ServerHealth.java b/src/main/java/io/cryostat/agent/model/ServerHealth.java
@@ -15,21 +15,13 @@
  */
 package io.cryostat.agent.model;
 
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 import io.cryostat.agent.VersionInfo.Semver;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 @SuppressFBWarnings(value = {"EI_EXPOSE_REP", "EI_EXPOSE_REP2"})
 public class ServerHealth {
 
-    private static final Pattern VERSION_PATTERN =
-            Pattern.compile(
-                    "^v(?<major>[\\d]+)\\.(?<minor>[\\d]+)\\.(?<patch>[\\d]+)(?:-[a-z0-9\\._-]*)?",
-                    Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
-
     private String cryostatVersion;
     private BuildInfo build;
 
@@ -53,14 +45,7 @@ public String cryostatVersion() {
     }
 
     public Semver cryostatSemver() {
-        Matcher m = VERSION_PATTERN.matcher(cryostatVersion());
-        if (!m.matches()) {
-            return Semver.fromString("0.0.0");
-        }
-        return new Semver(
-                Integer.parseInt(m.group("major")),
-                Integer.parseInt(m.group("minor")),
-                Integer.parseInt(m.group("patch")));
+        return Semver.fromString(cryostatVersion());
     }
 
     public BuildInfo buildInfo() {

diff --git a/src/test/java/io/cryostat/agent/SemverTest.java b/src/test/java/io/cryostat/agent/SemverTest.java
@@ -35,6 +35,9 @@ public class SemverTest {
         "2.0.0, 1.1.0, 1",
         "1.0.1, 1.1.0, -1",
         "1.1.1, 1.0.1, 1",
+        "1.0.0-SNAPSHOT, 1.0.0, 0",
+        "v1.0.0-SNAPSHOT, 1.0.0, 0",
+        "v1.0.0, 1.0.0, 0",
     })
     public void test(String first, String second, int result) {
         Semver a = Semver.fromString(first);