From 0a184a8d791e0707e364c8216e88c7d3292e84fd Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Fri, 15 Aug 2014 12:41:26 -0500 Subject: [PATCH] SEC-2697: Fix logging of Spring Version Check --- .../security/core/ComparableVersion.java | 469 ++++++++++++++++++ .../core/SpringSecurityCoreVersion.java | 13 +- .../core/SpringSecurityCoreVersionTests.java | 18 + 3 files changed, 498 insertions(+), 2 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/core/ComparableVersion.java diff --git a/core/src/main/java/org/springframework/security/core/ComparableVersion.java b/core/src/main/java/org/springframework/security/core/ComparableVersion.java new file mode 100644 index 0000000000..dc3c03c833 --- /dev/null +++ b/core/src/main/java/org/springframework/security/core/ComparableVersion.java @@ -0,0 +1,469 @@ +package org.springframework.security.core; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +import java.math.BigInteger; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; +import java.util.ListIterator; +import java.util.Locale; +import java.util.Properties; +import java.util.Stack; + +/** + * Generic implementation of version comparison. + * + * NOTE: This is a copy from https://svn.apache.org/repos/asf/maven/maven-3/tags/maven-3.1.0/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java + * + *

Features: + *

+ * + * @see "Versioning" on Maven Wiki + * @author Kenney Westerhof + * @author Hervé Boutemy + */ +class ComparableVersion + implements Comparable +{ + private String value; + + private String canonical; + + private ListItem items; + + private interface Item + { + final int INTEGER_ITEM = 0; + final int STRING_ITEM = 1; + final int LIST_ITEM = 2; + + int compareTo( Item item ); + + int getType(); + + boolean isNull(); + } + + /** + * Represents a numeric item in the version item list. + */ + private static class IntegerItem + implements Item + { + private static final BigInteger BigInteger_ZERO = new BigInteger( "0" ); + + private final BigInteger value; + + public static final IntegerItem ZERO = new IntegerItem(); + + private IntegerItem() + { + this.value = BigInteger_ZERO; + } + + public IntegerItem( String str ) + { + this.value = new BigInteger( str ); + } + + public int getType() + { + return INTEGER_ITEM; + } + + public boolean isNull() + { + return BigInteger_ZERO.equals( value ); + } + + public int compareTo( Item item ) + { + if ( item == null ) + { + return BigInteger_ZERO.equals( value ) ? 0 : 1; // 1.0 == 1, 1.1 > 1 + } + + switch ( item.getType() ) + { + case INTEGER_ITEM: + return value.compareTo( ( (IntegerItem) item ).value ); + + case STRING_ITEM: + return 1; // 1.1 > 1-sp + + case LIST_ITEM: + return 1; // 1.1 > 1-1 + + default: + throw new RuntimeException( "invalid item: " + item.getClass() ); + } + } + + public String toString() + { + return value.toString(); + } + } + + /** + * Represents a string in the version item list, usually a qualifier. + */ + private static class StringItem + implements Item + { + private static final String[] QUALIFIERS = { "alpha", "beta", "milestone", "rc", "snapshot", "", "sp" }; + + private static final List _QUALIFIERS = Arrays.asList( QUALIFIERS ); + + private static final Properties ALIASES = new Properties(); + static + { + ALIASES.put( "ga", "" ); + ALIASES.put( "final", "" ); + ALIASES.put( "cr", "rc" ); + } + + /** + * A comparable value for the empty-string qualifier. This one is used to determine if a given qualifier makes + * the version older than one without a qualifier, or more recent. + */ + private static final String RELEASE_VERSION_INDEX = String.valueOf( _QUALIFIERS.indexOf( "" ) ); + + private String value; + + public StringItem( String value, boolean followedByDigit ) + { + if ( followedByDigit && value.length() == 1 ) + { + // a1 = alpha-1, b1 = beta-1, m1 = milestone-1 + switch ( value.charAt( 0 ) ) + { + case 'a': + value = "alpha"; + break; + case 'b': + value = "beta"; + break; + case 'm': + value = "milestone"; + break; + } + } + this.value = ALIASES.getProperty( value , value ); + } + + public int getType() + { + return STRING_ITEM; + } + + public boolean isNull() + { + return ( comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ) == 0 ); + } + + /** + * Returns a comparable value for a qualifier. + * + * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical ordering. + * + * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1 + * or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first character, + * so this is still fast. If more characters are needed then it requires a lexical sort anyway. + * + * @param qualifier + * @return an equivalent value that can be used with lexical comparison + */ + public static String comparableQualifier( String qualifier ) + { + int i = _QUALIFIERS.indexOf( qualifier ); + + return i == -1 ? ( _QUALIFIERS.size() + "-" + qualifier ) : String.valueOf( i ); + } + + public int compareTo( Item item ) + { + if ( item == null ) + { + // 1-rc < 1, 1-ga > 1 + return comparableQualifier( value ).compareTo( RELEASE_VERSION_INDEX ); + } + switch ( item.getType() ) + { + case INTEGER_ITEM: + return -1; // 1.any < 1.1 ? + + case STRING_ITEM: + return comparableQualifier( value ).compareTo( comparableQualifier( ( (StringItem) item ).value ) ); + + case LIST_ITEM: + return -1; // 1.any < 1-1 + + default: + throw new RuntimeException( "invalid item: " + item.getClass() ); + } + } + + public String toString() + { + return value; + } + } + + /** + * Represents a version list item. This class is used both for the global item list and for sub-lists (which start + * with '-(number)' in the version specification). + */ + private static class ListItem + extends ArrayList + implements Item + { + public int getType() + { + return LIST_ITEM; + } + + public boolean isNull() + { + return ( size() == 0 ); + } + + void normalize() + { + for( ListIterator iterator = listIterator( size() ); iterator.hasPrevious(); ) + { + Item item = iterator.previous(); + if ( item.isNull() ) + { + iterator.remove(); // remove null trailing items: 0, "", empty list + } + else + { + break; + } + } + } + + public int compareTo( Item item ) + { + if ( item == null ) + { + if ( size() == 0 ) + { + return 0; // 1-0 = 1- (normalize) = 1 + } + Item first = get( 0 ); + return first.compareTo( null ); + } + switch ( item.getType() ) + { + case INTEGER_ITEM: + return -1; // 1-1 < 1.0.x + + case STRING_ITEM: + return 1; // 1-1 > 1-sp + + case LIST_ITEM: + Iterator left = iterator(); + Iterator right = ( (ListItem) item ).iterator(); + + while ( left.hasNext() || right.hasNext() ) + { + Item l = left.hasNext() ? left.next() : null; + Item r = right.hasNext() ? right.next() : null; + + // if this is shorter, then invert the compare and mul with -1 + int result = l == null ? -1 * r.compareTo( l ) : l.compareTo( r ); + + if ( result != 0 ) + { + return result; + } + } + + return 0; + + default: + throw new RuntimeException( "invalid item: " + item.getClass() ); + } + } + + public String toString() + { + StringBuilder buffer = new StringBuilder( "(" ); + for( Iterator iter = iterator(); iter.hasNext(); ) + { + buffer.append( iter.next() ); + if ( iter.hasNext() ) + { + buffer.append( ',' ); + } + } + buffer.append( ')' ); + return buffer.toString(); + } + } + + public ComparableVersion( String version ) + { + parseVersion( version ); + } + + public final void parseVersion( String version ) + { + this.value = version; + + items = new ListItem(); + + version = version.toLowerCase( Locale.ENGLISH ); + + ListItem list = items; + + Stack stack = new Stack(); + stack.push( list ); + + boolean isDigit = false; + + int startIndex = 0; + + for ( int i = 0; i < version.length(); i++ ) + { + char c = version.charAt( i ); + + if ( c == '.' ) + { + if ( i == startIndex ) + { + list.add( IntegerItem.ZERO ); + } + else + { + list.add( parseItem( isDigit, version.substring( startIndex, i ) ) ); + } + startIndex = i + 1; + } + else if ( c == '-' ) + { + if ( i == startIndex ) + { + list.add( IntegerItem.ZERO ); + } + else + { + list.add( parseItem( isDigit, version.substring( startIndex, i ) ) ); + } + startIndex = i + 1; + + if ( isDigit ) + { + list.normalize(); // 1.0-* = 1-* + + if ( ( i + 1 < version.length() ) && Character.isDigit( version.charAt( i + 1 ) ) ) + { + // new ListItem only if previous were digits and new char is a digit, + // ie need to differentiate only 1.1 from 1-1 + list.add( list = new ListItem() ); + + stack.push( list ); + } + } + } + else if ( Character.isDigit( c ) ) + { + if ( !isDigit && i > startIndex ) + { + list.add( new StringItem( version.substring( startIndex, i ), true ) ); + startIndex = i; + } + + isDigit = true; + } + else + { + if ( isDigit && i > startIndex ) + { + list.add( parseItem( true, version.substring( startIndex, i ) ) ); + startIndex = i; + } + + isDigit = false; + } + } + + if ( version.length() > startIndex ) + { + list.add( parseItem( isDigit, version.substring( startIndex ) ) ); + } + + while ( !stack.isEmpty() ) + { + list = (ListItem) stack.pop(); + list.normalize(); + } + + canonical = items.toString(); + } + + private static Item parseItem( boolean isDigit, String buf ) + { + return isDigit ? new IntegerItem( buf ) : new StringItem( buf, false ); + } + + public int compareTo( ComparableVersion o ) + { + return items.compareTo( o.items ); + } + + public String toString() + { + return value; + } + + public boolean equals( Object o ) + { + return ( o instanceof ComparableVersion ) && canonical.equals( ( (ComparableVersion) o ).canonical ); + } + + public int hashCode() + { + return canonical.hashCode(); + } +} \ No newline at end of file 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 b1d78ec659..9dee8ee49b 100644 --- a/core/src/main/java/org/springframework/security/core/SpringSecurityCoreVersion.java +++ b/core/src/main/java/org/springframework/security/core/SpringSecurityCoreVersion.java @@ -38,6 +38,15 @@ public class SpringSecurityCoreVersion { * Performs version checks */ private static void performVersionChecks() { + performVersionChecks(MIN_SPRING_VERSION); + } + + /** + * Perform version checks with specific min Spring Version + * + * @param minSpringVersion + */ + private static void performVersionChecks(String minSpringVersion) { // Check Spring Compatibility String springVersion = SpringVersion.getVersion(); String version = getVersion(); @@ -47,8 +56,8 @@ public class SpringSecurityCoreVersion { } logger.info("You are running with Spring Security Core " + version); - if (springVersion.compareTo(MIN_SPRING_VERSION) < 0) { - logger.warn("**** You are advised to use Spring " + MIN_SPRING_VERSION + + if (new ComparableVersion(springVersion).compareTo(new ComparableVersion(minSpringVersion)) < 0) { + logger.warn("**** You are advised to use Spring " + minSpringVersion + " or later with this version. You are running: " + springVersion); } } 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 1fd009be12..e6c622efdd 100644 --- a/core/src/test/java/org/springframework/security/core/SpringSecurityCoreVersionTests.java +++ b/core/src/test/java/org/springframework/security/core/SpringSecurityCoreVersionTests.java @@ -34,6 +34,7 @@ import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; import org.powermock.reflect.Whitebox; import org.springframework.core.SpringVersion; +import org.springframework.test.util.ReflectionTestUtils; /** * Checks that the embedded version information is up to date. @@ -128,6 +129,20 @@ public class SpringSecurityCoreVersionTests { verify(logger, never()).warn(any()); } + // SEC-2697 + @Test + public void noWarnIfSpringPatchVersionDoubleDigits() throws Exception { + String minSpringVersion = "3.2.8.RELEASE"; + spy(SpringSecurityCoreVersion.class); + spy(SpringVersion.class); + when(SpringSecurityCoreVersion.getVersion()).thenReturn("3.2.0.RELEASE"); + when(SpringVersion.getVersion()).thenReturn("3.2.10.RELEASE"); + + performChecks(minSpringVersion); + + verify(logger, never()).warn(any()); + } + @Test public void noLoggingIfPropertySet() throws Exception { spy(SpringSecurityCoreVersion.class); @@ -149,4 +164,7 @@ public class SpringSecurityCoreVersionTests { Whitebox.invokeMethod(SpringSecurityCoreVersion.class, "performVersionChecks"); } + private void performChecks(String minSpringVersion) throws Exception { + Whitebox.invokeMethod(SpringSecurityCoreVersion.class, "performVersionChecks", minSpringVersion); + } }