From 82e64af5a565c7ade690174665e9dc8a4c32a5e0 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 18 Sep 2020 19:01:49 +0200 Subject: [PATCH 1/2] Avoid throwing plain RuntimeException (plus related polishing) See gh-24805 --- .../org/springframework/util/StreamUtils.java | 7 ++-- .../function/DefaultServerRequestBuilder.java | 23 +++++------ .../DefaultServerResponseBuilder.java | 41 ++++++++----------- 3 files changed, 33 insertions(+), 38 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/StreamUtils.java b/spring-core/src/main/java/org/springframework/util/StreamUtils.java index 0085f444d8..0fe41fcde0 100644 --- a/spring-core/src/main/java/org/springframework/util/StreamUtils.java +++ b/spring-core/src/main/java/org/springframework/util/StreamUtils.java @@ -97,8 +97,6 @@ public abstract class StreamUtils { /** * Copy the contents of the given {@link ByteArrayOutputStream} into a {@link String}. *

This is a more effective equivalent of {@code new String(baos.toByteArray(), charset)}. - *

As long as the {@code charset} is already available at the point of - * invocation, no exception is expected to be thrown by this method. * @param baos the {@code ByteArrayOutputStream} to be copied into a String * @param charset the {@link Charset} to use to decode the bytes * @return the String that has been copied to (possibly empty) @@ -108,10 +106,12 @@ public abstract class StreamUtils { Assert.notNull(baos, "No ByteArrayOutputStream specified"); Assert.notNull(charset, "No Charset specified"); try { + // Can be replaced with toString(Charset) call in Java 10+ return baos.toString(charset.name()); } catch (UnsupportedEncodingException ex) { - throw new RuntimeException("Failed to copy contents of ByteArrayOutputStream into a String", ex); + // Should never happen + throw new IllegalArgumentException("Invalid charset name: " + charset, ex); } } @@ -261,6 +261,7 @@ public abstract class StreamUtils { return new NonClosingOutputStream(out); } + private static class NonClosingInputStream extends FilterInputStream { public NonClosingInputStream(InputStream in) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerRequestBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerRequestBuilder.java index cc8c44dca7..a3cac53070 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerRequestBuilder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerRequestBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -54,14 +54,15 @@ import org.springframework.web.util.UriComponentsBuilder; /** * Default {@link ServerRequest.Builder} implementation. + * * @author Arjen Poutsma * @since 5.2 */ class DefaultServerRequestBuilder implements ServerRequest.Builder { - private final List> messageConverters; + private final HttpServletRequest servletRequest; - private HttpServletRequest servletRequest; + private final List> messageConverters; private String methodName; @@ -78,8 +79,8 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { public DefaultServerRequestBuilder(ServerRequest other) { Assert.notNull(other, "ServerRequest must not be null"); - this.messageConverters = other.messageConverters(); this.servletRequest = other.servletRequest(); + this.messageConverters = other.messageConverters(); this.methodName = other.methodName(); this.uri = other.uri(); headers(headers -> headers.addAll(other.headers().asHttpHeaders())); @@ -155,10 +156,8 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { @Override public ServerRequest build() { - - return new BuiltServerRequest(this.servletRequest, - this.methodName, this.uri, this.headers, this.cookies, this.attributes, this.body, - this.messageConverters); + return new BuiltServerRequest(this.servletRequest, this.methodName, this.uri, + this.headers, this.cookies, this.attributes, this.body, this.messageConverters); } @@ -172,7 +171,7 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { private final HttpServletRequest servletRequest; - private MultiValueMap cookies; + private final MultiValueMap cookies; private final Map attributes; @@ -184,6 +183,7 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { HttpHeaders headers, MultiValueMap cookies, Map attributes, byte[] body, List> messageConverters) { + this.servletRequest = servletRequest; this.methodName = methodName; this.uri = uri; @@ -241,9 +241,7 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { } @SuppressWarnings("unchecked") - private T bodyInternal(Type bodyType, Class bodyClass) - throws ServletException, IOException { - + private T bodyInternal(Type bodyType, Class bodyClass) throws ServletException, IOException { HttpInputMessage inputMessage = new BuiltInputMessage(); MediaType contentType = headers().contentType().orElse(MediaType.APPLICATION_OCTET_STREAM); @@ -303,6 +301,7 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder { return this.servletRequest; } + private class BuiltInputMessage implements HttpInputMessage { @Override diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerResponseBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerResponseBuilder.java index 515b6a5bf8..aeabdc9128 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerResponseBuilder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/DefaultServerResponseBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -54,6 +54,7 @@ import org.springframework.web.servlet.ModelAndView; /** * Default {@link ServerResponse.BodyBuilder} implementation. + * * @author Arjen Poutsma * @since 5.2 */ @@ -184,6 +185,7 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { @Override public ServerResponse build( BiFunction writeFunction) { + return new WriterFunctionResponse(this.statusCode, this.headers, this.cookies, writeFunction); } @@ -241,7 +243,6 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { private final List> errorHandlers = new ArrayList<>(); - protected AbstractServerResponse( int statusCode, HttpHeaders headers, MultiValueMap cookies) { @@ -322,8 +323,7 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { if (servletResponse.getCharacterEncoding() == null && this.headers.getContentType() != null && this.headers.getContentType().getCharset() != null) { - servletResponse - .setCharacterEncoding(this.headers.getContentType().getCharset().name()); + servletResponse.setCharacterEncoding(this.headers.getContentType().getCharset().name()); } } @@ -334,9 +334,9 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { } @Nullable - protected abstract ModelAndView writeToInternal(HttpServletRequest request, - HttpServletResponse response, Context context) - throws ServletException, IOException; + protected abstract ModelAndView writeToInternal( + HttpServletRequest request, HttpServletResponse response, Context context) + throws ServletException, IOException; @Nullable protected ModelAndView handleError(Throwable t, HttpServletRequest servletRequest, @@ -346,21 +346,20 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { .filter(errorHandler -> errorHandler.test(t)) .findFirst() .map(errorHandler -> { - ServerRequest serverRequest = - (ServerRequest) servletRequest - .getAttribute(RouterFunctions.REQUEST_ATTRIBUTE); + ServerRequest serverRequest = (ServerRequest) + servletRequest.getAttribute(RouterFunctions.REQUEST_ATTRIBUTE); ServerResponse serverResponse = errorHandler.handle(t, serverRequest); try { return serverResponse.writeTo(servletRequest, servletResponse, context); } catch (ServletException ex) { - throw new RuntimeException(ex); + throw new IllegalStateException(ex); } catch (IOException ex) { throw new UncheckedIOException(ex); } }) - .orElseThrow(() -> new RuntimeException(t)); + .orElseThrow(() -> new IllegalStateException(t)); } @@ -373,6 +372,7 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { public ErrorHandler(Predicate predicate, BiFunction responseProvider) { + Assert.notNull(predicate, "Predicate must not be null"); Assert.notNull(responseProvider, "ResponseProvider must not be null"); this.predicate = predicate; @@ -387,8 +387,6 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { return this.responseProvider.apply(t, serverRequest); } } - - } @@ -396,24 +394,21 @@ class DefaultServerResponseBuilder implements ServerResponse.BodyBuilder { private final BiFunction writeFunction; - - public WriterFunctionResponse(int statusCode, HttpHeaders headers, - MultiValueMap cookies, + public WriterFunctionResponse( + int statusCode, HttpHeaders headers, MultiValueMap cookies, BiFunction writeFunction) { + super(statusCode, headers, cookies); Assert.notNull(writeFunction, "WriteFunction must not be null"); this.writeFunction = writeFunction; } @Override - protected ModelAndView writeToInternal(HttpServletRequest request, - HttpServletResponse response, Context context) { + protected ModelAndView writeToInternal( + HttpServletRequest request, HttpServletResponse response, Context context) { + return this.writeFunction.apply(request, response); } } - - - - } From f5d7161d6b4ca2616b300b5407071d950f642c77 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 18 Sep 2020 19:14:11 +0200 Subject: [PATCH 2/2] Consistent flushing of given OutputStream --- .../src/main/java/org/springframework/util/StreamUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/spring-core/src/main/java/org/springframework/util/StreamUtils.java b/spring-core/src/main/java/org/springframework/util/StreamUtils.java index 0fe41fcde0..de59989eb1 100644 --- a/spring-core/src/main/java/org/springframework/util/StreamUtils.java +++ b/spring-core/src/main/java/org/springframework/util/StreamUtils.java @@ -127,6 +127,7 @@ public abstract class StreamUtils { Assert.notNull(out, "No OutputStream specified"); out.write(in); + out.flush(); } /**