Skip to content

Commit 890c11d

Browse files
committed
Polish SecurityNamespaceHandler Version Checking
PR gh-17689
1 parent 01401f1 commit 890c11d

File tree

2 files changed

+39
-11
lines changed

2 files changed

+39
-11
lines changed

config/src/main/java/org/springframework/security/config/SecurityNamespaceHandler.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.util.HashMap;
2020
import java.util.Map;
21+
import java.util.Objects;
2122

2223
import org.apache.commons.logging.Log;
2324
import org.apache.commons.logging.LogFactory;
@@ -77,19 +78,18 @@ public final class SecurityNamespaceHandler implements NamespaceHandler {
7778

7879
public SecurityNamespaceHandler() {
7980
String coreVersion = SpringSecurityCoreVersion.getVersion();
80-
Package pkg = SpringSecurityCoreVersion.class.getPackage();
81-
if (pkg == null || coreVersion == null) {
82-
this.logger.info("Couldn't determine package version information.");
83-
return;
84-
}
85-
String version = pkg.getImplementationVersion();
86-
this.logger.info("Spring Security 'config' module version is " + version);
87-
if (version.compareTo(coreVersion) != 0) {
88-
this.logger
89-
.error("You are running with different versions of the Spring Security 'core' and 'config' modules");
81+
String configVersion = configVersion();
82+
if (!Objects.equals(coreVersion, configVersion)) {
83+
String message = "You are attempting to run spring-security-core:%s with spring-security-config:%s";
84+
this.logger.error(String.format(message, coreVersion, configVersion));
9085
}
9186
}
9287

88+
private static String configVersion() {
89+
Package pkg = SpringSecurityCoreVersion.class.getPackage();
90+
return (pkg != null) ? pkg.getImplementationVersion() : null;
91+
}
92+
9393
@Override
9494
public BeanDefinition parse(Element element, ParserContext pc) {
9595
if (!namespaceMatchesVersion(element)) {

config/src/test/java/org/springframework/security/config/SecurityNamespaceHandlerTests.java

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,25 @@
1616

1717
package org.springframework.security.config;
1818

19+
import ch.qos.logback.classic.Level;
20+
import ch.qos.logback.classic.Logger;
21+
import ch.qos.logback.classic.spi.ILoggingEvent;
22+
import ch.qos.logback.core.Appender;
1923
import org.apache.commons.logging.Log;
2024
import org.junit.jupiter.api.Test;
2125
import org.junit.jupiter.api.extension.ExtendWith;
2226
import org.mockito.Answers;
27+
import org.mockito.ArgumentCaptor;
2328
import org.mockito.Mock;
2429
import org.mockito.MockedStatic;
30+
import org.mockito.Mockito;
2531
import org.mockito.junit.jupiter.MockitoExtension;
32+
import org.slf4j.LoggerFactory;
2633

2734
import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
2835
import org.springframework.security.config.util.InMemoryXmlApplicationContext;
2936
import org.springframework.security.config.util.SpringSecurityVersions;
37+
import org.springframework.security.core.SpringSecurityCoreVersion;
3038
import org.springframework.test.util.ReflectionTestUtils;
3139
import org.springframework.util.ClassUtils;
3240

@@ -35,6 +43,8 @@
3543
import static org.mockito.ArgumentMatchers.any;
3644
import static org.mockito.ArgumentMatchers.eq;
3745
import static org.mockito.Mockito.mock;
46+
import static org.mockito.Mockito.never;
47+
import static org.mockito.Mockito.verify;
3848
import static org.mockito.Mockito.verifyNoMoreInteractions;
3949

4050
/**
@@ -63,8 +73,26 @@ public class SecurityNamespaceHandlerTests {
6373
private MockedStatic<ClassUtils> classUtils;
6474

6575
@Test
66-
public void constructionSucceeds() {
76+
public void constructionWhenVersionsMatchThenLogsNothing() {
77+
Appender<ILoggingEvent> appender = mock(Appender.class);
78+
Logger logger = (Logger) LoggerFactory.getLogger(SecurityNamespaceHandler.class);
79+
logger.addAppender(appender);
6780
assertThat(new SecurityNamespaceHandler()).isNotNull();
81+
verify(appender, never()).doAppend(any(ILoggingEvent.class));
82+
}
83+
84+
@Test
85+
public void constructorWhenDetectsMismatchingVersionsThenLogsError() {
86+
Appender<ILoggingEvent> appender = mock(Appender.class);
87+
Logger logger = (Logger) LoggerFactory.getLogger(SecurityNamespaceHandler.class);
88+
logger.addAppender(appender);
89+
try (MockedStatic<SpringSecurityCoreVersion> core = Mockito.mockStatic(SpringSecurityCoreVersion.class)) {
90+
core.when(SpringSecurityCoreVersion::getVersion).thenReturn("mismatching");
91+
assertThat(new SecurityNamespaceHandler()).isNotNull();
92+
ArgumentCaptor<ILoggingEvent> captor = ArgumentCaptor.forClass(ILoggingEvent.class);
93+
verify(appender).doAppend(captor.capture());
94+
assertThat(captor.getValue().getLevel()).isEqualTo(Level.ERROR);
95+
}
6896
}
6997

7098
@Test

0 commit comments

Comments
 (0)