Skip to content

Commit c1fe571

Browse files
poutsmapoutsma
committed
Clean multipart up only when data is read
This commit ensures that any storage used for multipart handling only gets cleaned up if multipart data is actually retrieved via ServerWebExchange::getMultipartData. Closes gh-30590
1 parent 2c8d1b7 commit c1fe571

File tree

5 files changed

+64
-38
lines changed

5 files changed

+64
-38
lines changed

‎spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -141,6 +141,13 @@ default <T> T getAttributeOrDefault(String name, T defaultValue) {
141141
*/
142142
Mono<MultiValueMap<String, Part>> getMultipartData();
143143

144+
/**
145+
* Cleans up any storage used for multipart handling.
146+
* @since 6.0.10
147+
* @see Part#delete()
148+
*/
149+
Mono<Void> cleanupMultipart();
150+
144151
/**
145152
* Return the {@link LocaleContext} using the configured
146153
* {@link org.springframework.web.server.i18n.LocaleContextResolver}.

‎spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -108,6 +108,11 @@ public Mono<MultiValueMap<String, Part>> getMultipartData() {
108108
return getDelegate().getMultipartData();
109109
}
110110

111+
@Override
112+
public Mono<Void> cleanupMultipart() {
113+
return getDelegate().cleanupMultipart();
114+
}
115+
111116
@Override
112117
public boolean isNotModified() {
113118
return getDelegate().isNotModified();

‎spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ public class DefaultServerWebExchange implements ServerWebExchange {
9393

9494
private final Mono<MultiValueMap<String, Part>> multipartDataMono;
9595

96+
private volatile boolean multipartRead = false;
97+
9698
@Nullable
9799
private final ApplicationContext applicationContext;
98100

@@ -131,7 +133,7 @@ public DefaultServerWebExchange(ServerHttpRequest request, ServerHttpResponse re
131133
this.sessionMono = sessionManager.getSession(this).cache();
132134
this.localeContextResolver = localeContextResolver;
133135
this.formDataMono = initFormData(request, codecConfigurer, getLogPrefix());
134-
this.multipartDataMono = initMultipartData(request, codecConfigurer, getLogPrefix());
136+
this.multipartDataMono = initMultipartData(codecConfigurer, getLogPrefix());
135137
this.applicationContext = applicationContext;
136138
}
137139

@@ -154,10 +156,9 @@ private static Mono<MultiValueMap<String, String>> initFormData(ServerHttpReques
154156
.cache();
155157
}
156158

157-
private static Mono<MultiValueMap<String, Part>> initMultipartData(ServerHttpRequest request,
158-
ServerCodecConfigurer configurer, String logPrefix) {
159+
private Mono<MultiValueMap<String, Part>> initMultipartData(ServerCodecConfigurer configurer, String logPrefix) {
159160

160-
MediaType contentType = getContentType(request);
161+
MediaType contentType = getContentType(this.request);
161162
if (contentType == null || !contentType.getType().equalsIgnoreCase("multipart")) {
162163
return EMPTY_MULTIPART_DATA;
163164
}
@@ -168,7 +169,8 @@ private static Mono<MultiValueMap<String, Part>> initMultipartData(ServerHttpReq
168169
}
169170

170171
return reader
171-
.readMono(MULTIPART_DATA_TYPE, request, Hints.from(Hints.LOG_PREFIX_HINT, logPrefix))
172+
.readMono(MULTIPART_DATA_TYPE, this.request, Hints.from(Hints.LOG_PREFIX_HINT, logPrefix))
173+
.doOnNext(ignored -> this.multipartRead = true)
172174
.switchIfEmpty(EMPTY_MULTIPART_DATA)
173175
.cache();
174176
}
@@ -243,6 +245,22 @@ public Mono<MultiValueMap<String, Part>> getMultipartData() {
243245
return this.multipartDataMono;
244246
}
245247

248+
@Override
249+
public Mono<Void> cleanupMultipart() {
250+
if (this.multipartRead) {
251+
return getMultipartData()
252+
.onErrorResume(t -> Mono.empty()) // ignore errors reading multipart data
253+
.flatMapIterable(Map::values)
254+
.flatMapIterable(Function.identity())
255+
.flatMap(part -> part.delete()
256+
.onErrorResume(ex -> Mono.empty()))
257+
.then();
258+
}
259+
else {
260+
return Mono.empty();
261+
}
262+
}
263+
246264
@Override
247265
public LocaleContext getLocaleContext() {
248266
return this.localeContextResolver.resolveLocaleContext(this);

‎spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,9 +16,7 @@
1616

1717
package org.springframework.web.server.adapter;
1818

19-
import java.util.Map;
2019
import java.util.Set;
21-
import java.util.function.Function;
2220

2321
import org.apache.commons.logging.Log;
2422
import org.apache.commons.logging.LogFactory;
@@ -32,7 +30,6 @@
3230
import org.springframework.http.HttpStatusCode;
3331
import org.springframework.http.codec.LoggingCodecSupport;
3432
import org.springframework.http.codec.ServerCodecConfigurer;
35-
import org.springframework.http.codec.multipart.Part;
3633
import org.springframework.http.server.reactive.HttpHandler;
3734
import org.springframework.http.server.reactive.ServerHttpRequest;
3835
import org.springframework.http.server.reactive.ServerHttpResponse;
@@ -250,7 +247,7 @@ public Mono<Void> handle(ServerHttpRequest request, ServerHttpResponse response)
250247
return getDelegate().handle(exchange)
251248
.doOnSuccess(aVoid -> logResponse(exchange))
252249
.onErrorResume(ex -> handleUnresolvedError(exchange, ex))
253-
.then(cleanupMultipart(exchange))
250+
.then(exchange.cleanupMultipart())
254251
.then(Mono.defer(response::setComplete));
255252
}
256253

@@ -325,23 +322,4 @@ private boolean isDisconnectedClientError(Throwable ex) {
325322
return DISCONNECTED_CLIENT_EXCEPTIONS.contains(ex.getClass().getSimpleName());
326323
}
327324

328-
private Mono<Void> cleanupMultipart(ServerWebExchange exchange) {
329-
return exchange.getMultipartData()
330-
.onErrorResume(t -> Mono.empty()) // ignore errors reading multipart data
331-
.flatMapIterable(Map::values)
332-
.flatMapIterable(Function.identity())
333-
.flatMap(this::deletePart)
334-
.then();
335-
}
336-
337-
private Mono<Void> deletePart(Part part) {
338-
return part.delete().onErrorResume(ex -> {
339-
if (logger.isWarnEnabled()) {
340-
logger.warn("Failed to perform cleanup of multipart items", ex);
341-
}
342-
return Mono.empty();
343-
});
344-
}
345-
346-
347325
}

‎spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilder.java

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -324,28 +324,30 @@ private static class DelegatingServerWebExchange implements ServerWebExchange {
324324

325325
private final Mono<MultiValueMap<String, Part>> multipartDataMono;
326326

327+
private volatile boolean multipartRead = false;
328+
329+
327330
DelegatingServerWebExchange(ServerHttpRequest request, Map<String, Object> attributes,
328331
ServerWebExchange delegate, List<HttpMessageReader<?>> messageReaders) {
329332

330333
this.request = request;
331334
this.attributes = attributes;
332335
this.delegate = delegate;
333-
this.formDataMono = initFormData(request, messageReaders);
336+
this.formDataMono = initFormData(messageReaders);
334337
this.multipartDataMono = initMultipartData(request, messageReaders);
335338
}
336339

337340
@SuppressWarnings("unchecked")
338-
private static Mono<MultiValueMap<String, String>> initFormData(ServerHttpRequest request,
339-
List<HttpMessageReader<?>> readers) {
340-
341+
private Mono<MultiValueMap<String, String>> initFormData(List<HttpMessageReader<?>> readers) {
341342
try {
342-
MediaType contentType = request.getHeaders().getContentType();
343+
MediaType contentType = this.request.getHeaders().getContentType();
343344
if (MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType)) {
344345
return ((HttpMessageReader<MultiValueMap<String, String>>) readers.stream()
345346
.filter(reader -> reader.canRead(FORM_DATA_TYPE, MediaType.APPLICATION_FORM_URLENCODED))
346347
.findFirst()
347348
.orElseThrow(() -> new IllegalStateException("No form data HttpMessageReader.")))
348-
.readMono(FORM_DATA_TYPE, request, Hints.none())
349+
.readMono(FORM_DATA_TYPE, this.request, Hints.none())
350+
.doOnNext(ignored -> this.multipartRead = true)
349351
.switchIfEmpty(EMPTY_FORM_DATA)
350352
.cache();
351353
}
@@ -398,7 +400,23 @@ public Mono<MultiValueMap<String, Part>> getMultipartData() {
398400
return this.multipartDataMono;
399401
}
400402

401-
// Delegating methods
403+
@Override
404+
public Mono<Void> cleanupMultipart() {
405+
if (this.multipartRead) {
406+
return getMultipartData()
407+
.onErrorResume(t -> Mono.empty()) // ignore errors reading multipart data
408+
.flatMapIterable(Map::values)
409+
.flatMapIterable(Function.identity())
410+
.flatMap(part -> part.delete()
411+
.onErrorResume(ex -> Mono.empty()))
412+
.then();
413+
}
414+
else {
415+
return Mono.empty();
416+
}
417+
}
418+
419+
// Delegating methods
402420

403421
@Override
404422
public ServerHttpResponse getResponse() {

0 commit comments

Comments
 (0)