From 96a7fc693b3bada8b3f428fd2ddbde63059e0087 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 9 Dec 2021 14:52:34 +0000 Subject: [PATCH] Apply LogFormatUtils in more places --- .../springframework/web/util/UrlPathHelper.java | 4 ++-- .../reactive/resource/PathResourceResolver.java | 15 +++++++++------ .../web/reactive/resource/ResourceWebHandler.java | 10 +++++++--- .../ExtendedServletRequestDataBinder.java | 7 +++---- .../method/annotation/ReactiveTypeHandler.java | 4 ++-- .../servlet/resource/PathResourceResolver.java | 15 +++++++++------ .../resource/ResourceHttpRequestHandler.java | 10 +++++++--- .../server/support/AbstractHandshakeHandler.java | 12 ++++++++---- .../sockjs/support/AbstractSockJsService.java | 7 +++++-- .../transport/TransportHandlingSockJsService.java | 7 ++++--- 10 files changed, 56 insertions(+), 35 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java index dc2f6424a4d..55cb6848946 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java +++ b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java @@ -521,8 +521,8 @@ public class UrlPathHelper { return UriUtils.decode(source, enc); } catch (UnsupportedCharsetException ex) { - if (logger.isWarnEnabled()) { - logger.warn("Could not decode request string [" + source + "] with encoding '" + enc + + if (logger.isDebugEnabled()) { + logger.debug("Could not decode request string [" + source + "] with encoding '" + enc + "': falling back to platform default encoding; exception message: " + ex.getMessage()); } return URLDecoder.decode(source); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java index beafe65d0d2..37d039381da 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java @@ -29,6 +29,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.core.io.UrlResource; +import org.springframework.core.log.LogFormatUtils; import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; @@ -119,11 +120,12 @@ public class PathResourceResolver extends AbstractResourceResolver { return Mono.just(resource); } else if (logger.isWarnEnabled()) { - Resource[] allowedLocations = getAllowedLocations(); - logger.warn("Resource path \"" + resourcePath + "\" was successfully resolved " + - "but resource \"" + resource.getURL() + "\" is neither under the " + - "current location \"" + location.getURL() + "\" nor under any of the " + - "allowed locations " + (allowedLocations != null ? Arrays.asList(allowedLocations) : "[]")); + Resource[] allowed = getAllowedLocations(); + logger.warn(LogFormatUtils.formatValue( + "Resource path \"" + resourcePath + "\" was successfully resolved " + + "but resource \"" + resource.getURL() + "\" is neither under the " + + "current location \"" + location.getURL() + "\" nor under any of the " + + "allowed locations " + (allowed != null ? Arrays.asList(allowed) : "[]"), -1, true)); } } return Mono.empty(); @@ -199,7 +201,8 @@ public class PathResourceResolver extends AbstractResourceResolver { try { String decodedPath = URLDecoder.decode(resourcePath, "UTF-8"); if (decodedPath.contains("../") || decodedPath.contains("..\\")) { - logger.warn("Resolved resource path contains encoded \"../\" or \"..\\\": " + resourcePath); + logger.warn(LogFormatUtils.formatValue( + "Resolved resource path contains encoded \"../\" or \"..\\\": " + resourcePath, -1, true)); return true; } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index 9da249f7c0e..fdf57c5764a 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java @@ -36,6 +36,7 @@ import org.springframework.core.ResolvableType; import org.springframework.core.codec.Hints; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.CacheControl; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; @@ -490,7 +491,8 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { protected boolean isInvalidPath(String path) { if (path.contains("WEB-INF") || path.contains("META-INF")) { if (logger.isWarnEnabled()) { - logger.warn("Path with \"WEB-INF\" or \"META-INF\": [" + path + "]"); + logger.warn(LogFormatUtils.formatValue( + "Path with \"WEB-INF\" or \"META-INF\": [" + path + "]", -1, true)); } return true; } @@ -498,14 +500,16 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { String relativePath = (path.charAt(0) == '/' ? path.substring(1) : path); if (ResourceUtils.isUrl(relativePath) || relativePath.startsWith("url:")) { if (logger.isWarnEnabled()) { - logger.warn("Path represents URL or has \"url:\" prefix: [" + path + "]"); + logger.warn(LogFormatUtils.formatValue( + "Path represents URL or has \"url:\" prefix: [" + path + "]", -1, true)); } return true; } } if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) { if (logger.isWarnEnabled()) { - logger.warn("Path contains \"../\" after call to StringUtils#cleanPath: [" + path + "]"); + logger.warn(LogFormatUtils.formatValue( + "Path contains \"../\" after call to StringUtils#cleanPath: [" + path + "]", -1, true)); } return true; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java index 3ee0c1f8131..74c20be91df 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2021 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. @@ -67,9 +67,8 @@ public class ExtendedServletRequestDataBinder extends ServletRequestDataBinder { if (uriVars != null) { uriVars.forEach((name, value) -> { if (mpvs.contains(name)) { - if (logger.isWarnEnabled()) { - logger.warn("Skipping URI variable '" + name + - "' because request contains bind value with same name."); + if (logger.isDebugEnabled()) { + logger.debug("URI variable '" + name + "' overridden by request bind value."); } } else { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java index 0bc88bfca2c..d51b48daa07 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -193,7 +193,7 @@ class ReactiveTypeHandler { "-------------------------------\n" + "Controller:\t" + returnType.getContainingClass().getName() + "\n" + "Method:\t\t" + returnType.getMethod().getName() + "\n" + - "Returning:\t" + ResolvableType.forMethodParameter(returnType).toString() + "\n" + + "Returning:\t" + ResolvableType.forMethodParameter(returnType) + "\n" + "!!!"); this.taskExecutorWarning = false; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java index 82dea1581d6..b9aad827b1e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java @@ -33,6 +33,7 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.core.io.UrlResource; +import org.springframework.core.log.LogFormatUtils; import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; import org.springframework.web.context.support.ServletContextResource; @@ -188,11 +189,12 @@ public class PathResourceResolver extends AbstractResourceResolver { return resource; } else if (logger.isWarnEnabled()) { - Resource[] allowedLocations = getAllowedLocations(); - logger.warn("Resource path \"" + resourcePath + "\" was successfully resolved " + - "but resource \"" + resource.getURL() + "\" is neither under the " + - "current location \"" + location.getURL() + "\" nor under any of the " + - "allowed locations " + (allowedLocations != null ? Arrays.asList(allowedLocations) : "[]")); + Resource[] allowed = getAllowedLocations(); + logger.warn(LogFormatUtils.formatValue( + "Resource path \"" + resourcePath + "\" was successfully resolved " + + "but resource \"" + resource.getURL() + "\" is neither under " + + "the current location \"" + location.getURL() + "\" nor under any of " + + "the allowed locations " + (allowed != null ? Arrays.asList(allowed) : "[]"), -1, true)); } } return null; @@ -285,7 +287,8 @@ public class PathResourceResolver extends AbstractResourceResolver { try { String decodedPath = URLDecoder.decode(resourcePath, "UTF-8"); if (decodedPath.contains("../") || decodedPath.contains("..\\")) { - logger.warn("Resolved resource path contains encoded \"../\" or \"..\\\": " + resourcePath); + logger.warn(LogFormatUtils.formatValue( + "Resolved resource path contains encoded \"../\" or \"..\\\": " + resourcePath, -1, true)); return true; } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index dbd56fffa6c..41c21568e80 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -40,6 +40,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.EmbeddedValueResolverAware; import org.springframework.core.io.Resource; import org.springframework.core.io.UrlResource; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpRange; @@ -662,7 +663,8 @@ public class ResourceHttpRequestHandler extends WebContentGenerator protected boolean isInvalidPath(String path) { if (path.contains("WEB-INF") || path.contains("META-INF")) { if (logger.isWarnEnabled()) { - logger.warn("Path with \"WEB-INF\" or \"META-INF\": [" + path + "]"); + logger.warn(LogFormatUtils.formatValue( + "Path with \"WEB-INF\" or \"META-INF\": [" + path + "]", -1, true)); } return true; } @@ -670,14 +672,16 @@ public class ResourceHttpRequestHandler extends WebContentGenerator String relativePath = (path.charAt(0) == '/' ? path.substring(1) : path); if (ResourceUtils.isUrl(relativePath) || relativePath.startsWith("url:")) { if (logger.isWarnEnabled()) { - logger.warn("Path represents URL or has \"url:\" prefix: [" + path + "]"); + logger.warn(LogFormatUtils.formatValue( + "Path represents URL or has \"url:\" prefix: [" + path + "]", -1, true)); } return true; } } if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) { if (logger.isWarnEnabled()) { - logger.warn("Path contains \"../\" after call to StringUtils#cleanPath: [" + path + "]"); + logger.warn(LogFormatUtils.formatValue( + "Path contains \"../\" after call to StringUtils#cleanPath: [" + path + "]", -1, true)); } return true; } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/AbstractHandshakeHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/AbstractHandshakeHandler.java index 47384d84b49..6c031c1e1fb 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/AbstractHandshakeHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/AbstractHandshakeHandler.java @@ -29,6 +29,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.context.Lifecycle; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.server.ServerHttpRequest; @@ -293,7 +294,8 @@ public abstract class AbstractHandshakeHandler implements HandshakeHandler, Life protected void handleInvalidUpgradeHeader(ServerHttpRequest request, ServerHttpResponse response) throws IOException { if (logger.isErrorEnabled()) { - logger.error("Handshake failed due to invalid Upgrade header: " + request.getHeaders().getUpgrade()); + logger.error(LogFormatUtils.formatValue( + "Handshake failed due to invalid Upgrade header: " + request.getHeaders().getUpgrade(), -1, true)); } response.setStatusCode(HttpStatus.BAD_REQUEST); response.getBody().write("Can \"Upgrade\" only to \"WebSocket\".".getBytes(StandardCharsets.UTF_8)); @@ -301,7 +303,8 @@ public abstract class AbstractHandshakeHandler implements HandshakeHandler, Life protected void handleInvalidConnectHeader(ServerHttpRequest request, ServerHttpResponse response) throws IOException { if (logger.isErrorEnabled()) { - logger.error("Handshake failed due to invalid Connection header " + request.getHeaders().getConnection()); + logger.error(LogFormatUtils.formatValue( + "Handshake failed due to invalid Connection header" + request.getHeaders().getConnection(), -1, true)); } response.setStatusCode(HttpStatus.BAD_REQUEST); response.getBody().write("\"Connection\" must be \"upgrade\".".getBytes(StandardCharsets.UTF_8)); @@ -325,8 +328,9 @@ public abstract class AbstractHandshakeHandler implements HandshakeHandler, Life protected void handleWebSocketVersionNotSupported(ServerHttpRequest request, ServerHttpResponse response) { if (logger.isErrorEnabled()) { String version = request.getHeaders().getFirst("Sec-WebSocket-Version"); - logger.error("Handshake failed due to unsupported WebSocket version: " + version + - ". Supported versions: " + Arrays.toString(getSupportedVersions())); + logger.error(LogFormatUtils.formatValue( + "Handshake failed due to unsupported WebSocket version: " + version + + ". Supported versions: " + Arrays.toString(getSupportedVersions()), -1, true)); } response.setStatusCode(HttpStatus.UPGRADE_REQUIRED); response.getHeaders().set(WebSocketHttpHeaders.SEC_WEBSOCKET_VERSION, diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java index bd72610b604..7848872d191 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java @@ -34,6 +34,7 @@ import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; @@ -340,7 +341,8 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig if (sockJsPath == null) { if (logger.isWarnEnabled()) { - logger.warn("Expected SockJS path. Failing request: " + request.getURI()); + logger.warn(LogFormatUtils.formatValue( + "Expected SockJS path. Failing request: " + request.getURI(), -1, true)); } response.setStatusCode(HttpStatus.NOT_FOUND); return; @@ -405,7 +407,8 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig String[] pathSegments = StringUtils.tokenizeToStringArray(sockJsPath.substring(1), "/"); if (pathSegments.length != 3) { if (logger.isWarnEnabled()) { - logger.warn("Invalid SockJS path '" + sockJsPath + "' - required to have 3 path segments"); + logger.warn(LogFormatUtils.formatValue("Invalid SockJS path '" + sockJsPath + "' - " + + "required to have 3 path segments", -1, true)); } if (requestInfo != null) { logger.debug("Ignoring transport request: " + requestInfo); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java index 272700b49a4..1d4a6c9b5f9 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -30,6 +30,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; import org.springframework.context.Lifecycle; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.server.ServerHttpRequest; @@ -234,7 +235,7 @@ public class TransportHandlingSockJsService extends AbstractSockJsService implem TransportType transportType = TransportType.fromValue(transport); if (transportType == null) { if (logger.isWarnEnabled()) { - logger.warn("Unknown transport type for " + request.getURI()); + logger.warn(LogFormatUtils.formatValue("Unknown transport type for " + request.getURI(), -1, true)); } response.setStatusCode(HttpStatus.NOT_FOUND); return; @@ -243,7 +244,7 @@ public class TransportHandlingSockJsService extends AbstractSockJsService implem TransportHandler transportHandler = this.handlers.get(transportType); if (transportHandler == null) { if (logger.isWarnEnabled()) { - logger.warn("No TransportHandler for " + request.getURI()); + logger.warn(LogFormatUtils.formatValue("No TransportHandler for " + request.getURI(), -1, true)); } response.setStatusCode(HttpStatus.NOT_FOUND); return;