Skip to content

Commit cf5b863

Browse files
jhoellerjhoeller
committed
Explicitly close InputStream after resolution in RequestPartMethodArgumentResolver
Closes gh-27773
1 parent 7067461 commit cf5b863

File tree

3 files changed

+73
-26
lines changed

3 files changed

+73
-26
lines changed

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java

+16-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 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.
@@ -169,7 +169,7 @@ protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage, Me
169169
HttpMethod httpMethod = (inputMessage instanceof HttpRequest ? ((HttpRequest) inputMessage).getMethod() : null);
170170
Object body = NO_VALUE;
171171

172-
EmptyBodyCheckingHttpInputMessage message;
172+
EmptyBodyCheckingHttpInputMessage message = null;
173173
try {
174174
message = new EmptyBodyCheckingHttpInputMessage(inputMessage);
175175

@@ -196,6 +196,11 @@ protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage, Me
196196
catch (IOException ex) {
197197
throw new HttpMessageNotReadableException("I/O error while reading input message", ex, inputMessage);
198198
}
199+
finally {
200+
if (message != null && message.hasBody()) {
201+
closeStreamIfNecessary(message.getBody());
202+
}
203+
}
199204

200205
if (body == NO_VALUE) {
201206
if (httpMethod == null || !SUPPORTED_METHODS.contains(httpMethod) ||
@@ -298,6 +303,15 @@ protected Object adaptArgumentIfNecessary(@Nullable Object arg, MethodParameter
298303
return arg;
299304
}
300305

306+
/**
307+
* Allow for closing the body stream if necessary,
308+
* e.g. for part streams in a multipart request.
309+
*/
310+
void closeStreamIfNecessary(InputStream body) {
311+
// No-op by default: A standard HttpInputMessage exposes the HTTP request stream
312+
// (ServletRequest#getInputStream), with its lifecycle managed by the container.
313+
}
314+
301315

302316
private static class EmptyBodyCheckingHttpInputMessage implements HttpInputMessage {
303317

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolver.java

+16-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 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,6 +16,8 @@
1616

1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

19+
import java.io.IOException;
20+
import java.io.InputStream;
1921
import java.util.List;
2022

2123
import javax.servlet.http.HttpServletRequest;
@@ -180,4 +182,17 @@ private String getPartName(MethodParameter methodParam, @Nullable RequestPart re
180182
return partName;
181183
}
182184

185+
@Override
186+
void closeStreamIfNecessary(InputStream body) {
187+
// RequestPartServletServerHttpRequest exposes individual part streams,
188+
// potentially from temporary files -> explicit close call after resolution
189+
// in order to prevent file descriptor leaks.
190+
try {
191+
body.close();
192+
}
193+
catch (IOException ex) {
194+
// ignore
195+
}
196+
}
197+
183198
}

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolverTests.java

+41-23
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 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,6 +16,9 @@
1616

1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

19+
import java.io.FilterInputStream;
20+
import java.io.IOException;
21+
import java.io.InputStream;
1922
import java.lang.reflect.Method;
2023
import java.nio.charset.StandardCharsets;
2124
import java.util.Arrays;
@@ -83,6 +86,8 @@ public class RequestPartMethodArgumentResolverTests {
8386

8487
private MultipartFile multipartFile2;
8588

89+
private CloseTrackingInputStream trackedStream;
90+
8691
private MockMultipartHttpServletRequest multipartRequest;
8792

8893
private NativeWebRequest webRequest;
@@ -116,7 +121,14 @@ public void setup() throws Exception {
116121
reset(messageConverter);
117122

118123
byte[] content = "doesn't matter as long as not empty".getBytes(StandardCharsets.UTF_8);
119-
multipartFile1 = new MockMultipartFile("requestPart", "", "text/plain", content);
124+
multipartFile1 = new MockMultipartFile("requestPart", "", "text/plain", content) {
125+
@Override
126+
public InputStream getInputStream() throws IOException {
127+
CloseTrackingInputStream in = new CloseTrackingInputStream(super.getInputStream());
128+
trackedStream = in;
129+
return in;
130+
}
131+
};
120132
multipartFile2 = new MockMultipartFile("requestPart", "", "text/plain", content);
121133
multipartRequest = new MockMultipartHttpServletRequest();
122134
multipartRequest.addFile(multipartFile1);
@@ -182,17 +194,15 @@ public void resolveMultipartFile() throws Exception {
182194
@Test
183195
public void resolveMultipartFileList() throws Exception {
184196
Object actual = resolver.resolveArgument(paramMultipartFileList, null, webRequest, null);
185-
boolean condition = actual instanceof List;
186-
assertThat(condition).isTrue();
197+
assertThat(actual instanceof List).isTrue();
187198
assertThat(actual).isEqualTo(Arrays.asList(multipartFile1, multipartFile2));
188199
}
189200

190201
@Test
191202
public void resolveMultipartFileArray() throws Exception {
192203
Object actual = resolver.resolveArgument(paramMultipartFileArray, null, webRequest, null);
193204
assertThat(actual).isNotNull();
194-
boolean condition = actual instanceof MultipartFile[];
195-
assertThat(condition).isTrue();
205+
assertThat(actual instanceof MultipartFile[]).isTrue();
196206
MultipartFile[] parts = (MultipartFile[]) actual;
197207
assertThat(parts.length).isEqualTo(2);
198208
assertThat(multipartFile1).isEqualTo(parts[0]);
@@ -209,8 +219,7 @@ public void resolveMultipartFileNotAnnotArgument() throws Exception {
209219

210220
Object result = resolver.resolveArgument(paramMultipartFileNotAnnot, null, webRequest, null);
211221

212-
boolean condition = result instanceof MultipartFile;
213-
assertThat(condition).isTrue();
222+
assertThat(result instanceof MultipartFile).isTrue();
214223
assertThat(result).as("Invalid result").isEqualTo(expected);
215224
}
216225

@@ -225,8 +234,7 @@ public void resolvePartArgument() throws Exception {
225234
webRequest = new ServletWebRequest(request);
226235

227236
Object result = resolver.resolveArgument(paramPart, null, webRequest, null);
228-
boolean condition = result instanceof Part;
229-
assertThat(condition).isTrue();
237+
assertThat(result instanceof Part).isTrue();
230238
assertThat(result).as("Invalid result").isEqualTo(expected);
231239
}
232240

@@ -243,8 +251,7 @@ public void resolvePartListArgument() throws Exception {
243251
webRequest = new ServletWebRequest(request);
244252

245253
Object result = resolver.resolveArgument(paramPartList, null, webRequest, null);
246-
boolean condition = result instanceof List;
247-
assertThat(condition).isTrue();
254+
assertThat(result instanceof List).isTrue();
248255
assertThat(result).isEqualTo(Arrays.asList(part1, part2));
249256
}
250257

@@ -261,8 +268,7 @@ public void resolvePartArrayArgument() throws Exception {
261268
webRequest = new ServletWebRequest(request);
262269

263270
Object result = resolver.resolveArgument(paramPartArray, null, webRequest, null);
264-
boolean condition = result instanceof Part[];
265-
assertThat(condition).isTrue();
271+
assertThat(result instanceof Part[]).isTrue();
266272
Part[] parts = (Part[]) result;
267273
assertThat(parts.length).isEqualTo(2);
268274
assertThat(part1).isEqualTo(parts[0]);
@@ -357,8 +363,7 @@ public void resolveOptionalMultipartFileArgument() throws Exception {
357363
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected);
358364

359365
actualValue = resolver.resolveArgument(optionalMultipartFile, null, webRequest, null);
360-
boolean condition = actualValue instanceof Optional;
361-
assertThat(condition).isTrue();
366+
assertThat(actualValue instanceof Optional).isTrue();
362367
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected);
363368
}
364369

@@ -399,8 +404,7 @@ public void resolveOptionalMultipartFileList() throws Exception {
399404
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected));
400405

401406
actualValue = resolver.resolveArgument(optionalMultipartFileList, null, webRequest, null);
402-
boolean condition = actualValue instanceof Optional;
403-
assertThat(condition).isTrue();
407+
assertThat(actualValue instanceof Optional).isTrue();
404408
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected));
405409
}
406410

@@ -443,8 +447,7 @@ public void resolveOptionalPartArgument() throws Exception {
443447
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected);
444448

445449
actualValue = resolver.resolveArgument(optionalPart, null, webRequest, null);
446-
boolean condition = actualValue instanceof Optional;
447-
assertThat(condition).isTrue();
450+
assertThat(actualValue instanceof Optional).isTrue();
448451
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected);
449452
}
450453

@@ -489,8 +492,7 @@ public void resolveOptionalPartList() throws Exception {
489492
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected));
490493

491494
actualValue = resolver.resolveArgument(optionalPartList, null, webRequest, null);
492-
boolean condition = actualValue instanceof Optional;
493-
assertThat(condition).isTrue();
495+
assertThat(actualValue instanceof Optional).isTrue();
494496
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected));
495497
}
496498

@@ -572,6 +574,7 @@ private void testResolveArgument(SimpleBean argValue, MethodParameter parameter)
572574
Object actualValue = resolver.resolveArgument(parameter, mavContainer, webRequest, new ValidatingBinderFactory());
573575
assertThat(actualValue).as("Invalid argument value").isEqualTo(argValue);
574576
assertThat(mavContainer.isRequestHandled()).as("The requestHandled flag shouldn't change").isFalse();
577+
assertThat(trackedStream != null && trackedStream.closed).isTrue();
575578
}
576579

577580

@@ -591,7 +594,7 @@ public String getName() {
591594
}
592595

593596

594-
private final class ValidatingBinderFactory implements WebDataBinderFactory {
597+
private static class ValidatingBinderFactory implements WebDataBinderFactory {
595598

596599
@Override
597600
public WebDataBinder createBinder(NativeWebRequest webRequest, @Nullable Object target,
@@ -606,6 +609,21 @@ public WebDataBinder createBinder(NativeWebRequest webRequest, @Nullable Object
606609
}
607610

608611

612+
private static class CloseTrackingInputStream extends FilterInputStream {
613+
614+
public boolean closed = false;
615+
616+
public CloseTrackingInputStream(InputStream in) {
617+
super(in);
618+
}
619+
620+
@Override
621+
public void close() {
622+
this.closed = true;
623+
}
624+
}
625+
626+
609627
@SuppressWarnings("unused")
610628
public void handle(
611629
@RequestPart SimpleBean requestPart,

0 commit comments

Comments
 (0)