Versions
Spring Boot 2.4.2 Spring Web: 5.3.3 Spring Security: 5.4.2
Problem
I am accessing to Cookie of Spring WebFlux in user code.
At one time, I noticed that an error "java.util.ConcurrentModificationException: null" was logged. The following is the actual log recorded (reactor checkpoint cannot be provided because it contains business code).
Problem log.
java.util.ConcurrentModificationException: null
at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1134)
Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException:
(Removed checkpoint)
Stack trace:
at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1134)
at org.springframework.util.MultiValueMapAdapter.add(MultiValueMapAdapter.java:66)
at org.springframework.http.server.reactive.AbstractServerHttpResponse.addCookie(AbstractServerHttpResponse.java:185)
at org.springframework.security.web.server.csrf.CookieServerCsrfTokenRepository.lambda$saveToken$0(CookieServerCsrfTokenRepository.java:83)
at reactor.core.publisher.MonoRunnable.subscribe(MonoRunnable.java:49)
at reactor.core.publisher.Mono.subscribe(Mono.java:4046)
at reactor.core.publisher.MonoDelayUntil$DelayUntilCoordinator.subscribeNextTrigger(MonoDelayUntil.java:226)
at reactor.core.publisher.MonoDelayUntil$DelayUntilCoordinator.onNext(MonoDelayUntil.java:170)
(Removed: business class name)
at reactor.core.publisher.MonoSubscribeOn$SubscribeOnSubscriber.onNext(MonoSubscribeOn.java:146)
(Removed: business class name)
at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1789)
at reactor.core.publisher.MonoCallable.subscribe(MonoCallable.java:61)
at reactor.core.publisher.Mono.subscribe(Mono.java:4046)
at reactor.core.publisher.MonoSubscribeOn$SubscribeOnSubscriber.run(MonoSubscribeOn.java:126)
at reactor.core.scheduler.WorkerTask.call(WorkerTask.java:84)
at reactor.core.scheduler.WorkerTask.call(WorkerTask.java:37)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:834)
The reason is that cookies in AbstractServerHttpResponse is a LinkedMultiValueMap, which is not thread-safe.
https://github.com/spring-projects/spring-framework/blob/91509805b759a108d7eca0b6b3041c434c61d837/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java#L88
I believe this error is a result of Spring Security and the user code accessing the cookie at the same time. So far, I have only been able to see the error that occurred in the Spring Securty stack trace, so I don't know which cookie operation it conflicted with.
How do I safely access cookies?
Comment From: rstoyanchev
Generally speaking, through the use of Reactor operators and declarative composition, the logic should be executed without concurrent updates to the response. However, if you can provide some snippet of application code that shows what the application does then we can reason more specifically? Or if it's possible to isolate in an executable sample.
Comment From: tt4g
I am continuing to test the application, but so far have not reproduced the problem. There are a number of points in the application where the Reactor operator is manipulating cookies, so I will investigate the cause when I have time.
If I am unable to reproduce the problem, I will close this issue.
Comment From: rstoyanchev
There are a number of points in the application where the Reactor operator is manipulating cookies
It depends on how operators are used, e.g. flatMap or the use of publishOn/subscribeOn can introduce concurrent handling. This is why if would help if you could show examples of how application code updates cookies, even if it's not a full sample that reproduces the issue.
Comment From: tt4g
It depends on how operators are used, e.g. flatMap or the use of publishOn/subscribeOn can introduce concurrent handling.
Yes, it is. flatMap and subscribeOn(Schedulers.boundedElastic()) are used many times.
Furthermore, these operators are used in custom WebFilter and ServerHttpResponse#beforeCommit(Supplier<? extends Mono<Void>>.
Some WebFilter calls subscribeOn(Schedulers.boundedElastic()) because they use a database.
Comment From: rstoyanchev
Sure but this still a bit abstract. It's okay to use flatMap and subscribeOn, but updates to the response need to be made more carefully to avoid competition. If you think of request handling as a kind of scatter-gather, async tasks can be spawned in parallel and then re-joined in some way to produce a single response. It's at this stage that updates to the response should be made and the beforeCommit happens at that point, it should not compete.
Comment From: tt4g
Today I got the complete log file. Attached with information removed that cannot be published: log.txt The error occurred in the HTTP request immediately after Spring Boot was started.
Unfortunately, as far as I checked the user code shown in the Reactor checkpoint included in the log, I could not find any code accessing the cookie. Now I'm thinking that the cause might be somewhere in the Spring framework.
I have also attached the user code: source.tar.gz This is not a complete copy of the actual working code, as some code that cannot be published has been removed and some logic has been ported to reduce the number of files. The Japanese comments in the source code have been removed. I have replaced some of them with English comments, but since I am not a native speaker, it may be unnatural.
And sorry, I cannot provide a detailed implementation of the LoginApiAuthenticationFilter, but it works almost identically to Spring Security's Form Login authentication.
Comment From: tt4g
Unfortunately, as far as I checked the user code shown in the Reactor checkpoint included in the log, I could not find any code accessing the cookie.
I was wrong.
The GenerateCsrfTokenWebFilter gets the CSRF token from the beforeCommit() operator.
CSRF token is stored in the web session, so session ID is added to the cookie.
CSRF token is also written to the cookie to notify the client.
Comment From: tt4g
This weekend I will create a small project to see if I can reproduce the problem.
Comment From: tt4g
I created a project https://github.com/tt4g/spring-cookie-problem and sent 100,000 requests with the ac command, but was unable to reproduce the problem.
Perhaps there is a problem with the business logic code that could not be provided.
@rstoyanchev thank you for your kind response.
Comment From: rstoyanchev
Okay, no worries. If you find out more, let us know.
Comment From: tt4g
I discovered the cause of this problem and solved it. It was our product code that had the problem.
Since our product needed to output CSRF tokens in Single Page Application, we used spring-projects/spring-security#5766 as a reference and wrote the following code.
SubscribeCsrfTokenWebFilter .java
import org.springframework.security.web.server.csrf.CookieServerCsrfTokenRepository;
import org.springframework.security.web.server.csrf.CsrfToken;
import org.springframework.security.web.server.csrf.CsrfWebFilter;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.WebFilter;
import org.springframework.web.server.WebFilterChain;
import reactor.core.publisher.Mono;
public class SubscribeCsrfTokenWebFilter implements WebFilter {
@Override
public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
exchange.getResponse().beforeCommit(() -> {
Mono<CsrfToken> csrfToken = exchange.getAttribute(CsrfToken.class.getName());
if (csrfToken != null) {
return csrfToken.then();
} else {
return Mono.empty();
}
});
return chain.filter(exchange);
}
}
Very simple mistake, Supplier passed to ReactiveHttpOutputMessage#beforeCommit(Supplier<? extends Mono<Void>> action) must wrap Mono#defer(Supplier<? extends Mono<? extends T>>).
The problem is no longer reproduced with the following fix.
Fixed SubscribeCsrfTokenWebFilter .java
import org.springframework.security.web.server.csrf.CookieServerCsrfTokenRepository;
import org.springframework.security.web.server.csrf.CsrfToken;
import org.springframework.security.web.server.csrf.CsrfWebFilter;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.WebFilter;
import org.springframework.web.server.WebFilterChain;
import reactor.core.publisher.Mono;
public class SubscribeCsrfTokenWebFilter implements WebFilter {
@Override
public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
- exchange.getResponse().beforeCommit(() -> {
- Mono<CsrfToken> csrfToken = exchange.getAttribute(CsrfToken.class.getName());
-
- if (csrfToken != null) {
- return csrfToken.then();
- } else {
- return Mono.empty();
- }
- });
+ exchange.getResponse().beforeCommit(() ->
+ Mono.defer(() -> this.subscribeCsrfToken(exchange)));
return chain.filter(exchange);
}
+ private Mono<Void> subscribeCsrfToken(ServerWebExchange exchange) {
+ Mono<CsrfToken> csrfToken = exchange.getAttribute(CsrfToken.class.getName());
+
+ if (csrfToken != null) {
+ return csrfToken.then();
+ } else {
+ return Mono.empty();
+ }
+ }
+
}
Details
Mono<CsrfToken> is generated by org.springframework.security.web.server.csrf.CookieServerCsrfTokenRepository#generateToken(ServerWebExchange) via CsrfWebFilter and is to be executed by the subscribeOn(Schedulers.boundedElastic()).
https://github.com/spring-projects/spring-security/blob/45bca751c727295a754ea4e8f8059054e2d08270/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java#L69-L72
https://github.com/spring-projects/spring-security/blob/45bca751c727295a754ea4e8f8059054e2d08270/web/src/main/java/org/springframework/security/web/server/csrf/CsrfWebFilter.java#L169-L171
The error occurred because the task of writing the CSRF token to the cookie was started when the callback registered in beforeCommit() was retrieved, and the cookie was accessed by allActions.then() in AbstractServerHttpResponse#doCommit(@Nullable Supplier<? extends Mono<Void>>).
https://github.com/spring-projects/spring-framework/blob/7c2a72c9b43d066ae9e71d4f39d7bab8f6d9c2ff/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java#L300
This problem only occurred on servers with a lot of heavy processes, so it took us a while to notice the cause.
Comment From: rstoyanchev
Thanks for following up. Yes the Javadoc for beforeCommit mentions this but it's easy to overlook. I suppose we could wrap the invocation of the suppliers ourselves to make this more reliable.
Comment From: tt4g
It is a good option to have a wrapper to reduce mistakes.