From 28932e58e9c8fd7c51bf63f3f2ef2bb29783e3b5 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Mon, 1 Feb 2021 12:30:16 -0500 Subject: [PATCH] Fix false positives in SpringBootVersionVerifier If user was using boot 2.4 with Hoxton, the manifest check would return false, then try the reflection based predicates which would match 2.3 since the 2.3 predicate still holds true in 2.4 Fixes gh-895 --- .../SpringBootVersionVerifier.java | 18 +++++--- .../SpringBootDependencyTests.java | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/configuration/SpringBootVersionVerifier.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/configuration/SpringBootVersionVerifier.java index c295c7b6..efb445d9 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/configuration/SpringBootVersionVerifier.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/configuration/SpringBootVersionVerifier.java @@ -82,14 +82,14 @@ class SpringBootVersionVerifier implements CompatibilityVerifier { }; } - private boolean bootVersionFromManifest(String s) { + private Boolean bootVersionFromManifest(String s) { String version = getVersionFromManifest(); if (log.isDebugEnabled()) { log.debug("Version found in Boot manifest [" + version + "]"); } if (!StringUtils.hasText(version)) { - log.info("Cannot check Boot version"); - return true; + log.info("Cannot check Boot version from manifest"); + return null; } return version.startsWith(stripWildCardFromVersion(s)); } @@ -214,11 +214,17 @@ class SpringBootVersionVerifier implements CompatibilityVerifier { private boolean springBootVersionMatches() { for (String acceptedVersion : this.acceptedVersions) { - if (bootVersionFromManifest(acceptedVersion)) { + Boolean versionFromManifest = bootVersionFromManifest(acceptedVersion); + // if manifest has version and matches, return + // otherwise need to check other versions in list + // if all return false, then the return false at end will apply + if (versionFromManifest != null && versionFromManifest) { return true; } - else { - // 2.0, 2.1 + else if (versionFromManifest == null) { + // only check these if the manifest does not have a version. + // otherwise this could lead to false positives for future + // versions of boot CompatibilityPredicate predicate = this.ACCEPTED_VERSIONS .get(stripWildCardFromVersion(acceptedVersion)); if (predicate != null && predicate.isCompatible()) { diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/SpringBootDependencyTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/SpringBootDependencyTests.java index 341b24e8..f304d01e 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/SpringBootDependencyTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/SpringBootDependencyTests.java @@ -16,8 +16,10 @@ package org.springframework.cloud.configuration; +import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.Ignore; import org.junit.Test; @@ -143,6 +145,50 @@ public class SpringBootDependencyTests { then(verificationResult.action).isNotEmpty(); } + @Test + public void should_not_match_when_manifest_has_version_and_not_compatible() { + List acceptedVersions = Collections.singletonList("2.5"); + SpringBootVersionVerifier versionVerifier = new SpringBootVersionVerifier( + acceptedVersions) { + @Override + String getVersionFromManifest() { + return "2.1"; + } + }; + versionVerifier.ACCEPTED_VERSIONS.clear(); + AtomicBoolean verifierRun = new AtomicBoolean(false); + versionVerifier.ACCEPTED_VERSIONS.put("2.5", + () -> verifierRun.compareAndSet(false, true)); + + VerificationResult verificationResult = versionVerifier.verify(); + + then(verifierRun).isFalse(); + then(verificationResult.description).isNotEmpty(); + then(verificationResult.action).isNotEmpty(); + } + + @Test + public void should_match_when_manifest_has_version_and_compatible_list() { + List acceptedVersions = Arrays.asList("2.0", "2.1"); + SpringBootVersionVerifier versionVerifier = new SpringBootVersionVerifier( + acceptedVersions) { + @Override + String getVersionFromManifest() { + return "2.1"; + } + }; + versionVerifier.ACCEPTED_VERSIONS.clear(); + AtomicBoolean verifierRun = new AtomicBoolean(false); + versionVerifier.ACCEPTED_VERSIONS.put("2.5", + () -> verifierRun.compareAndSet(false, true)); + + VerificationResult verificationResult = versionVerifier.verify(); + + then(verifierRun).isFalse(); + then(verificationResult.description).isEmpty(); + then(verificationResult.action).isEmpty(); + } + @Ignore // FIXME: https://github.com/spring-cloud/spring-cloud-commons/issues/717 @Test public void should_match_against_current_manifest() {