Browse Source

Make ApplicationContextRequestMatcher and subclasses thread-safe

Previously, when performing lazy initialisation of the context,
ApplicationContextRequestMatcher assigned the context field before it
called initialized. The context being non-null is used as the signal
that it’s ok to call a subclass’s matches method. If one thread checks
for a non-null context in between the field being assigned and
initialized being called on another thread, matches will be called
before the subclass is ready.

This commit closes the window for the race condition by only assigning
the context field once the subclass’s initialized method has been
called.

There is a secondary problem in each of the subclasses. Due to the use
of double-checked locking in ApplicationContextRequestMatcher, it’s
possible for a subclass’s matches method to be called by a thread that
has not synchronised on the context lock that’s held when initialized
is called and the delegate field is assigned. This means that the
value assigned to the field may not be visible to that thread.

This commit declares the delegate field of each
ApplicationContextRequestMatcher subclass as volatile to ensure that,
following initialisation, its value is guaranteed to be visible to
all threads.

Closes gh-12380
pull/12428/head
Andy Wilkinson 8 years ago
parent
commit
317b51f2ad
  1. 2
      spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java
  2. 2
      spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/PathRequest.java
  3. 2
      spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequest.java
  4. 5
      spring-boot-project/spring-boot/src/main/java/org/springframework/boot/security/servlet/ApplicationContextRequestMatcher.java

2
spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java

@ -100,7 +100,7 @@ public final class EndpointRequest {
private final List<Object> excludes; private final List<Object> excludes;
private RequestMatcher delegate; private volatile RequestMatcher delegate;
private EndpointRequestMatcher() { private EndpointRequestMatcher() {
this(Collections.emptyList(), Collections.emptyList()); this(Collections.emptyList(), Collections.emptyList());

2
spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/PathRequest.java

@ -64,7 +64,7 @@ public final class PathRequest {
public static final class H2ConsoleRequestMatcher public static final class H2ConsoleRequestMatcher
extends ApplicationContextRequestMatcher<H2ConsoleProperties> { extends ApplicationContextRequestMatcher<H2ConsoleProperties> {
private RequestMatcher delegate; private volatile RequestMatcher delegate;
private H2ConsoleRequestMatcher() { private H2ConsoleRequestMatcher() {
super(H2ConsoleProperties.class); super(H2ConsoleProperties.class);

2
spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/StaticResourceRequest.java

@ -100,7 +100,7 @@ public final class StaticResourceRequest {
private final Set<StaticResourceLocation> locations; private final Set<StaticResourceLocation> locations;
private RequestMatcher delegate; private volatile RequestMatcher delegate;
private StaticResourceRequestMatcher(Set<StaticResourceLocation> locations) { private StaticResourceRequestMatcher(Set<StaticResourceLocation> locations) {
super(ServerProperties.class); super(ServerProperties.class);

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

@ -69,8 +69,9 @@ public abstract class ApplicationContextRequestMatcher<C> implements RequestMatc
if (this.context == null) { if (this.context == null) {
synchronized (this.contextLock) { synchronized (this.contextLock) {
if (this.context == null) { if (this.context == null) {
this.context = createContext(request); Supplier<C> createdContext = createContext(request);
initialized(this.context); initialized(createdContext);
this.context = createdContext;
} }
} }
} }

Loading…
Cancel
Save