Merge branch '5.3.x'

# Conflicts:
#	build.gradle
This commit is contained in:
Juergen Hoeller 2022-01-12 16:38:19 +01:00
commit 50faa29329
6 changed files with 105 additions and 54 deletions

View File

@ -170,14 +170,14 @@ configure(allprojects) { project ->
dependency "org.junit.support:testng-engine:1.0.1" dependency "org.junit.support:testng-engine:1.0.1"
dependency "org.hamcrest:hamcrest:2.1" dependency "org.hamcrest:hamcrest:2.1"
dependency "org.awaitility:awaitility:3.1.6" dependency "org.awaitility:awaitility:3.1.6"
dependency "org.assertj:assertj-core:3.21.0" dependency "org.assertj:assertj-core:3.22.0"
dependencySet(group: 'org.xmlunit', version: '2.8.3') { dependencySet(group: 'org.xmlunit', version: '2.8.4') {
entry 'xmlunit-assertj' entry 'xmlunit-assertj'
entry('xmlunit-matchers') { entry('xmlunit-matchers') {
exclude group: "org.hamcrest", name: "hamcrest-core" exclude group: "org.hamcrest", name: "hamcrest-core"
} }
} }
dependencySet(group: 'org.mockito', version: '4.1.0') { dependencySet(group: 'org.mockito', version: '4.2.0') {
entry('mockito-core') { entry('mockito-core') {
exclude group: "org.hamcrest", name: "hamcrest-core" exclude group: "org.hamcrest", name: "hamcrest-core"
} }
@ -185,10 +185,10 @@ configure(allprojects) { project ->
} }
dependency "io.mockk:mockk:1.12.1" dependency "io.mockk:mockk:1.12.1"
dependency("net.sourceforge.htmlunit:htmlunit:2.55.0") { dependency("net.sourceforge.htmlunit:htmlunit:2.56.0") {
exclude group: "commons-logging", name: "commons-logging" exclude group: "commons-logging", name: "commons-logging"
} }
dependency("org.seleniumhq.selenium:htmlunit-driver:2.55.0") { dependency("org.seleniumhq.selenium:htmlunit-driver:2.56.0") {
exclude group: "commons-logging", name: "commons-logging" exclude group: "commons-logging", name: "commons-logging"
} }
dependency("org.seleniumhq.selenium:selenium-java:3.141.59") { dependency("org.seleniumhq.selenium:selenium-java:3.141.59") {

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2021 the original author or authors. * Copyright 2002-2022 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -42,6 +42,7 @@ import org.springframework.lang.Nullable;
* *
* @author Sebastien Deleuze * @author Sebastien Deleuze
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @author Juergen Hoeller
* @since 5.0 * @since 5.0
*/ */
public class ServerSentEventHttpMessageReader implements HttpMessageReader<Object> { public class ServerSentEventHttpMessageReader implements HttpMessageReader<Object> {
@ -140,22 +141,23 @@ public class ServerSentEventHttpMessageReader implements HttpMessageReader<Objec
private Object buildEvent(List<String> lines, ResolvableType valueType, boolean shouldWrap, private Object buildEvent(List<String> lines, ResolvableType valueType, boolean shouldWrap,
Map<String, Object> hints) { Map<String, Object> hints) {
ServerSentEvent.Builder<Object> sseBuilder = shouldWrap ? ServerSentEvent.builder() : null; ServerSentEvent.Builder<Object> sseBuilder = (shouldWrap ? ServerSentEvent.builder() : null);
StringBuilder data = null; StringBuilder data = null;
StringBuilder comment = null; StringBuilder comment = null;
for (String line : lines) { for (String line : lines) {
if (line.startsWith("data:")) { if (line.startsWith("data:")) {
data = (data != null ? data : new StringBuilder()); int length = line.length();
if (line.charAt(5) != ' ') { if (length > 5) {
data.append(line, 5, line.length()); int index = (line.charAt(5) != ' ' ? 5 : 6);
if (length > index) {
data = (data != null ? data : new StringBuilder());
data.append(line, index, line.length());
data.append('\n');
}
} }
else {
data.append(line, 6, line.length());
}
data.append('\n');
} }
if (shouldWrap) { else if (shouldWrap) {
if (line.startsWith("id:")) { if (line.startsWith("id:")) {
sseBuilder.id(line.substring(3).trim()); sseBuilder.id(line.substring(3).trim());
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2020 the original author or authors. * Copyright 2002-2022 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -40,6 +40,7 @@ import static org.assertj.core.api.Assertions.assertThat;
* Unit tests for {@link ServerSentEventHttpMessageReader}. * Unit tests for {@link ServerSentEventHttpMessageReader}.
* *
* @author Sebastien Deleuze * @author Sebastien Deleuze
* @author Juergen Hoeller
*/ */
public class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingTests { public class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingTests {
@ -66,7 +67,7 @@ public class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingT
MockServerHttpRequest request = MockServerHttpRequest.post("/") MockServerHttpRequest request = MockServerHttpRequest.post("/")
.body(Mono.just(stringBuffer( .body(Mono.just(stringBuffer(
"id:c42\nevent:foo\nretry:123\n:bla\n:bla bla\n:bla bla bla\ndata:bar\n\n" + "id:c42\nevent:foo\nretry:123\n:bla\n:bla bla\n:bla bla bla\ndata:bar\n\n" +
"id:c43\nevent:bar\nretry:456\ndata:baz\n\n"))); "id:c43\nevent:bar\nretry:456\ndata:baz\n\ndata:\n\ndata: \n\n")));
Flux<ServerSentEvent> events = this.reader Flux<ServerSentEvent> events = this.reader
.read(ResolvableType.forClassWithGenerics(ServerSentEvent.class, String.class), .read(ResolvableType.forClassWithGenerics(ServerSentEvent.class, String.class),
@ -87,6 +88,8 @@ public class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingT
assertThat(event.comment()).isNull(); assertThat(event.comment()).isNull();
assertThat(event.data()).isEqualTo("baz"); assertThat(event.data()).isEqualTo("baz");
}) })
.consumeNextWith(event -> assertThat(event.data()).isNull())
.consumeNextWith(event -> assertThat(event.data()).isNull())
.expectComplete() .expectComplete()
.verify(); .verify();
} }
@ -175,7 +178,7 @@ public class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingT
.verify(); .verify();
} }
@Test // gh-24389 @Test // gh-24389
void readPojoWithCommentOnly() { void readPojoWithCommentOnly() {
MockServerHttpRequest request = MockServerHttpRequest.post("/") MockServerHttpRequest request = MockServerHttpRequest.post("/")
.body(Flux.just(stringBuffer(":ping\n"), stringBuffer("\n"))); .body(Flux.just(stringBuffer(":ping\n"), stringBuffer("\n")));
@ -221,7 +224,6 @@ public class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingT
@Test @Test
public void maxInMemoryLimit() { public void maxInMemoryLimit() {
this.reader.setMaxInMemorySize(17); this.reader.setMaxInMemorySize(17);
MockServerHttpRequest request = MockServerHttpRequest.post("/") MockServerHttpRequest request = MockServerHttpRequest.post("/")
@ -235,9 +237,8 @@ public class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingT
.verify(); .verify();
} }
@Test // gh-24312 @Test // gh-24312
public void maxInMemoryLimitAllowsReadingPojoLargerThanDefaultSize() { public void maxInMemoryLimitAllowsReadingPojoLargerThanDefaultSize() {
int limit = this.jsonDecoder.getMaxInMemorySize(); int limit = this.jsonDecoder.getMaxInMemorySize();
String fooValue = getStringOfSize(limit) + "and then some more"; String fooValue = getStringOfSize(limit) + "and then some more";
@ -259,13 +260,6 @@ public class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingT
.verify(); .verify();
} }
private static String getStringOfSize(long size) {
StringBuilder content = new StringBuilder("Aa");
while (content.length() < size) {
content.append(content);
}
return content.toString();
}
private DataBuffer stringBuffer(String value) { private DataBuffer stringBuffer(String value) {
byte[] bytes = value.getBytes(StandardCharsets.UTF_8); byte[] bytes = value.getBytes(StandardCharsets.UTF_8);
@ -274,4 +268,12 @@ public class ServerSentEventHttpMessageReaderTests extends AbstractLeakCheckingT
return buffer; return buffer;
} }
private static String getStringOfSize(long size) {
StringBuilder content = new StringBuilder("Aa");
while (content.length() < size) {
content.append(content);
}
return content.toString();
}
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2021 the original author or authors. * Copyright 2002-2022 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -167,7 +167,7 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements
HttpMethod httpMethod = (inputMessage instanceof HttpRequest ? ((HttpRequest) inputMessage).getMethod() : null); HttpMethod httpMethod = (inputMessage instanceof HttpRequest ? ((HttpRequest) inputMessage).getMethod() : null);
Object body = NO_VALUE; Object body = NO_VALUE;
EmptyBodyCheckingHttpInputMessage message; EmptyBodyCheckingHttpInputMessage message = null;
try { try {
message = new EmptyBodyCheckingHttpInputMessage(inputMessage); message = new EmptyBodyCheckingHttpInputMessage(inputMessage);
@ -194,6 +194,11 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements
catch (IOException ex) { catch (IOException ex) {
throw new HttpMessageNotReadableException("I/O error while reading input message", ex, inputMessage); throw new HttpMessageNotReadableException("I/O error while reading input message", ex, inputMessage);
} }
finally {
if (message != null && message.hasBody()) {
closeStreamIfNecessary(message.getBody());
}
}
if (body == NO_VALUE) { if (body == NO_VALUE) {
if (httpMethod == null || !SUPPORTED_METHODS.contains(httpMethod) || if (httpMethod == null || !SUPPORTED_METHODS.contains(httpMethod) ||
@ -296,6 +301,15 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements
return arg; return arg;
} }
/**
* Allow for closing the body stream if necessary,
* e.g. for part streams in a multipart request.
*/
void closeStreamIfNecessary(InputStream body) {
// No-op by default: A standard HttpInputMessage exposes the HTTP request stream
// (ServletRequest#getInputStream), with its lifecycle managed by the container.
}
private static class EmptyBodyCheckingHttpInputMessage implements HttpInputMessage { private static class EmptyBodyCheckingHttpInputMessage implements HttpInputMessage {

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2021 the original author or authors. * Copyright 2002-2022 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,6 +16,8 @@
package org.springframework.web.servlet.mvc.method.annotation; package org.springframework.web.servlet.mvc.method.annotation;
import java.io.IOException;
import java.io.InputStream;
import java.util.List; import java.util.List;
import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequest;
@ -180,4 +182,17 @@ public class RequestPartMethodArgumentResolver extends AbstractMessageConverterM
return partName; return partName;
} }
@Override
void closeStreamIfNecessary(InputStream body) {
// RequestPartServletServerHttpRequest exposes individual part streams,
// potentially from temporary files -> explicit close call after resolution
// in order to prevent file descriptor leaks.
try {
body.close();
}
catch (IOException ex) {
// ignore
}
}
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2021 the original author or authors. * Copyright 2002-2022 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,6 +16,9 @@
package org.springframework.web.servlet.mvc.method.annotation; package org.springframework.web.servlet.mvc.method.annotation;
import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.util.Arrays; import java.util.Arrays;
@ -82,6 +85,8 @@ public class RequestPartMethodArgumentResolverTests {
private MultipartFile multipartFile2; private MultipartFile multipartFile2;
private CloseTrackingInputStream trackedStream;
private MockMultipartHttpServletRequest multipartRequest; private MockMultipartHttpServletRequest multipartRequest;
private NativeWebRequest webRequest; private NativeWebRequest webRequest;
@ -115,7 +120,14 @@ public class RequestPartMethodArgumentResolverTests {
reset(messageConverter); reset(messageConverter);
byte[] content = "doesn't matter as long as not empty".getBytes(StandardCharsets.UTF_8); byte[] content = "doesn't matter as long as not empty".getBytes(StandardCharsets.UTF_8);
multipartFile1 = new MockMultipartFile("requestPart", "", "text/plain", content); multipartFile1 = new MockMultipartFile("requestPart", "", "text/plain", content) {
@Override
public InputStream getInputStream() throws IOException {
CloseTrackingInputStream in = new CloseTrackingInputStream(super.getInputStream());
trackedStream = in;
return in;
}
};
multipartFile2 = new MockMultipartFile("requestPart", "", "text/plain", content); multipartFile2 = new MockMultipartFile("requestPart", "", "text/plain", content);
multipartRequest = new MockMultipartHttpServletRequest(); multipartRequest = new MockMultipartHttpServletRequest();
multipartRequest.addFile(multipartFile1); multipartRequest.addFile(multipartFile1);
@ -181,8 +193,7 @@ public class RequestPartMethodArgumentResolverTests {
@Test @Test
public void resolveMultipartFileList() throws Exception { public void resolveMultipartFileList() throws Exception {
Object actual = resolver.resolveArgument(paramMultipartFileList, null, webRequest, null); Object actual = resolver.resolveArgument(paramMultipartFileList, null, webRequest, null);
boolean condition = actual instanceof List; assertThat(actual instanceof List).isTrue();
assertThat(condition).isTrue();
assertThat(actual).isEqualTo(Arrays.asList(multipartFile1, multipartFile2)); assertThat(actual).isEqualTo(Arrays.asList(multipartFile1, multipartFile2));
} }
@ -190,8 +201,7 @@ public class RequestPartMethodArgumentResolverTests {
public void resolveMultipartFileArray() throws Exception { public void resolveMultipartFileArray() throws Exception {
Object actual = resolver.resolveArgument(paramMultipartFileArray, null, webRequest, null); Object actual = resolver.resolveArgument(paramMultipartFileArray, null, webRequest, null);
assertThat(actual).isNotNull(); assertThat(actual).isNotNull();
boolean condition = actual instanceof MultipartFile[]; assertThat(actual instanceof MultipartFile[]).isTrue();
assertThat(condition).isTrue();
MultipartFile[] parts = (MultipartFile[]) actual; MultipartFile[] parts = (MultipartFile[]) actual;
assertThat(parts.length).isEqualTo(2); assertThat(parts.length).isEqualTo(2);
assertThat(multipartFile1).isEqualTo(parts[0]); assertThat(multipartFile1).isEqualTo(parts[0]);
@ -208,8 +218,7 @@ public class RequestPartMethodArgumentResolverTests {
Object result = resolver.resolveArgument(paramMultipartFileNotAnnot, null, webRequest, null); Object result = resolver.resolveArgument(paramMultipartFileNotAnnot, null, webRequest, null);
boolean condition = result instanceof MultipartFile; assertThat(result instanceof MultipartFile).isTrue();
assertThat(condition).isTrue();
assertThat(result).as("Invalid result").isEqualTo(expected); assertThat(result).as("Invalid result").isEqualTo(expected);
} }
@ -224,8 +233,7 @@ public class RequestPartMethodArgumentResolverTests {
webRequest = new ServletWebRequest(request); webRequest = new ServletWebRequest(request);
Object result = resolver.resolveArgument(paramPart, null, webRequest, null); Object result = resolver.resolveArgument(paramPart, null, webRequest, null);
boolean condition = result instanceof Part; assertThat(result instanceof Part).isTrue();
assertThat(condition).isTrue();
assertThat(result).as("Invalid result").isEqualTo(expected); assertThat(result).as("Invalid result").isEqualTo(expected);
} }
@ -242,8 +250,7 @@ public class RequestPartMethodArgumentResolverTests {
webRequest = new ServletWebRequest(request); webRequest = new ServletWebRequest(request);
Object result = resolver.resolveArgument(paramPartList, null, webRequest, null); Object result = resolver.resolveArgument(paramPartList, null, webRequest, null);
boolean condition = result instanceof List; assertThat(result instanceof List).isTrue();
assertThat(condition).isTrue();
assertThat(result).isEqualTo(Arrays.asList(part1, part2)); assertThat(result).isEqualTo(Arrays.asList(part1, part2));
} }
@ -260,8 +267,7 @@ public class RequestPartMethodArgumentResolverTests {
webRequest = new ServletWebRequest(request); webRequest = new ServletWebRequest(request);
Object result = resolver.resolveArgument(paramPartArray, null, webRequest, null); Object result = resolver.resolveArgument(paramPartArray, null, webRequest, null);
boolean condition = result instanceof Part[]; assertThat(result instanceof Part[]).isTrue();
assertThat(condition).isTrue();
Part[] parts = (Part[]) result; Part[] parts = (Part[]) result;
assertThat(parts.length).isEqualTo(2); assertThat(parts.length).isEqualTo(2);
assertThat(part1).isEqualTo(parts[0]); assertThat(part1).isEqualTo(parts[0]);
@ -356,8 +362,7 @@ public class RequestPartMethodArgumentResolverTests {
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected); assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected);
actualValue = resolver.resolveArgument(optionalMultipartFile, null, webRequest, null); actualValue = resolver.resolveArgument(optionalMultipartFile, null, webRequest, null);
boolean condition = actualValue instanceof Optional; assertThat(actualValue instanceof Optional).isTrue();
assertThat(condition).isTrue();
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected); assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected);
} }
@ -398,8 +403,7 @@ public class RequestPartMethodArgumentResolverTests {
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected)); assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected));
actualValue = resolver.resolveArgument(optionalMultipartFileList, null, webRequest, null); actualValue = resolver.resolveArgument(optionalMultipartFileList, null, webRequest, null);
boolean condition = actualValue instanceof Optional; assertThat(actualValue instanceof Optional).isTrue();
assertThat(condition).isTrue();
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected)); assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected));
} }
@ -442,8 +446,7 @@ public class RequestPartMethodArgumentResolverTests {
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected); assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected);
actualValue = resolver.resolveArgument(optionalPart, null, webRequest, null); actualValue = resolver.resolveArgument(optionalPart, null, webRequest, null);
boolean condition = actualValue instanceof Optional; assertThat(actualValue instanceof Optional).isTrue();
assertThat(condition).isTrue();
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected); assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(expected);
} }
@ -488,8 +491,7 @@ public class RequestPartMethodArgumentResolverTests {
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected)); assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected));
actualValue = resolver.resolveArgument(optionalPartList, null, webRequest, null); actualValue = resolver.resolveArgument(optionalPartList, null, webRequest, null);
boolean condition = actualValue instanceof Optional; assertThat(actualValue instanceof Optional).isTrue();
assertThat(condition).isTrue();
assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected)); assertThat(((Optional<?>) actualValue).get()).as("Invalid result").isEqualTo(Collections.singletonList(expected));
} }
@ -571,6 +573,7 @@ public class RequestPartMethodArgumentResolverTests {
Object actualValue = resolver.resolveArgument(parameter, mavContainer, webRequest, new ValidatingBinderFactory()); Object actualValue = resolver.resolveArgument(parameter, mavContainer, webRequest, new ValidatingBinderFactory());
assertThat(actualValue).as("Invalid argument value").isEqualTo(argValue); assertThat(actualValue).as("Invalid argument value").isEqualTo(argValue);
assertThat(mavContainer.isRequestHandled()).as("The requestHandled flag shouldn't change").isFalse(); assertThat(mavContainer.isRequestHandled()).as("The requestHandled flag shouldn't change").isFalse();
assertThat(trackedStream != null && trackedStream.closed).isTrue();
} }
@ -590,7 +593,7 @@ public class RequestPartMethodArgumentResolverTests {
} }
private final class ValidatingBinderFactory implements WebDataBinderFactory { private static class ValidatingBinderFactory implements WebDataBinderFactory {
@Override @Override
public WebDataBinder createBinder(NativeWebRequest webRequest, @Nullable Object target, public WebDataBinder createBinder(NativeWebRequest webRequest, @Nullable Object target,
@ -605,6 +608,21 @@ public class RequestPartMethodArgumentResolverTests {
} }
private static class CloseTrackingInputStream extends FilterInputStream {
public boolean closed = false;
public CloseTrackingInputStream(InputStream in) {
super(in);
}
@Override
public void close() {
this.closed = true;
}
}
@SuppressWarnings("unused") @SuppressWarnings("unused")
public void handle( public void handle(
@RequestPart SimpleBean requestPart, @RequestPart SimpleBean requestPart,