Skip to content

Fix version comparison logic in update checker #4466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Gautam-aman wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix version comparison logic in update checker #4466

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8615c6
Fix version comparison for update check and add unit tests
Gautam-aman Dec 7, 2025
00c77d8
Support prerelease version suffixes (-SNAPSHOT, -alpha, -beta) and ad…
Gautam-aman Dec 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions ...plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/UpdateChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,34 @@ private static class VersionJsonTemplate implements JsonTemplate {
* @param log {@link Consumer} used to log messages
* @return a new {@link UpdateChecker}
*/

@VisibleForTesting
static int compareVersions(String v1, String v2) {
String[] p1 = v1.split("\\.");
String[] p2 = v2.split("\\.");

int len = Math.max(p1.length, p2.length);
for (int i = 0; i < len; i++) {
int a = i < p1.length ? parsePart(p1[i]) : 0;
int b = i < p2.length ? parsePart(p2[i]) : 0;
if (a != b) {
return Integer.compare(a, b);
}
}
return 0;
}
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation correctly handles numeric version parts but does not correctly handle pre-release identifiers like -alpha or -SNAPSHOT. For example, 1.0.0-SNAPSHOT should be considered an older version than 1.0.0, but this implementation will treat them as equal. This could lead to missed update notifications for users on pre-release versions.

I recommend adding test cases to cover these scenarios, for example:

assertThat(UpdateChecker.compareVersions("1.0.0-SNAPSHOT", "1.0.0")).isLessThan(0);
assertThat(UpdateChecker.compareVersions("1.0.0-alpha", "1.0.0-beta")).isLessThan(0);

To support this, the compareVersions method would need to be updated to handle these suffixes. One approach is to check for a hyphen in version parts. If the numeric parts are equal, the version part with a suffix can be considered smaller than one without. If both have suffixes, they can be compared lexicographically.


private static int parsePart(String s) {
try {
// Extract leading numeric portion (e.g., "1-SNAPSHOT" → "1")
String digits = s.replaceAll("^(\\d+).*$", "$1");
return Integer.parseInt(digits);
} catch (NumberFormatException e) {
return 0;
}
}
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using String.replaceAll() with a regex can be inefficient, as it typically compiles the regular expression on each call. Since this method is called within a loop in compareVersions, this could have a performance impact. A more performant approach would be to parse the leading digits manually. This also makes the logic for extracting the numeric part more explicit.

  private static int parsePart(String s) {
    // Extract leading numeric portion (e.g., "1-SNAPSHOT" → "1")
    int i = 0;
    while (i < s.length() && Character.isDigit(s.charAt(i))) {
      i++;
    }
    if (i == 0) {
      return 0;
    }
    try {
      return Integer.parseInt(s.substring(0, i));
    } catch (NumberFormatException e) {
      // Should only happen on integer overflow for very long version parts.
      return 0;
    }
  }



public static Future<Optional<String>> checkForUpdate(
ExecutorService executorService,
String versionUrl,
Expand Down Expand Up @@ -118,10 +146,16 @@ static Optional<String> performUpdateCheck(
Files.write(lastUpdateCheckTemp, Instant.now().toString().getBytes(StandardCharsets.UTF_8));
Files.move(lastUpdateCheckTemp, lastUpdateCheck, StandardCopyOption.REPLACE_EXISTING);

if (currentVersion.equals(version.latest)) {
if (version.latest == null || version.latest.isEmpty()) {
return Optional.empty();
}
return Optional.of(version.latest);

if (compareVersions(currentVersion, version.latest) < 0) {
return Optional.of(version.latest); // only report if newer
}

return Optional.empty(); // otherwise equal or older

} finally {
httpClient.shutDown();
}
Expand Down
10 changes: 10 additions & 0 deletions ...ins-common/src/test/java/com/google/cloud/tools/jib/plugins/common/UpdateCheckerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ public void setUp()
configDir = temporaryFolder.getRoot().toPath();
}

@Test
public void testCompareVersions() {
assertThat(UpdateChecker.compareVersions("3.5.0", "3.4.6")).isGreaterThan(0);
assertThat(UpdateChecker.compareVersions("3.4.6", "3.5.0")).isLessThan(0);
assertThat(UpdateChecker.compareVersions("3.5.0", "3.5.0")).isEqualTo(0);
assertThat(UpdateChecker.compareVersions("3.5", "3.5.0")).isEqualTo(0);
assertThat(UpdateChecker.compareVersions("3.5.1", "3.5")).isGreaterThan(0);
}


@After
public void tearDown() throws IOException {
testWebServer.close();
Expand Down