From b95362a3fd170a5ee16b90b74d4939efde06db02 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 30 Jun 2022 14:03:21 +0100 Subject: [PATCH 1/3] Avoid request params access for form data in logRequest Close gh-28587 --- .../springframework/web/servlet/DispatcherServlet.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index cd4dc70872b..59a47a07a6a 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -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"); * you may not use this file except in compliance with the License. @@ -978,7 +978,8 @@ public class DispatcherServlet extends FrameworkServlet { private void logRequest(HttpServletRequest request) { LogFormatUtils.traceDebug(logger, traceOn -> { String params; - if (StringUtils.startsWithIgnoreCase(request.getContentType(), "multipart/")) { + String contentType = request.getContentType(); + if (StringUtils.startsWithIgnoreCase(contentType, "multipart/")) { params = "multipart"; } else if (isEnableLoggingRequestDetails()) { @@ -987,7 +988,9 @@ public class DispatcherServlet extends FrameworkServlet { .collect(Collectors.joining(", ")); } else { - params = (request.getParameterMap().isEmpty() ? "" : "masked"); + // Avoid request body parsing for form data + params = (StringUtils.startsWithIgnoreCase(contentType, "application/x-www-form-urlencoded") || + !request.getParameterMap().isEmpty() ? "masked" : ""); } String queryString = request.getQueryString(); From 2afe560e41c52c51a36314de3884416a9b1ed432 Mon Sep 17 00:00:00 2001 From: Vikey Chen Date: Fri, 17 Jun 2022 17:45:00 +0800 Subject: [PATCH 2/3] Replace forEach with putAll See gh-28646 --- .../org/springframework/web/reactive/BindingContext.java | 4 ++-- .../web/reactive/socket/client/JettyWebSocketClient.java | 4 ++-- .../web/reactive/socket/client/StandardWebSocketClient.java | 4 ++-- .../web/reactive/socket/client/UndertowWebSocketClient.java | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java b/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java index d10616e6b64..12b21d61a1a 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java @@ -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"); * you may not use this file except in compliance with the License. @@ -136,7 +136,7 @@ public class BindingContext { return Mono.zip(Mono.just(vars), Mono.just(queryParams), formData, multipartData) .map(tuple -> { Map result = new TreeMap<>(); - tuple.getT1().forEach(result::put); + result.putAll(tuple.getT1()); tuple.getT2().forEach((key, values) -> addBindValue(result, key, values)); tuple.getT3().forEach((key, values) -> addBindValue(result, key, values)); tuple.getT4().forEach((key, values) -> addBindValue(result, key, values)); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/JettyWebSocketClient.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/JettyWebSocketClient.java index 9bbeb08f845..48c46ec67ff 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/JettyWebSocketClient.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/JettyWebSocketClient.java @@ -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"); * you may not use this file except in compliance with the License. @@ -182,7 +182,7 @@ public class JettyWebSocketClient implements WebSocketClient, Lifecycle { private HandshakeInfo createHandshakeInfo(URI url, Session jettySession) { HttpHeaders headers = new HttpHeaders(); - jettySession.getUpgradeResponse().getHeaders().forEach(headers::put); + headers.putAll(jettySession.getUpgradeResponse().getHeaders()); String protocol = headers.getFirst("Sec-WebSocket-Protocol"); return new HandshakeInfo(url, headers, Mono.empty(), protocol); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/StandardWebSocketClient.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/StandardWebSocketClient.java index 4bf98585a05..67ef0e7db4c 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/StandardWebSocketClient.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/StandardWebSocketClient.java @@ -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"); * you may not use this file except in compliance with the License. @@ -171,7 +171,7 @@ public class StandardWebSocketClient implements WebSocketClient { @Override public void afterResponse(HandshakeResponse response) { - response.getHeaders().forEach(this.responseHeaders::put); + this.responseHeaders.putAll(response.getHeaders()); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/UndertowWebSocketClient.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/UndertowWebSocketClient.java index 272e04f345d..e8280c84b44 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/UndertowWebSocketClient.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/UndertowWebSocketClient.java @@ -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"); * you may not use this file except in compliance with the License. @@ -243,7 +243,7 @@ public class UndertowWebSocketClient implements WebSocketClient { @Override public void beforeRequest(Map> headers) { - this.requestHeaders.forEach(headers::put); + headers.putAll(this.requestHeaders); if (this.delegate != null) { this.delegate.beforeRequest(headers); } @@ -251,7 +251,7 @@ public class UndertowWebSocketClient implements WebSocketClient { @Override public void afterRequest(Map> headers) { - headers.forEach(this.responseHeaders::put); + this.responseHeaders.putAll(headers); if (this.delegate != null) { this.delegate.afterRequest(headers); } From 058ce36402049c121ef7a61ce0a5848997753b66 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 30 Jun 2022 16:26:14 +0100 Subject: [PATCH 3/3] Improve ExtendedWebExchangeDataBinder implementation Close gh-28646 --- .../bind/support/WebExchangeDataBinder.java | 4 ++-- .../web/reactive/BindingContext.java | 21 +++---------------- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java index b4957970a5d..9ac40eb9e20 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java @@ -84,8 +84,8 @@ public class WebExchangeDataBinder extends WebDataBinder { } /** - * Protected method to obtain the values for data binding. By default this - * method delegates to {@link #extractValuesToBind(ServerWebExchange)}. + * Obtain the values for data binding. By default, this delegates to + * {@link #extractValuesToBind(ServerWebExchange)}. * @param exchange the current exchange * @return a map of bind values * @since 5.3 diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java b/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java index 12b21d61a1a..0756cc8b58f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java @@ -18,14 +18,11 @@ package org.springframework.web.reactive; import java.util.Collections; import java.util.Map; -import java.util.TreeMap; import reactor.core.publisher.Mono; -import org.springframework.http.codec.multipart.Part; import org.springframework.lang.Nullable; import org.springframework.ui.Model; -import org.springframework.util.MultiValueMap; import org.springframework.validation.support.BindingAwareConcurrentModel; import org.springframework.web.bind.support.WebBindingInitializer; import org.springframework.web.bind.support.WebExchangeDataBinder; @@ -127,21 +124,9 @@ public class BindingContext { @Override public Mono> getValuesToBind(ServerWebExchange exchange) { - Map vars = exchange.getAttributeOrDefault( - HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, Collections.emptyMap()); - MultiValueMap queryParams = exchange.getRequest().getQueryParams(); - Mono> formData = exchange.getFormData(); - Mono> multipartData = exchange.getMultipartData(); - - return Mono.zip(Mono.just(vars), Mono.just(queryParams), formData, multipartData) - .map(tuple -> { - Map result = new TreeMap<>(); - result.putAll(tuple.getT1()); - tuple.getT2().forEach((key, values) -> addBindValue(result, key, values)); - tuple.getT3().forEach((key, values) -> addBindValue(result, key, values)); - tuple.getT4().forEach((key, values) -> addBindValue(result, key, values)); - return result; - }); + return super.getValuesToBind(exchange).doOnNext(map -> + map.putAll(exchange.>getAttributeOrDefault( + HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, Collections.emptyMap()))); } }