Skip to content

Commit 1ceb96f

Browse files
committed
Ensure matches is not called before initialization
Update `ApplicationContextRequestMatcher` to ensure that the `matches` method is never called before `initialized`. This fixes an issue accidentally introduced in commit 5938ca7 where concurrent calls to `matches` could trigger unexpected errors due to the fact that the second call proceeded before the `initialized` method had returned. Fixes gh-18211
1 parent 5427526 commit 1ceb96f

File tree

2 files changed

+83
-4
lines changed

2 files changed

+83
-4
lines changed

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.boot.security.servlet;
1818

19-
import java.util.concurrent.atomic.AtomicBoolean;
2019
import java.util.function.Supplier;
2120

2221
import javax.servlet.http.HttpServletRequest;
@@ -44,7 +43,9 @@ public abstract class ApplicationContextRequestMatcher<C> implements RequestMatc
4443

4544
private final Class<? extends C> contextClass;
4645

47-
private final AtomicBoolean initialized = new AtomicBoolean(false);
46+
private volatile boolean initialized;
47+
48+
private final Object initializeLock = new Object();
4849

4950
public ApplicationContextRequestMatcher(Class<? extends C> contextClass) {
5051
Assert.notNull(contextClass, "Context class must not be null");
@@ -59,8 +60,13 @@ public final boolean matches(HttpServletRequest request) {
5960
return false;
6061
}
6162
Supplier<C> context = () -> getContext(webApplicationContext);
62-
if (this.initialized.compareAndSet(false, true)) {
63-
initialized(context);
63+
if (!this.initialized) {
64+
synchronized (this.initializeLock) {
65+
if (!this.initialized) {
66+
initialized(context);
67+
this.initialized = true;
68+
}
69+
}
6470
}
6571
return matches(request, context);
6672
}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcherTests.java

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616

1717
package org.springframework.boot.security.servlet;
1818

19+
import java.lang.Thread.UncaughtExceptionHandler;
20+
import java.util.ArrayList;
21+
import java.util.List;
22+
import java.util.concurrent.atomic.AtomicBoolean;
1923
import java.util.function.Supplier;
2024

2125
import javax.servlet.http.HttpServletRequest;
@@ -26,6 +30,7 @@
2630
import org.springframework.context.ApplicationContext;
2731
import org.springframework.mock.web.MockHttpServletRequest;
2832
import org.springframework.mock.web.MockServletContext;
33+
import org.springframework.util.ReflectionUtils;
2934
import org.springframework.web.context.WebApplicationContext;
3035
import org.springframework.web.context.support.StaticWebApplicationContext;
3136

@@ -105,6 +110,31 @@ protected boolean matches(HttpServletRequest request, Supplier<ApplicationContex
105110
assertThat(matcher.matches(request)).isFalse();
106111
}
107112

113+
@Test // gh-18211
114+
public void matchesWhenConcurrentlyCalledWaitsForInitialize() {
115+
ConcurrentApplicationContextRequestMatcher matcher = new ConcurrentApplicationContextRequestMatcher();
116+
StaticWebApplicationContext context = createWebApplicationContext();
117+
Runnable target = () -> matcher.matches(new MockHttpServletRequest(context.getServletContext()));
118+
List<Thread> threads = new ArrayList<>();
119+
AssertingUncaughtExceptionHandler exceptionHandler = new AssertingUncaughtExceptionHandler();
120+
for (int i = 0; i < 2; i++) {
121+
Thread thread = new Thread(target);
122+
thread.setUncaughtExceptionHandler(exceptionHandler);
123+
threads.add(thread);
124+
}
125+
threads.forEach(Thread::start);
126+
threads.forEach(this::join);
127+
exceptionHandler.assertNoExceptions();
128+
}
129+
130+
private void join(Thread thread) {
131+
try {
132+
thread.join(1000);
133+
}
134+
catch (InterruptedException ex) {
135+
}
136+
}
137+
108138
private StaticWebApplicationContext createWebApplicationContext() {
109139
StaticWebApplicationContext context = new StaticWebApplicationContext();
110140
MockServletContext servletContext = new MockServletContext();
@@ -160,4 +190,47 @@ public Supplier<C> getProvidedContext() {
160190

161191
}
162192

193+
static class ConcurrentApplicationContextRequestMatcher extends ApplicationContextRequestMatcher<Object> {
194+
195+
ConcurrentApplicationContextRequestMatcher() {
196+
super(Object.class);
197+
}
198+
199+
private AtomicBoolean initialized = new AtomicBoolean();
200+
201+
@Override
202+
protected void initialized(Supplier<Object> context) {
203+
try {
204+
Thread.sleep(200);
205+
}
206+
catch (InterruptedException ex) {
207+
}
208+
this.initialized.set(true);
209+
}
210+
211+
@Override
212+
protected boolean matches(HttpServletRequest request, Supplier<Object> context) {
213+
assertThat(this.initialized.get()).isTrue();
214+
return true;
215+
}
216+
217+
}
218+
219+
private static class AssertingUncaughtExceptionHandler implements UncaughtExceptionHandler {
220+
221+
private volatile Throwable ex;
222+
223+
@Override
224+
public void uncaughtException(Thread thead, Throwable ex) {
225+
this.ex = ex;
226+
}
227+
228+
public void assertNoExceptions() {
229+
if (this.ex != null) {
230+
ReflectionUtils.rethrowRuntimeException(this.ex);
231+
}
232+
}
233+
234+
}
235+
163236
}

0 commit comments

Comments
 (0)