From 664220f3044f17e45691faf622e7f9b9923535bf Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 29 Aug 2013 16:48:49 -0500 Subject: [PATCH] SEC-2295: Remove error logging when Spring version equals Spring Security --- .../core/SpringSecurityCoreVersion.java | 53 ++++++--- .../core/SpringSecurityCoreVersionTests.java | 104 ++++++++++++++++++ 2 files changed, 140 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/springframework/security/core/SpringSecurityCoreVersion.java b/core/src/main/java/org/springframework/security/core/SpringSecurityCoreVersion.java index ce087bcf82..bb74a5ef6a 100644 --- a/core/src/main/java/org/springframework/security/core/SpringSecurityCoreVersion.java +++ b/core/src/main/java/org/springframework/security/core/SpringSecurityCoreVersion.java @@ -8,8 +8,11 @@ import org.springframework.core.SpringVersion; * Internal class used for checking version compatibility in a deployed application. * * @author Luke Taylor + * @author Rob Winch */ public class SpringSecurityCoreVersion { + private static final String DISABLE_CHECKS = SpringSecurityCoreVersion.class.getName().concat(".DISABLE_CHECKS"); + private static final Log logger = LogFactory.getLog(SpringSecurityCoreVersion.class); /** @@ -25,27 +28,43 @@ public class SpringSecurityCoreVersion { static final String MIN_SPRING_VERSION = "3.2.3.RELEASE"; static { - // Check Spring Compatibility - String springVersion = SpringVersion.getVersion(); - String version = getVersion(); - - if (springVersion != null) { - logger.info("You are running with Spring Security Core " + version); - if (!springVersion.startsWith(SPRING_MAJOR_VERSION)) { - logger.error("*** Spring Major version '" + SPRING_MAJOR_VERSION + - "' expected, but you are running with version: " + springVersion + - ". Please check your classpath for unwanted jar files."); - } - - if (springVersion.compareTo(MIN_SPRING_VERSION) < 0) { - logger.warn("**** You are advised to use Spring " + MIN_SPRING_VERSION + - " or later with this version. You are running: " + springVersion); - } - } + performVersionChecks(); } public static String getVersion() { Package pkg = SpringSecurityCoreVersion.class.getPackage(); return (pkg != null ? pkg.getImplementationVersion() : null); } + + /** + * Performs version checks + */ + private static void performVersionChecks() { + // Check Spring Compatibility + String springVersion = SpringVersion.getVersion(); + String version = getVersion(); + + if(disableChecks(springVersion, version)) { + return; + } + + logger.info("You are running with Spring Security Core " + version); + if (!springVersion.startsWith(SPRING_MAJOR_VERSION)) { + logger.warn("*** Spring Major version '" + SPRING_MAJOR_VERSION + + "' expected, but you are running with version: " + springVersion + + ". Please check your classpath for unwanted jar files."); + } + + if (springVersion.compareTo(MIN_SPRING_VERSION) < 0) { + logger.warn("**** You are advised to use Spring " + MIN_SPRING_VERSION + + " or later with this version. You are running: " + springVersion); + } + } + + private static boolean disableChecks(String springVersion, String springSecurityVersion) { + if(springVersion == null || springVersion.equals(springSecurityVersion)) { + return true; + } + return Boolean.getBoolean(DISABLE_CHECKS); + } } diff --git a/core/src/test/java/org/springframework/security/core/SpringSecurityCoreVersionTests.java b/core/src/test/java/org/springframework/security/core/SpringSecurityCoreVersionTests.java index 749a263bc0..5eaf453922 100644 --- a/core/src/test/java/org/springframework/security/core/SpringSecurityCoreVersionTests.java +++ b/core/src/test/java/org/springframework/security/core/SpringSecurityCoreVersionTests.java @@ -1,16 +1,60 @@ +/* + * Copyright 2002-2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.springframework.security.core; import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.powermock.api.mockito.PowerMockito.spy; +import static org.powermock.api.mockito.PowerMockito.when; +import org.apache.commons.logging.Log; import org.junit.*; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.powermock.reflect.Whitebox; +import org.springframework.core.SpringVersion; /** * Checks that the embedded version information is up to date. * * @author Luke Taylor + * @author Rob Winch */ +@RunWith(PowerMockRunner.class) +@PrepareForTest({SpringSecurityCoreVersion.class,SpringVersion.class}) public class SpringSecurityCoreVersionTests { + @Mock + private Log logger; + + @Before + public void setup() { + Whitebox.setInternalState(SpringSecurityCoreVersion.class, logger); + } + + @After + public void cleanup() throws Exception { + System.clearProperty(getDisableChecksProperty()); + } + @Test public void springVersionIsUpToDate() throws Exception { // Property is set by the build script @@ -30,4 +74,64 @@ public class SpringSecurityCoreVersionTests { assertEquals(version.charAt(2), serialVersion.charAt(1)); } + + // SEC-2295 + @Test + public void noLoggingIfVersionsAreEqual() throws Exception { + String version = "1"; + spy(SpringSecurityCoreVersion.class); + spy(SpringVersion.class); + when(SpringSecurityCoreVersion.getVersion()).thenReturn(version); + when(SpringVersion.getVersion()).thenReturn(version); + + performChecks(); + + verifyZeroInteractions(logger); + } + + @Test + public void noLoggingIfSpringVersionNull() throws Exception { + spy(SpringSecurityCoreVersion.class); + spy(SpringVersion.class); + when(SpringSecurityCoreVersion.getVersion()).thenReturn("1"); + when(SpringVersion.getVersion()).thenReturn(null); + + performChecks(); + + verifyZeroInteractions(logger); + } + + @Test + public void warnIfSpringVersionTooSmall() throws Exception { + spy(SpringSecurityCoreVersion.class); + spy(SpringVersion.class); + when(SpringSecurityCoreVersion.getVersion()).thenReturn("3"); + when(SpringVersion.getVersion()).thenReturn("2"); + + performChecks(); + + verify(logger, times(2)).warn(any()); + } + + @Test + public void noLoggingIfPropertySet() throws Exception { + spy(SpringSecurityCoreVersion.class); + spy(SpringVersion.class); + when(SpringSecurityCoreVersion.getVersion()).thenReturn("3"); + when(SpringVersion.getVersion()).thenReturn("2"); + System.setProperty(getDisableChecksProperty(), Boolean.TRUE.toString()); + + performChecks(); + + verifyZeroInteractions(logger); + } + + private String getDisableChecksProperty() throws Exception { + return SpringSecurityCoreVersion.class.getName().concat(".DISABLE_CHECKS"); + } + + private void performChecks() throws Exception { + Whitebox.invokeMethod(SpringSecurityCoreVersion.class, "performVersionChecks"); + } + }