diff --git a/spring-core/src/main/java/org/springframework/core/codec/AbstractDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/AbstractDecoder.java index 638a7c48d90..d4edcb5be1a 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/AbstractDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/AbstractDecoder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -20,6 +20,8 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; import reactor.core.publisher.Mono; @@ -37,6 +39,8 @@ import org.springframework.util.MimeType; */ public abstract class AbstractDecoder implements Decoder { + protected final Log logger = LogFactory.getLog(getClass()); + private final List decodableMimeTypes; diff --git a/spring-core/src/main/java/org/springframework/core/codec/AbstractEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/AbstractEncoder.java index 10242e9a701..01d1fccfea2 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/AbstractEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/AbstractEncoder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -18,6 +18,10 @@ package org.springframework.core.codec; import java.util.Arrays; import java.util.List; +import java.util.Map; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.core.ResolvableType; import org.springframework.lang.Nullable; @@ -32,6 +36,8 @@ import org.springframework.util.MimeType; */ public abstract class AbstractEncoder implements Encoder { + protected final Log logger = LogFactory.getLog(getClass()); + private final List encodableMimeTypes; @@ -53,4 +59,17 @@ public abstract class AbstractEncoder implements Encoder { return this.encodableMimeTypes.stream().anyMatch(candidate -> candidate.isCompatibleWith(mimeType)); } + /** + * Helper method to obtain the logger to use from the Map of hints, or fall + * back on the default logger. This may be used for example to override + * logging, e.g. for a multipart request where the full map of part values + * has already been logged. + * @param hints the hints passed to the encode method + * @return the logger to use + * @since 5.1 + */ + protected Log getLogger(@Nullable Map hints) { + return hints != null ? ((Log) hints.getOrDefault(Log.class.getName(), logger)) : logger; + } + } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java index cf8a34c1056..ce013679d76 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayDecoder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -53,6 +53,9 @@ public class ByteArrayDecoder extends AbstractDataBufferDecoder { byte[] result = new byte[dataBuffer.readableByteCount()]; dataBuffer.read(result); DataBufferUtils.release(dataBuffer); + if (logger.isDebugEnabled()) { + logger.debug("Read " + result.length + " bytes"); + } return result; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayEncoder.java index 395e54c06eb..33672fb399d 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteArrayEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteArrayEncoder.java @@ -18,6 +18,7 @@ package org.springframework.core.codec; import java.util.Map; +import org.apache.commons.logging.Log; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; @@ -52,7 +53,14 @@ public class ByteArrayEncoder extends AbstractEncoder { DataBufferFactory bufferFactory, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { - return Flux.from(inputStream).map(bufferFactory::wrap); + return Flux.from(inputStream).map(bytes -> { + DataBuffer dataBuffer = bufferFactory.wrap(bytes); + Log logger = getLogger(hints); + if (logger.isDebugEnabled()) { + logger.debug("Writing " + dataBuffer.readableByteCount() + " bytes"); + } + return dataBuffer; + }); } } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java index df4a642f540..147bf10f55e 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferDecoder.java @@ -52,10 +52,14 @@ public class ByteBufferDecoder extends AbstractDataBufferDecoder { protected ByteBuffer decodeDataBuffer(DataBuffer dataBuffer, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { - ByteBuffer copy = ByteBuffer.allocate(dataBuffer.readableByteCount()); + int byteCount = dataBuffer.readableByteCount(); + ByteBuffer copy = ByteBuffer.allocate(byteCount); copy.put(dataBuffer.asByteBuffer()); copy.flip(); DataBufferUtils.release(dataBuffer); + if (logger.isDebugEnabled()) { + logger.debug("Read " + byteCount + " bytes"); + } return copy; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferEncoder.java index e1332fd8434..93744aafeec 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ByteBufferEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ByteBufferEncoder.java @@ -19,6 +19,7 @@ package org.springframework.core.codec; import java.nio.ByteBuffer; import java.util.Map; +import org.apache.commons.logging.Log; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; @@ -53,7 +54,14 @@ public class ByteBufferEncoder extends AbstractEncoder { DataBufferFactory bufferFactory, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { - return Flux.from(inputStream).map(bufferFactory::wrap); + return Flux.from(inputStream).map(byteBuffer -> { + DataBuffer dataBuffer = bufferFactory.wrap(byteBuffer); + Log logger = getLogger(hints); + if (logger.isDebugEnabled()) { + logger.debug("Writing " + dataBuffer.readableByteCount() + " bytes"); + } + return dataBuffer; + }); } } diff --git a/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java index 5618dff729e..abfa5552f78 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java @@ -22,6 +22,7 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Map; +import org.apache.commons.logging.Log; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; @@ -65,6 +66,10 @@ public class CharSequenceEncoder extends AbstractEncoder { Charset charset = getCharset(mimeType); return Flux.from(inputStream).map(charSequence -> { + Log logger = getLogger(hints); + if (logger.isDebugEnabled()) { + logger.debug("Writing '" + charSequence + "'"); + } CharBuffer charBuffer = CharBuffer.wrap(charSequence); ByteBuffer byteBuffer = charset.encode(charBuffer); return bufferFactory.wrap(byteBuffer); diff --git a/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java index 25e2d5f482c..24bd9eaeb44 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/DataBufferDecoder.java @@ -62,6 +62,9 @@ public class DataBufferDecoder extends AbstractDataBufferDecoder { protected DataBuffer decodeDataBuffer(DataBuffer buffer, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { + if (logger.isDebugEnabled()) { + logger.debug("Read " + buffer.readableByteCount() + " bytes"); + } return buffer; } diff --git a/spring-core/src/main/java/org/springframework/core/codec/DataBufferEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/DataBufferEncoder.java index af4f9364b9e..c2d3d0d7534 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/DataBufferEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/DataBufferEncoder.java @@ -18,6 +18,7 @@ package org.springframework.core.codec; import java.util.Map; +import org.apache.commons.logging.Log; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; @@ -52,7 +53,14 @@ public class DataBufferEncoder extends AbstractEncoder { DataBufferFactory bufferFactory, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { - return Flux.from(inputStream); + Flux flux = Flux.from(inputStream); + + Log logger = getLogger(hints); + if (logger.isDebugEnabled()) { + flux = flux.doOnNext(buffer -> logger.debug("Writing " + buffer.readableByteCount() + " bytes")); + } + + return flux; } } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java index c158204c1bb..ccd54d4ff51 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ResourceDecoder.java @@ -72,6 +72,10 @@ public class ResourceDecoder extends AbstractDataBufferDecoder { Class clazz = elementType.getRawClass(); Assert.state(clazz != null, "No resource class"); + if (logger.isDebugEnabled()) { + logger.debug("Read " + bytes.length + " bytes"); + } + if (InputStreamResource.class == clazz) { return new InputStreamResource(new ByteArrayInputStream(bytes)); } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ResourceEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/ResourceEncoder.java index f60e0f25e0d..89a3b395047 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ResourceEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ResourceEncoder.java @@ -18,6 +18,7 @@ package org.springframework.core.codec; import java.util.Map; +import org.apache.commons.logging.Log; import reactor.core.publisher.Flux; import org.springframework.core.ResolvableType; @@ -65,6 +66,11 @@ public class ResourceEncoder extends AbstractSingleValueEncoder { protected Flux encode(Resource resource, DataBufferFactory dataBufferFactory, ResolvableType type, @Nullable MimeType mimeType, @Nullable Map hints) { + Log logger = getLogger(hints); + if (logger.isDebugEnabled()) { + logger.debug("Writing [" + resource + "]"); + } + return DataBufferUtils.read(resource, dataBufferFactory, this.bufferSize); } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java index 379b31c4a07..2a2a53c0c02 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java @@ -22,6 +22,7 @@ import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.OptionalLong; +import org.apache.commons.logging.Log; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -81,7 +82,7 @@ public class ResourceRegionEncoder extends AbstractEncoder { if (inputStream instanceof Mono) { return ((Mono) inputStream) - .flatMapMany(region -> writeResourceRegion(region, bufferFactory)); + .flatMapMany(region -> writeResourceRegion(region, bufferFactory, hints)); } else { Assert.notNull(hints, "'hints' must not be null"); @@ -96,7 +97,7 @@ public class ResourceRegionEncoder extends AbstractEncoder { concatMap(region -> Flux.concat( getRegionPrefix(bufferFactory, startBoundary, contentType, region), - writeResourceRegion(region, bufferFactory) + writeResourceRegion(region, bufferFactory, hints) )); return Flux.concat(regions, getRegionSuffix(bufferFactory, boundaryString)); } @@ -112,11 +113,20 @@ public class ResourceRegionEncoder extends AbstractEncoder { ); } - private Flux writeResourceRegion(ResourceRegion region, DataBufferFactory bufferFactory) { + private Flux writeResourceRegion( + ResourceRegion region, DataBufferFactory bufferFactory, @Nullable Map hints) { + Resource resource = region.getResource(); long position = region.getPosition(); + long count = region.getCount(); + + Log logger = getLogger(hints); + if (logger.isDebugEnabled()) { + logger.debug("Writing region " + position + "-" + (position + count) + " of [" + resource + "]"); + } + Flux in = DataBufferUtils.read(resource, position, bufferFactory, this.bufferSize); - return DataBufferUtils.takeUntilByteCount(in, region.getCount()); + return DataBufferUtils.takeUntilByteCount(in, count); } private Flux getRegionSuffix(DataBufferFactory bufferFactory, String boundaryString) { diff --git a/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java index 6b21dbc2904..97efd6984e0 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java @@ -205,7 +205,11 @@ public class StringDecoder extends AbstractDataBufferDecoder { Charset charset = getCharset(mimeType); CharBuffer charBuffer = charset.decode(dataBuffer.asByteBuffer()); DataBufferUtils.release(dataBuffer); - return charBuffer.toString(); + String value = charBuffer.toString(); + if (logger.isDebugEnabled()) { + logger.debug("Decoded '" + "'"); + } + return value; } private static Charset getCharset(@Nullable MimeType mimeType) { diff --git a/spring-web/src/main/java/org/springframework/http/client/support/HttpAccessor.java b/spring-web/src/main/java/org/springframework/http/client/support/HttpAccessor.java index 3a3d02cc9c1..abfedea35b0 100644 --- a/spring-web/src/main/java/org/springframework/http/client/support/HttpAccessor.java +++ b/spring-web/src/main/java/org/springframework/http/client/support/HttpAccessor.java @@ -86,7 +86,7 @@ public abstract class HttpAccessor { protected ClientHttpRequest createRequest(URI url, HttpMethod method) throws IOException { ClientHttpRequest request = getRequestFactory().createRequest(url, method); if (logger.isDebugEnabled()) { - logger.debug("Created " + method.name() + " request for \"" + url + "\""); + logger.debug("HTTP " + method.name() + " " + url); } return request; } diff --git a/spring-web/src/main/java/org/springframework/http/codec/CodecConfigurer.java b/spring-web/src/main/java/org/springframework/http/codec/CodecConfigurer.java index 214319c0a2e..1d1a30a1af6 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/CodecConfigurer.java +++ b/spring-web/src/main/java/org/springframework/http/codec/CodecConfigurer.java @@ -109,6 +109,21 @@ public interface CodecConfigurer { * @see org.springframework.http.codec.json.Jackson2JsonEncoder */ void jackson2JsonEncoder(Encoder encoder); + + /** + * Whether to disable logging of request details for form and multipart + * requests at any log level. By default such data is logged under + * {@code "org.springframework.http.codec"} but may contain sensitive + * information. Typically that's not an issue since DEBUG is used in + * development, but this option may be used to explicitly disable any + * logging of form and multipart data at any log level. + *

By default this is set to {@code false} in which case form and + * multipart data is logged at DEBUG or TRACE. When set to {@code true} + * values will not be logged at any level. + * @param disableLoggingRequestDetails whether to disable loggins + * @since 5.1 + */ + void disableLoggingRequestDetails(boolean disableLoggingRequestDetails); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageReader.java index 8611d2fb09e..597c18ac3d7 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageReader.java +++ b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageReader.java @@ -25,6 +25,8 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -46,13 +48,15 @@ import org.springframework.util.StringUtils; * @author Rossen Stoyanchev * @since 5.0 */ -public class FormHttpMessageReader implements HttpMessageReader> { +public class FormHttpMessageReader extends LoggingCodecSupport + implements HttpMessageReader> { public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; private static final ResolvableType MULTIVALUE_TYPE = ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class); + private Charset defaultCharset = DEFAULT_CHARSET; @@ -101,7 +105,11 @@ public class FormHttpMessageReader implements HttpMessageReader formData = parseFormData(charset, body); + if (shouldLogRequestDetails()) { + logger.debug("Decoded " + formData); + } + return formData; }); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java index 35ee06d4db4..f3e294cc03f 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java @@ -22,10 +22,11 @@ import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Collections; -import java.util.Iterator; import java.util.List; import java.util.Map; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; import reactor.core.publisher.Mono; @@ -57,7 +58,8 @@ import org.springframework.util.MultiValueMap; * @since 5.0 * @see org.springframework.http.codec.multipart.MultipartHttpMessageWriter */ -public class FormHttpMessageWriter implements HttpMessageWriter> { +public class FormHttpMessageWriter extends LoggingCodecSupport + implements HttpMessageWriter> { public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; @@ -127,12 +129,15 @@ public class FormHttpMessageWriter implements HttpMessageWriter { - String value = serializeForm(form, charset); - ByteBuffer byteBuffer = charset.encode(value); - DataBuffer buffer = message.bufferFactory().wrap(byteBuffer); - message.getHeaders().setContentLength(byteBuffer.remaining()); - return message.writeWith(Mono.just(buffer)); - }); + if (shouldLogRequestDetails()) { + logger.debug("Encoding " + form); + } + String value = serializeForm(form, charset); + ByteBuffer byteBuffer = charset.encode(value); + DataBuffer buffer = message.bufferFactory().wrap(byteBuffer); + message.getHeaders().setContentLength(byteBuffer.remaining()); + return message.writeWith(Mono.just(buffer)); + }); } private MediaType getMediaType(@Nullable MediaType mediaType) { diff --git a/spring-web/src/main/java/org/springframework/http/codec/LoggingCodecSupport.java b/spring-web/src/main/java/org/springframework/http/codec/LoggingCodecSupport.java new file mode 100644 index 00000000000..480e28f9698 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/codec/LoggingCodecSupport.java @@ -0,0 +1,71 @@ +/* + * Copyright 2002-2018 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.http.codec; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +/** + * Base class for {@link org.springframework.core.codec.Encoder}, + * {@link org.springframework.core.codec.Decoder}, {@link HttpMessageReader}, or + * {@link HttpMessageWriter} that uses a logger and shows potentially sensitive + * request data. + * + * @author Rossen Stoyanchev + * @since 5.1 + */ +public class LoggingCodecSupport { + + protected final Log logger = LogFactory.getLog(getClass()); + + /** Do not log potentially sensitive information (params at DEBUG and headers at TRACE). */ + private boolean disableLoggingRequestDetails = false; + + + /** + * Whether to disable any logging of request details by this codec. + * By default values being encoded or decoded are logged at DEBUG and TRACE + * level under {@code "org.springframework.http.codec"} which may show + * sensitive data for form and multipart data. Typically that's not an issue + * since DEBUG and TRACE are intended for development, but this property may + * be used to explicitly disable any logging of such information regardless + * of the log level. + *

By default this is set to {@code false} in which case values encoded + * or decoded are logged at DEBUG level. When set to {@code true} values + * will not be logged at any level. + * @param disableLoggingRequestDetails whether to disable + */ + public void setDisableLoggingRequestDetails(boolean disableLoggingRequestDetails) { + this.disableLoggingRequestDetails = disableLoggingRequestDetails; + } + + /** + * Whether any logging of values being encoded or decoded is explicitly + * disabled regardless of log level. + */ + public boolean isDisableLoggingRequestDetails() { + return this.disableLoggingRequestDetails; + } + + /** + * Returns "true" if logger is at DEBUG level and the logging of values + * being encoded or decoded is not explicitly disabled. + */ + protected boolean shouldLogRequestDetails() { + return !this.disableLoggingRequestDetails && logger.isDebugEnabled(); + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java index caf515bc149..0656bfdc8f3 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java @@ -23,6 +23,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -72,6 +74,8 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { private static final ResolvableType REGION_TYPE = ResolvableType.forClass(ResourceRegion.class); + private static final Log logger = LogFactory.getLog(ResourceHttpMessageWriter.class); + private final ResourceEncoder encoder; @@ -139,7 +143,11 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { if (mediaType != null && mediaType.isConcrete() && !mediaType.equals(MediaType.APPLICATION_OCTET_STREAM)) { return mediaType; } - return MediaTypeFactory.getMediaType(resource).orElse(MediaType.APPLICATION_OCTET_STREAM); + mediaType = MediaTypeFactory.getMediaType(resource).orElse(MediaType.APPLICATION_OCTET_STREAM); + if (logger.isDebugEnabled()) { + logger.debug("Resource associated with '" + mediaType + "'"); + } + return mediaType; } private static long lengthOf(Resource resource) { @@ -162,6 +170,10 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { File file = resource.getFile(); long pos = region != null ? region.getPosition() : 0; long count = region != null ? region.getCount() : file.length(); + if (logger.isDebugEnabled()) { + String formatted = region != null ? "region " + pos + "-" + (count) + " of " : ""; + logger.debug("Zero-copy " + formatted + "[" + resource + "]"); + } return Optional.of(((ZeroCopyHttpOutputMessage) message).writeWith(file, pos, count)); } catch (IOException ex) { diff --git a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java index 0acdbdedb94..d4bc4a2ddc3 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java @@ -110,7 +110,11 @@ public abstract class AbstractJackson2Decoder extends Jackson2CodecSupport imple return tokens.map(tokenBuffer -> { try { - return reader.readValue(tokenBuffer.asParser(getObjectMapper())); + Object value = reader.readValue(tokenBuffer.asParser(getObjectMapper())); + if (logger.isDebugEnabled()) { + logger.debug("Decoded [" + value + "]"); + } + return value; } catch (InvalidDefinitionException ex) { throw new CodecException("Type definition error: " + ex.getType(), ex); diff --git a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java index 89062082a50..3d8ef96b84b 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java @@ -33,6 +33,7 @@ import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectWriter; import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; +import org.apache.commons.logging.Log; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -140,6 +141,11 @@ public abstract class AbstractJackson2Encoder extends Jackson2CodecSupport imple private DataBuffer encodeValue(Object value, @Nullable MimeType mimeType, DataBufferFactory bufferFactory, ResolvableType elementType, @Nullable Map hints, JsonEncoding encoding) { + Log logger = getLogger(hints); + if (logger.isDebugEnabled()) { + logger.debug("Encoding [" + value + "]"); + } + JavaType javaType = getJavaType(elementType.getType(), null); Class jsonView = (hints != null ? (Class) hints.get(Jackson2CodecSupport.JSON_VIEW_HINT) : null); ObjectWriter writer = (jsonView != null ? diff --git a/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java b/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java index d732b9e1855..7f7af24a4c2 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java +++ b/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java @@ -28,6 +28,8 @@ import com.fasterxml.jackson.annotation.JsonView; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.type.TypeFactory; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.core.GenericTypeResolver; import org.springframework.core.MethodParameter; @@ -62,6 +64,8 @@ public abstract class Jackson2CodecSupport { new MimeType("application", "*+json", StandardCharsets.UTF_8))); + protected final Log logger = LogFactory.getLog(getClass()); + private final ObjectMapper objectMapper; private final List mimeTypes; @@ -94,6 +98,19 @@ public abstract class Jackson2CodecSupport { return (mimeType == null || this.mimeTypes.stream().anyMatch(m -> m.isCompatibleWith(mimeType))); } + /** + * Helper method to obtain the logger to use from the Map of hints, or fall + * back on the default logger. This may be used for example to override + * logging, e.g. for a multipart request where the full map of part values + * has already been logged. + * @param hints the hints passed to the encode method + * @return the logger to use + * @since 5.1 + */ + protected Log getLogger(@Nullable Map hints) { + return hints != null ? ((Log) hints.getOrDefault(Log.class.getName(), logger)) : logger; + } + protected JavaType getJavaType(Type type, @Nullable Class contextClass) { TypeFactory typeFactory = this.objectMapper.getTypeFactory(); return typeFactory.constructType(GenericTypeResolver.resolveType(type, contextClass)); diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java index bbc08b3c68a..8932e7f93d4 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java @@ -29,6 +29,8 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import java.util.stream.Collectors; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.impl.NoOpLog; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -48,6 +50,7 @@ import org.springframework.http.client.MultipartBodyBuilder; import org.springframework.http.codec.EncoderHttpMessageWriter; import org.springframework.http.codec.FormHttpMessageWriter; import org.springframework.http.codec.HttpMessageWriter; +import org.springframework.http.codec.LoggingCodecSupport; import org.springframework.http.codec.ResourceHttpMessageWriter; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -75,10 +78,15 @@ import org.springframework.util.MultiValueMap; * @since 5.0 * @see FormHttpMessageWriter */ -public class MultipartHttpMessageWriter implements HttpMessageWriter> { +public class MultipartHttpMessageWriter extends LoggingCodecSupport + implements HttpMessageWriter> { public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; + /** Suppress logging from individual part writers (full map logged at this level) */ + private static final Map DEFAULT_HINTS = + Collections.singletonMap(Log.class.getName(), new NoOpLog()); + private final List> partWriters; @@ -214,6 +222,10 @@ public class MultipartHttpMessageWriter implements HttpMessageWriter body = Flux.fromIterable(map.entrySet()) .concatMap(entry -> encodePartValues(boundary, entry.getKey(), entry.getValue())) .concatWith(Mono.just(generateLastLine(boundary))); @@ -291,7 +303,7 @@ public class MultipartHttpMessageWriter implements HttpMessageWriter partContentReady = ((HttpMessageWriter) writer.get()) - .write(bodyPublisher, resolvableType, contentType, outputMessage, Collections.emptyMap()); + .write(bodyPublisher, resolvableType, contentType, outputMessage, DEFAULT_HINTS); // After partContentReady, we can access the part content from MultipartHttpOutputMessage // and use it for writing to the actual request body diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java index 4e4974b194f..8982529678e 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java @@ -32,6 +32,8 @@ import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.synchronoss.cloud.nio.multipart.DefaultPartBodyStreamStorageFactory; import org.synchronoss.cloud.nio.multipart.Multipart; import org.synchronoss.cloud.nio.multipart.MultipartContext; @@ -53,6 +55,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.ReactiveHttpInputMessage; import org.springframework.http.codec.HttpMessageReader; +import org.springframework.http.codec.LoggingCodecSupport; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -70,7 +73,7 @@ import org.springframework.util.Assert; * @see Synchronoss NIO Multipart * @see MultipartHttpMessageReader */ -public class SynchronossPartHttpMessageReader implements HttpMessageReader { +public class SynchronossPartHttpMessageReader extends LoggingCodecSupport implements HttpMessageReader { private final DataBufferFactory bufferFactory = new DefaultDataBufferFactory(); @@ -91,7 +94,12 @@ public class SynchronossPartHttpMessageReader implements HttpMessageReader @Override public Flux read(ResolvableType elementType, ReactiveHttpInputMessage message, Map hints) { - return Flux.create(new SynchronossPartGenerator(message, this.bufferFactory, this.streamStorageFactory)); + return Flux.create(new SynchronossPartGenerator(message, this.bufferFactory, this.streamStorageFactory)) + .doOnNext(part -> { + if (shouldLogRequestDetails()) { + logger.debug("Decoded [" + part + "]"); + } + }); } @@ -263,6 +271,11 @@ public class SynchronossPartHttpMessageReader implements HttpMessageReader DataBufferFactory getBufferFactory() { return this.bufferFactory; } + + @Override + public String toString() { + return "Part '" + this.name + "', headers=" + this.headers; + } } @@ -342,6 +355,11 @@ public class SynchronossPartHttpMessageReader implements HttpMessageReader } return Mono.empty(); } + + @Override + public String toString() { + return "Part '" + name() + "', filename='" + this.filename + "'"; + } } @@ -371,6 +389,11 @@ public class SynchronossPartHttpMessageReader implements HttpMessageReader String name = MultipartUtils.getCharEncoding(headers()); return (name != null ? Charset.forName(name) : StandardCharsets.UTF_8); } + + @Override + public String toString() { + return "Part '" + name() + "=" + this.content + "'"; + } } } diff --git a/spring-web/src/main/java/org/springframework/http/codec/support/BaseDefaultCodecs.java b/spring-web/src/main/java/org/springframework/http/codec/support/BaseDefaultCodecs.java index cdeb47548c9..c155b42e6ba 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/support/BaseDefaultCodecs.java +++ b/spring-web/src/main/java/org/springframework/http/codec/support/BaseDefaultCodecs.java @@ -74,6 +74,8 @@ class BaseDefaultCodecs implements CodecConfigurer.DefaultCodecs { @Nullable private Encoder jackson2JsonEncoder; + private boolean disableLoggingRequestDetails = false; + private boolean registerDefaults = true; @@ -87,6 +89,15 @@ class BaseDefaultCodecs implements CodecConfigurer.DefaultCodecs { this.jackson2JsonEncoder = encoder; } + @Override + public void disableLoggingRequestDetails(boolean disableLoggingRequestDetails) { + this.disableLoggingRequestDetails = disableLoggingRequestDetails; + } + + protected boolean isDisableLoggingRequestDetails() { + return this.disableLoggingRequestDetails; + } + /** * Delegate method used from {@link BaseCodecConfigurer#registerDefaults}. */ @@ -108,8 +119,13 @@ class BaseDefaultCodecs implements CodecConfigurer.DefaultCodecs { readers.add(new DecoderHttpMessageReader<>(new DataBufferDecoder())); readers.add(new DecoderHttpMessageReader<>(new ResourceDecoder())); readers.add(new DecoderHttpMessageReader<>(StringDecoder.textPlainOnly())); - readers.add(new FormHttpMessageReader()); + + FormHttpMessageReader formReader = new FormHttpMessageReader(); + formReader.setDisableLoggingRequestDetails(this.disableLoggingRequestDetails); + readers.add(formReader); + extendTypedReaders(readers); + return readers; } diff --git a/spring-web/src/main/java/org/springframework/http/codec/support/ClientDefaultCodecsImpl.java b/spring-web/src/main/java/org/springframework/http/codec/support/ClientDefaultCodecsImpl.java index 10a57151b87..3682be171bd 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/support/ClientDefaultCodecsImpl.java +++ b/spring-web/src/main/java/org/springframework/http/codec/support/ClientDefaultCodecsImpl.java @@ -86,7 +86,14 @@ class ClientDefaultCodecsImpl extends BaseDefaultCodecs implements ClientCodecCo @Override protected void extendTypedWriters(List> typedWriters) { - typedWriters.add(new MultipartHttpMessageWriter(getPartWriters(), new FormHttpMessageWriter())); + + FormHttpMessageWriter formWriter = new FormHttpMessageWriter(); + formWriter.setDisableLoggingRequestDetails(isDisableLoggingRequestDetails()); + + MultipartHttpMessageWriter multipartWriter = new MultipartHttpMessageWriter(getPartWriters(), formWriter); + multipartWriter.setDisableLoggingRequestDetails(isDisableLoggingRequestDetails()); + + typedWriters.add(multipartWriter); } private List> getPartWriters() { diff --git a/spring-web/src/main/java/org/springframework/http/codec/support/ServerDefaultCodecsImpl.java b/spring-web/src/main/java/org/springframework/http/codec/support/ServerDefaultCodecsImpl.java index ae43d3b295b..1f1958d59f5 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/support/ServerDefaultCodecsImpl.java +++ b/spring-web/src/main/java/org/springframework/http/codec/support/ServerDefaultCodecsImpl.java @@ -51,6 +51,7 @@ class ServerDefaultCodecsImpl extends BaseDefaultCodecs implements ServerCodecCo protected void extendTypedReaders(List> typedReaders) { if (synchronossMultipartPresent) { SynchronossPartHttpMessageReader partReader = new SynchronossPartHttpMessageReader(); + partReader.setDisableLoggingRequestDetails(isDisableLoggingRequestDetails()); typedReaders.add(partReader); typedReaders.add(new MultipartHttpMessageReader(partReader)); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java index 18f3e6605b1..ebda58b74d3 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java @@ -104,7 +104,13 @@ public class Jaxb2XmlDecoder extends AbstractDecoder { QName typeName = toQName(outputClass); Flux> splitEvents = split(xmlEventFlux, typeName); - return splitEvents.map(events -> unmarshal(events, outputClass)); + return splitEvents.map(events -> { + Object value = unmarshal(events, outputClass); + if (logger.isDebugEnabled()) { + logger.debug("Decoded [" + value + "]"); + } + return value; + }); } @Override diff --git a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlEncoder.java b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlEncoder.java index e9867ee3e5b..025f292a8de 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlEncoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlEncoder.java @@ -25,6 +25,7 @@ import javax.xml.bind.Marshaller; import javax.xml.bind.annotation.XmlRootElement; import javax.xml.bind.annotation.XmlType; +import org.apache.commons.logging.Log; import reactor.core.publisher.Flux; import org.springframework.core.ResolvableType; @@ -73,6 +74,10 @@ public class Jaxb2XmlEncoder extends AbstractSingleValueEncoder { protected Flux encode(Object value, DataBufferFactory dataBufferFactory, ResolvableType type, @Nullable MimeType mimeType, @Nullable Map hints) { try { + Log logger = getLogger(hints); + if (logger.isDebugEnabled()) { + logger.debug("Encoding [" + value + "]"); + } DataBuffer buffer = dataBufferFactory.allocateBuffer(1024); OutputStream outputStream = buffer.asOutputStream(); Class clazz = ClassUtils.getUserClass(value); diff --git a/spring-web/src/main/java/org/springframework/http/server/DefaultPathContainer.java b/spring-web/src/main/java/org/springframework/http/server/DefaultPathContainer.java index e97418f8182..4cf47888fd7 100644 --- a/spring-web/src/main/java/org/springframework/http/server/DefaultPathContainer.java +++ b/spring-web/src/main/java/org/springframework/http/server/DefaultPathContainer.java @@ -85,7 +85,7 @@ class DefaultPathContainer implements PathContainer { @Override public String toString() { - return "[path='" + this.path + "\']"; + return value(); } diff --git a/spring-web/src/main/java/org/springframework/http/server/DefaultRequestPath.java b/spring-web/src/main/java/org/springframework/http/server/DefaultRequestPath.java index 0fa21e803ed..a8bd993e6b5 100644 --- a/spring-web/src/main/java/org/springframework/http/server/DefaultRequestPath.java +++ b/spring-web/src/main/java/org/springframework/http/server/DefaultRequestPath.java @@ -148,9 +148,7 @@ class DefaultRequestPath implements RequestPath { @Override public String toString() { - return "DefaultRequestPath[fullPath='" + this.fullPath + "', " + - "contextPath='" + this.contextPath.value() + "', " + - "pathWithinApplication='" + this.pathWithinApplication.value() + "']"; + return this.fullPath.toString(); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java index ec6d5aaf94d..4900f99ba7b 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java @@ -88,16 +88,15 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { } @Override - public boolean setStatusCode(@Nullable HttpStatus statusCode) { + public boolean setStatusCode(@Nullable HttpStatus status) { if (this.state.get() == State.COMMITTED) { if (logger.isTraceEnabled()) { - logger.trace("HTTP response already committed. " + - "Status not set to " + (statusCode != null ? statusCode.toString() : "null")); + logger.trace("Ignoring status " + status + ": response already committed"); } return false; } else { - this.statusCode = (statusCode != null ? statusCode.value() : null); + this.statusCode = (status != null ? status.value() : null); return true; } } @@ -203,9 +202,6 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { */ protected Mono doCommit(@Nullable Supplier> writeAction) { if (!this.state.compareAndSet(State.NEW, State.COMMITTING)) { - if (logger.isDebugEnabled()) { - logger.debug("Skipping doCommit (response already committed)."); - } return Mono.empty(); } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorHttpHandlerAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorHttpHandlerAdapter.java index c061a0e2c91..2ea3214f87f 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorHttpHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorHttpHandlerAdapter.java @@ -61,8 +61,8 @@ public class ReactorHttpHandlerAdapter implements BiFunction logger.warn("Handling completed with error: " + ex.getMessage())) - .doOnSuccess(aVoid -> logger.debug("Handling completed with success")); + .doOnError(ex -> logger.trace("Failed to complete: " + ex.getMessage())) + .doOnSuccess(aVoid -> logger.trace("Handling completed")); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java index e8e620c7616..ece7ee11431 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java @@ -141,7 +141,7 @@ public class ServletHttpHandlerAdapter implements Servlet { if (mapping.endsWith("/*")) { String path = mapping.substring(0, mapping.length() - 2); if (!path.isEmpty()) { - logger.info("Found Servlet mapping '" + path + "' for Servlet '" + name + "'"); + logger.info("Found servlet mapping prefix '" + path + "' for '" + name + "'"); } return path; } @@ -159,7 +159,7 @@ public class ServletHttpHandlerAdapter implements Servlet { if (DispatcherType.ASYNC.equals(request.getDispatcherType())) { Throwable ex = (Throwable) request.getAttribute(WRITE_ERROR_ATTRIBUTE_NAME); - throw new ServletException("Write publisher error", ex); + throw new ServletException("Failed to create response content", ex); } // Start async before Read/WriteListener registration @@ -171,8 +171,8 @@ public class ServletHttpHandlerAdapter implements Servlet { httpRequest = createRequest(((HttpServletRequest) request), asyncContext); } catch (URISyntaxException ex) { - if (logger.isWarnEnabled()) { - logger.warn("Invalid URL for incoming request: " + ex.getMessage()); + if (logger.isDebugEnabled()) { + logger.debug("Failed to get request URL: " + ex.getMessage()); } ((HttpServletResponse) response).setStatus(400); asyncContext.complete(); @@ -247,14 +247,15 @@ public class ServletHttpHandlerAdapter implements Servlet { @Override public void onTimeout(AsyncEvent event) { - logger.debug("Timeout notification from Servlet container"); + logger.debug("Timeout notification"); AsyncContext context = event.getAsyncContext(); runIfAsyncNotComplete(context, this.isCompleted, context::complete); } @Override public void onError(AsyncEvent event) { - logger.debug("Error notification from Servlet container"); + Throwable ex = event.getThrowable(); + logger.debug("Error notification: " + (ex != null ? ex : "")); AsyncContext context = event.getAsyncContext(); runIfAsyncNotComplete(context, this.isCompleted, context::complete); } @@ -294,16 +295,16 @@ public class ServletHttpHandlerAdapter implements Servlet { @Override public void onError(Throwable ex) { - logger.warn("Handling completed with error: " + ex.getMessage()); + logger.trace("Failed to complete: " + ex.getMessage()); runIfAsyncNotComplete(this.asyncContext, this.isCompleted, () -> { if (this.asyncContext.getResponse().isCommitted()) { - logger.debug("Dispatching into container to raise error"); + logger.trace("Dispatch to container, to raise the error on servlet thread"); this.asyncContext.getRequest().setAttribute(WRITE_ERROR_ATTRIBUTE_NAME, ex); this.asyncContext.dispatch(); } else { try { - logger.debug("Setting response status code to 500"); + logger.trace("Setting ServletResponse status to 500 Server Error"); this.asyncContext.getResponse().resetBuffer(); ((HttpServletResponse) this.asyncContext.getResponse()).setStatus(500); } @@ -316,7 +317,7 @@ public class ServletHttpHandlerAdapter implements Servlet { @Override public void onComplete() { - logger.debug("Handling completed with success"); + logger.trace("Handling completed"); runIfAsyncNotComplete(this.asyncContext, this.isCompleted, this.asyncContext::complete); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java index 26235a83d6e..4aa5b0865f0 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java @@ -204,7 +204,7 @@ class ServletServerHttpRequest extends AbstractServerHttpRequest { DataBuffer readFromInputStream() throws IOException { int read = this.request.getInputStream().read(this.buffer); if (logger.isTraceEnabled()) { - logger.trace("InputStream read returned " + read + (read != -1 ? " bytes" : "")); + logger.trace("InputStream.read returned " + read + (read != -1 ? " bytes" : "")); } if (read > 0) { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowHttpHandlerAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowHttpHandlerAdapter.java index 2ca55f0e046..f7b55aea0c4 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowHttpHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowHttpHandlerAdapter.java @@ -72,7 +72,7 @@ public class UndertowHttpHandlerAdapter implements io.undertow.server.HttpHandle } catch (URISyntaxException ex) { if (logger.isWarnEnabled()) { - logger.warn("Invalid URL for incoming request: " + ex.getMessage()); + logger.debug("Failed to get request URI: " + ex.getMessage()); } exchange.setStatusCode(400); return; @@ -108,7 +108,7 @@ public class UndertowHttpHandlerAdapter implements io.undertow.server.HttpHandle @Override public void onError(Throwable ex) { - logger.warn("Handling completed with error: " + ex.getMessage()); + logger.trace("Failed to complete: " + ex.getMessage()); if (this.exchange.isResponseStarted()) { try { logger.debug("Closing connection"); @@ -119,7 +119,7 @@ public class UndertowHttpHandlerAdapter implements io.undertow.server.HttpHandle } } else { - logger.debug("Setting response status code to 500"); + logger.debug("Setting HttpServerExchange status to 500 Server Error"); this.exchange.setStatusCode(500); this.exchange.endExchange(); } @@ -127,7 +127,7 @@ public class UndertowHttpHandlerAdapter implements io.undertow.server.HttpHandle @Override public void onComplete() { - logger.debug("Handling completed with success"); + logger.trace("Handling completed"); this.exchange.endExchange(); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java index 8ef9eb90c3f..220007b4234 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java @@ -174,7 +174,7 @@ class UndertowServerHttpRequest extends AbstractServerHttpRequest { int read = this.channel.read(byteBuffer); if (logger.isTraceEnabled()) { - logger.trace("Channel read returned " + read + (read != -1 ? " bytes" : "")); + logger.trace("Channel.read returned " + read + (read != -1 ? " bytes" : "")); } if (read > 0) { diff --git a/spring-web/src/main/java/org/springframework/web/client/HttpMessageConverterExtractor.java b/spring-web/src/main/java/org/springframework/web/client/HttpMessageConverterExtractor.java index 1315f67cd75..eded3e3b0e8 100644 --- a/spring-web/src/main/java/org/springframework/web/client/HttpMessageConverterExtractor.java +++ b/spring-web/src/main/java/org/springframework/web/client/HttpMessageConverterExtractor.java @@ -94,8 +94,7 @@ public class HttpMessageConverterExtractor implements ResponseExtractor { (GenericHttpMessageConverter) messageConverter; if (genericMessageConverter.canRead(this.responseType, null, contentType)) { if (logger.isDebugEnabled()) { - logger.debug("Reading [" + this.responseType + "] as \"" + - contentType + "\" using [" + messageConverter + "]"); + logger.debug("Reading [" + this.responseType + "]"); } return (T) genericMessageConverter.read(this.responseType, null, responseWrapper); } @@ -103,8 +102,8 @@ public class HttpMessageConverterExtractor implements ResponseExtractor { if (this.responseClass != null) { if (messageConverter.canRead(this.responseClass, contentType)) { if (logger.isDebugEnabled()) { - logger.debug("Reading [" + this.responseClass.getName() + "] as \"" + - contentType + "\" using [" + messageConverter + "]"); + String className = this.responseClass.getName(); + logger.debug("Reading [" + className + "] as \"" + contentType + "\""); } return (T) messageConverter.read((Class) this.responseClass, responseWrapper); } @@ -131,7 +130,7 @@ public class HttpMessageConverterExtractor implements ResponseExtractor { MediaType contentType = response.getHeaders().getContentType(); if (contentType == null) { if (logger.isTraceEnabled()) { - logger.trace("No Content-Type header found, defaulting to application/octet-stream"); + logger.trace("No content-type, using 'application/octet-stream'"); } contentType = MediaType.APPLICATION_OCTET_STREAM; } diff --git a/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java b/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java index 2aaa67fd158..ebabd48ebb6 100644 --- a/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java +++ b/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java @@ -32,6 +32,7 @@ import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.RequestEntity; import org.springframework.http.ResponseEntity; @@ -789,9 +790,9 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat boolean hasError = errorHandler.hasError(response); if (logger.isDebugEnabled()) { try { - logger.debug(method.name() + " request for \"" + url + "\" resulted in " + - response.getRawStatusCode() + " (" + response.getStatusText() + ")" + - (hasError ? "; invoking error handler" : "")); + int code = response.getRawStatusCode(); + HttpStatus status = HttpStatus.resolve(code); + logger.debug("Response " + (status != null ? status : code)); } catch (IOException ex) { // ignore @@ -873,7 +874,7 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat .sorted(MediaType.SPECIFICITY_COMPARATOR) .collect(Collectors.toList()); if (logger.isDebugEnabled()) { - logger.debug("Setting request Accept header to " + allSupportedMediaTypes); + logger.debug("Accept=" + allSupportedMediaTypes); } request.getHeaders().setAccept(allSupportedMediaTypes); } @@ -958,16 +959,7 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat if (!requestHeaders.isEmpty()) { requestHeaders.forEach((key, values) -> httpHeaders.put(key, new LinkedList<>(values))); } - if (logger.isDebugEnabled()) { - if (requestContentType != null) { - logger.debug("Writing [" + requestBody + "] as \"" + requestContentType + - "\" using [" + messageConverter + "]"); - } - else { - logger.debug("Writing [" + requestBody + "] using [" + messageConverter + "]"); - } - - } + logBody(requestBody, requestContentType, genericConverter); genericConverter.write(requestBody, requestBodyType, requestContentType, httpRequest); return; } @@ -976,29 +968,31 @@ public class RestTemplate extends InterceptingHttpAccessor implements RestOperat if (!requestHeaders.isEmpty()) { requestHeaders.forEach((key, values) -> httpHeaders.put(key, new LinkedList<>(values))); } - if (logger.isDebugEnabled()) { - if (requestContentType != null) { - logger.debug("Writing [" + requestBody + "] as \"" + requestContentType + - "\" using [" + messageConverter + "]"); - } - else { - logger.debug("Writing [" + requestBody + "] using [" + messageConverter + "]"); - } - - } + logBody(requestBody, requestContentType, messageConverter); ((HttpMessageConverter) messageConverter).write( requestBody, requestContentType, httpRequest); return; } } - String message = "Could not write request: no suitable HttpMessageConverter found for request type [" + - requestBodyClass.getName() + "]"; + String message = "No HttpMessageConverter for [" + requestBodyClass.getName() + "]"; if (requestContentType != null) { message += " and content type [" + requestContentType + "]"; } throw new RestClientException(message); } } + + private void logBody(Object body, @Nullable MediaType mediaType, HttpMessageConverter converter) { + if (logger.isDebugEnabled()) { + if (mediaType != null) { + logger.debug("Writing [" + body + "] as \"" + mediaType + "\""); + } + else { + String classname = converter.getClass().getName(); + logger.debug("Writing [" + body + "] with " + classname); + } + } + } } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java index 492d80cae23..6f225bf60e3 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java @@ -75,7 +75,9 @@ class CallableInterceptorChain { catch (Throwable ex) { // Save the first exception but invoke all interceptors if (exceptionResult != null) { - logger.warn("Unhandled error from interceptor postProcess method", ex); + if (logger.isTraceEnabled()) { + logger.trace("Ignoring failure in postProcess method", ex); + } } else { exceptionResult = ex; @@ -141,7 +143,9 @@ class CallableInterceptorChain { this.interceptors.get(i).afterCompletion(request, task); } catch (Throwable ex) { - logger.error("Unhandled error from interceptor afterCompletion method", ex); + if (logger.isTraceEnabled()) { + logger.trace("Ignoring failure in afterCompletion method", ex); + } } } } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java index 2b4551caae6..fee367c4d56 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java @@ -209,7 +209,7 @@ public class DeferredResult { resultHandler.handleResult(resultToHandle); } catch (Throwable ex) { - logger.debug("Failed to handle existing result", ex); + logger.debug("Failed to process async result", ex); } } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultInterceptorChain.java b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultInterceptorChain.java index 98bc5c1adb0..62b1048bcb0 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultInterceptorChain.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResultInterceptorChain.java @@ -106,7 +106,7 @@ class DeferredResultInterceptorChain { this.interceptors.get(i).afterCompletion(request, deferredResult); } catch (Throwable ex) { - logger.error("Unhandled error from interceptor afterCompletion method", ex); + logger.trace("Ignoring failure in afterCompletion method", ex); } } } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java index d0bdeb2ca3e..9f4af63486e 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java @@ -34,7 +34,6 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.web.context.request.RequestAttributes; import org.springframework.web.context.request.async.DeferredResult.DeferredResultHandler; -import org.springframework.web.util.UrlPathHelper; /** * The central class for managing asynchronous request processing, mainly intended @@ -64,8 +63,6 @@ public final class WebAsyncManager { private static final Log logger = LogFactory.getLog(WebAsyncManager.class); - private static final UrlPathHelper urlPathHelper = new UrlPathHelper(); - private static final CallableProcessingInterceptor timeoutCallableInterceptor = new TimeoutCallableProcessingInterceptor(); @@ -335,7 +332,7 @@ public final class WebAsyncManager { private String formatRequestUri() { HttpServletRequest request = this.asyncWebRequest.getNativeRequest(HttpServletRequest.class); - return request != null ? urlPathHelper.getRequestUri(request) : "servlet container"; + return request != null ? request.getRequestURI() : "servlet container"; } private void setConcurrentResultAndDispatch(Object result) { @@ -347,7 +344,9 @@ public final class WebAsyncManager { } if (this.asyncWebRequest.isAsyncComplete()) { - logger.error("Could not complete async processing due to timeout or network error"); + if (logger.isDebugEnabled()) { + logger.debug("Async request already complete for " + formatRequestUri()); + } return; } diff --git a/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java b/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java index ea7ddb07718..3426a859b40 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java @@ -68,13 +68,13 @@ public class DefaultCorsProcessor implements CorsProcessor { ServletServerHttpResponse serverResponse = new ServletServerHttpResponse(response); if (responseHasCors(serverResponse)) { - logger.trace("Skip CORS processing: response already contains \"Access-Control-Allow-Origin\" header"); + logger.trace("Skip: response already contains \"Access-Control-Allow-Origin\""); return true; } ServletServerHttpRequest serverRequest = new ServletServerHttpRequest(request); if (WebUtils.isSameOrigin(serverRequest)) { - logger.trace("Skip CORS processing: request is from same origin"); + logger.trace("Skip: request is from same origin"); return true; } @@ -126,7 +126,7 @@ public class DefaultCorsProcessor implements CorsProcessor { HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS)); if (allowOrigin == null) { - logger.debug("Rejecting CORS request because '" + requestOrigin + "' origin is not allowed"); + logger.debug("Reject: '" + requestOrigin + "' origin is not allowed"); rejectRequest(response); return false; } @@ -134,7 +134,7 @@ public class DefaultCorsProcessor implements CorsProcessor { HttpMethod requestMethod = getMethodToUse(request, preFlightRequest); List allowMethods = checkMethods(config, requestMethod); if (allowMethods == null) { - logger.debug("Rejecting CORS request because '" + requestMethod + "' request method is not allowed"); + logger.debug("Reject: HTTP '" + requestMethod + "' is not allowed"); rejectRequest(response); return false; } @@ -142,7 +142,7 @@ public class DefaultCorsProcessor implements CorsProcessor { List requestHeaders = getHeadersToUse(request, preFlightRequest); List allowHeaders = checkHeaders(config, requestHeaders); if (preFlightRequest && allowHeaders == null) { - logger.debug("Rejecting CORS request because '" + requestHeaders + "' request headers are not allowed"); + logger.debug("Reject: headers '" + requestHeaders + "' are not allowed"); rejectRequest(response); return false; } diff --git a/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java b/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java index 418d9acee06..b673e1e463a 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java @@ -63,12 +63,12 @@ public class DefaultCorsProcessor implements CorsProcessor { } if (responseHasCors(response)) { - logger.debug("Skip CORS: response already contains \"Access-Control-Allow-Origin\" header"); + logger.trace("Skip: response already contains \"Access-Control-Allow-Origin\""); return true; } if (CorsUtils.isSameOrigin(request)) { - logger.debug("Skip CORS: request is from same origin"); + logger.trace("Skip: request is from same origin"); return true; } @@ -95,7 +95,6 @@ public class DefaultCorsProcessor implements CorsProcessor { */ protected void rejectRequest(ServerHttpResponse response) { response.setStatusCode(HttpStatus.FORBIDDEN); - logger.debug("Invalid CORS request"); } /** @@ -114,7 +113,7 @@ public class DefaultCorsProcessor implements CorsProcessor { String requestOrigin = request.getHeaders().getOrigin(); String allowOrigin = checkOrigin(config, requestOrigin); if (allowOrigin == null) { - logger.debug("Rejecting CORS request because '" + requestOrigin + "' origin is not allowed"); + logger.debug("Reject: '" + requestOrigin + "' origin is not allowed"); rejectRequest(response); return false; } @@ -122,7 +121,7 @@ public class DefaultCorsProcessor implements CorsProcessor { HttpMethod requestMethod = getMethodToUse(request, preFlightRequest); List allowMethods = checkMethods(config, requestMethod); if (allowMethods == null) { - logger.debug("Rejecting CORS request because '" + requestMethod + "' request method is not allowed"); + logger.debug("Reject: HTTP '" + requestMethod + "' is not allowed"); rejectRequest(response); return false; } @@ -130,7 +129,7 @@ public class DefaultCorsProcessor implements CorsProcessor { List requestHeaders = getHeadersToUse(request, preFlightRequest); List allowHeaders = checkHeaders(config, requestHeaders); if (preFlightRequest && allowHeaders == null) { - logger.debug("Rejecting CORS request because '" + requestHeaders + "' request headers are not allowed"); + logger.debug("Reject: headers '" + requestHeaders + "' are not allowed"); rejectRequest(response); return false; } diff --git a/spring-web/src/main/java/org/springframework/web/filter/GenericFilterBean.java b/spring-web/src/main/java/org/springframework/web/filter/GenericFilterBean.java index c894145475c..97890931b91 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/GenericFilterBean.java +++ b/spring-web/src/main/java/org/springframework/web/filter/GenericFilterBean.java @@ -209,9 +209,6 @@ public abstract class GenericFilterBean implements Filter, BeanNameAware, Enviro @Override public final void init(FilterConfig filterConfig) throws ServletException { Assert.notNull(filterConfig, "FilterConfig must not be null"); - if (logger.isDebugEnabled()) { - logger.debug("Initializing filter '" + filterConfig.getFilterName() + "'"); - } this.filterConfig = filterConfig; @@ -241,7 +238,7 @@ public abstract class GenericFilterBean implements Filter, BeanNameAware, Enviro initFilterBean(); if (logger.isDebugEnabled()) { - logger.debug("Filter '" + filterConfig.getFilterName() + "' configured successfully"); + logger.debug("Filter '" + filterConfig.getFilterName() + "' configured for use"); } } diff --git a/spring-web/src/main/java/org/springframework/web/filter/RequestContextFilter.java b/spring-web/src/main/java/org/springframework/web/filter/RequestContextFilter.java index c5925e5e903..319774ae0a5 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/RequestContextFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/RequestContextFilter.java @@ -100,8 +100,8 @@ public class RequestContextFilter extends OncePerRequestFilter { } finally { resetContextHolders(); - if (logger.isDebugEnabled()) { - logger.debug("Cleared thread-bound request context: " + request); + if (logger.isTraceEnabled()) { + logger.trace("Cleared thread-bound request context: " + request); } attributes.requestCompleted(); } @@ -110,8 +110,8 @@ public class RequestContextFilter extends OncePerRequestFilter { private void initContextHolders(HttpServletRequest request, ServletRequestAttributes requestAttributes) { LocaleContextHolder.setLocale(request.getLocale(), this.threadContextInheritable); RequestContextHolder.setRequestAttributes(requestAttributes, this.threadContextInheritable); - if (logger.isDebugEnabled()) { - logger.debug("Bound request context to thread: " + request); + if (logger.isTraceEnabled()) { + logger.trace("Bound request context to thread: " + request); } } diff --git a/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java b/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java index 517c336e85b..efe12d77e8e 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java @@ -128,23 +128,13 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter { String requestETag = request.getHeader(HEADER_IF_NONE_MATCH); if (requestETag != null && ("*".equals(requestETag) || responseETag.equals(requestETag) || responseETag.replaceFirst("^W/", "").equals(requestETag.replaceFirst("^W/", "")))) { - if (logger.isTraceEnabled()) { - logger.trace("ETag [" + responseETag + "] equal to If-None-Match, sending 304"); - } rawResponse.setStatus(HttpServletResponse.SC_NOT_MODIFIED); } else { - if (logger.isTraceEnabled()) { - logger.trace("ETag [" + responseETag + "] not equal to If-None-Match [" + requestETag + - "], sending normal response"); - } responseWrapper.copyBodyToResponse(); } } else { - if (logger.isTraceEnabled()) { - logger.trace("Response with status code [" + statusCode + "] not eligible for ETag"); - } responseWrapper.copyBodyToResponse(); } } diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java index cd9c9d7409b..568a659caad 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java @@ -155,18 +155,11 @@ public final class ModelFactory { private ModelMethod getNextModelMethod(ModelAndViewContainer container) { for (ModelMethod modelMethod : this.modelMethods) { if (modelMethod.checkDependencies(container)) { - if (logger.isTraceEnabled()) { - logger.trace("Selected @ModelAttribute method " + modelMethod); - } this.modelMethods.remove(modelMethod); return modelMethod; } } ModelMethod modelMethod = this.modelMethods.get(0); - if (logger.isTraceEnabled()) { - logger.trace("Selected @ModelAttribute method (not present: " + - modelMethod.getUnresolvedDependencies(container)+ ") " + modelMethod); - } this.modelMethods.remove(modelMethod); return modelMethod; } diff --git a/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodArgumentResolverComposite.java b/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodArgumentResolverComposite.java index 7d872510d2a..a8c1bd9f525 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodArgumentResolverComposite.java +++ b/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodArgumentResolverComposite.java @@ -132,10 +132,6 @@ public class HandlerMethodArgumentResolverComposite implements HandlerMethodArgu HandlerMethodArgumentResolver result = this.argumentResolverCache.get(parameter); if (result == null) { for (HandlerMethodArgumentResolver methodArgumentResolver : this.argumentResolvers) { - if (logger.isTraceEnabled()) { - logger.trace("Testing if argument resolver [" + methodArgumentResolver + "] supports [" + - parameter.getGenericParameterType() + "]"); - } if (methodArgumentResolver.supportsParameter(parameter)) { result = methodArgumentResolver; this.argumentResolverCache.put(parameter, result); diff --git a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java index 88c17aa7ec8..500903955c3 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java +++ b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java @@ -19,6 +19,8 @@ package org.springframework.web.method.support; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Arrays; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.MethodParameter; @@ -131,15 +133,9 @@ public class InvocableHandlerMethod extends HandlerMethod { Object[] args = getMethodArgumentValues(request, mavContainer, providedArgs); if (logger.isTraceEnabled()) { - logger.trace("Invoking '" + ClassUtils.getQualifiedMethodName(getMethod(), getBeanType()) + - "' with arguments " + Arrays.toString(args)); + logger.trace("Arguments: " + Arrays.toString(args)); } - Object returnValue = doInvoke(args); - if (logger.isTraceEnabled()) { - logger.trace("Method [" + ClassUtils.getQualifiedMethodName(getMethod(), getBeanType()) + - "] returned [" + returnValue + "]"); - } - return returnValue; + return doInvoke(args); } /** @@ -214,7 +210,7 @@ public class InvocableHandlerMethod extends HandlerMethod { catch (IllegalArgumentException ex) { assertTargetBean(getBridgedMethod(), getBean(), args); String text = (ex.getMessage() != null ? ex.getMessage() : "Illegal argument"); - throw new IllegalStateException(getInvocationErrorMessage(text, args), ex); + throw new IllegalStateException(formatInvokeError(text, args), ex); } catch (InvocationTargetException ex) { // Unwrap for HandlerExceptionResolvers ... @@ -229,8 +225,7 @@ public class InvocableHandlerMethod extends HandlerMethod { throw (Exception) targetException; } else { - String text = getInvocationErrorMessage("Failed to invoke handler method", args); - throw new IllegalStateException(text, targetException); + throw new IllegalStateException(formatInvokeError("Invocation failure", args), targetException); } } } @@ -250,36 +245,22 @@ public class InvocableHandlerMethod extends HandlerMethod { "' is not an instance of the actual controller bean class '" + targetBeanClass.getName() + "'. If the controller requires proxying " + "(e.g. due to @Transactional), please use class-based proxying."; - throw new IllegalStateException(getInvocationErrorMessage(text, args)); + throw new IllegalStateException(formatInvokeError(text, args)); } } - private String getInvocationErrorMessage(String text, Object[] resolvedArgs) { - StringBuilder sb = new StringBuilder(getDetailedErrorMessage(text)); - sb.append("Resolved arguments: \n"); - for (int i = 0; i < resolvedArgs.length; i++) { - sb.append("[").append(i).append("] "); - if (resolvedArgs[i] == null) { - sb.append("[null] \n"); - } - else { - sb.append("[type=").append(resolvedArgs[i].getClass().getName()).append("] "); - sb.append("[value=").append(resolvedArgs[i]).append("]\n"); - } - } - return sb.toString(); - } + private String formatInvokeError(String text, Object[] args) { - /** - * Adds HandlerMethod details such as the bean type and method signature to the message. - * @param text error message to append the HandlerMethod details to - */ - protected String getDetailedErrorMessage(String text) { - StringBuilder sb = new StringBuilder(text).append("\n"); - sb.append("HandlerMethod details: \n"); - sb.append("Controller [").append(getBeanType().getName()).append("]\n"); - sb.append("Method [").append(getBridgedMethod().toGenericString()).append("]\n"); - return sb.toString(); + String formattedArgs = IntStream.range(0, args.length) + .mapToObj(i -> (args[i] != null ? + "[" + i + "] [type=" + args[i].getClass().getName() + "] [value=" + args[i] + "]" : + "[" + i + "] [null]")) + .collect(Collectors.joining(",\n", " ", " ")); + + return text + "\n" + + "Controller [" + getBeanType().getName() + "]\n" + + "Method [" + getBridgedMethod().toGenericString() + "] " + + "with argument values:\n" + formattedArgs; } } diff --git a/spring-web/src/main/java/org/springframework/web/server/ResponseStatusException.java b/spring-web/src/main/java/org/springframework/web/server/ResponseStatusException.java index 2995c162e6c..38491d8aac4 100644 --- a/spring-web/src/main/java/org/springframework/web/server/ResponseStatusException.java +++ b/spring-web/src/main/java/org/springframework/web/server/ResponseStatusException.java @@ -88,8 +88,7 @@ public class ResponseStatusException extends NestedRuntimeException { @Override public String getMessage() { - String msg = "Response status " + this.status + - (this.reason != null ? " with reason \"" + reason + "\"" : ""); + String msg = this.status + (this.reason != null ? " \"" + reason + "\"" : ""); return NestedExceptionUtils.buildMessage(msg, getCause()); } diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java index d52b46b1780..0e9fea8c5dc 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java @@ -27,12 +27,14 @@ import reactor.core.publisher.Mono; import org.springframework.context.ApplicationContext; import org.springframework.core.NestedExceptionUtils; import org.springframework.http.HttpStatus; +import org.springframework.http.codec.LoggingCodecSupport; import org.springframework.http.codec.ServerCodecConfigurer; import org.springframework.http.server.reactive.HttpHandler; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; import org.springframework.web.server.handler.WebHandlerDecorator; @@ -80,20 +82,21 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa private static final Log logger = LogFactory.getLog(HttpWebHandlerAdapter.class); - private static final Log disconnectedClientLogger = LogFactory.getLog(DISCONNECTED_CLIENT_LOG_CATEGORY); + private static final Log lostClientLogger = LogFactory.getLog(DISCONNECTED_CLIENT_LOG_CATEGORY); private WebSessionManager sessionManager = new DefaultWebSessionManager(); - @Nullable - private ServerCodecConfigurer codecConfigurer; + private ServerCodecConfigurer codecConfigurer = ServerCodecConfigurer.create(); - @Nullable - private LocaleContextResolver localeContextResolver; + private LocaleContextResolver localeContextResolver = new AcceptHeaderLocaleContextResolver(); @Nullable private ApplicationContext applicationContext; + /** Do not log potentially sensitive data (query/form at DEBUG, headers at TRACE). */ + private boolean disableLoggingRequestDetails = false; + public HttpWebHandlerAdapter(WebHandler delegate) { super(delegate); @@ -126,15 +129,24 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa * @param codecConfigurer the codec configurer to use */ public void setCodecConfigurer(ServerCodecConfigurer codecConfigurer) { - Assert.notNull(codecConfigurer, "ServerCodecConfigurer must not be null"); + Assert.notNull(codecConfigurer, "ServerCodecConfigurer is required"); this.codecConfigurer = codecConfigurer; + + this.disableLoggingRequestDetails = false; + this.codecConfigurer.getReaders().stream() + .filter(LoggingCodecSupport.class::isInstance) + .forEach(reader -> { + if (((LoggingCodecSupport) reader).isDisableLoggingRequestDetails()) { + this.disableLoggingRequestDetails = true; + } + }); } /** * Return the configured {@link ServerCodecConfigurer}. */ public ServerCodecConfigurer getCodecConfigurer() { - return (this.codecConfigurer != null ? this.codecConfigurer : ServerCodecConfigurer.create()); + return this.codecConfigurer; } /** @@ -142,18 +154,18 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa * each created {@link DefaultServerWebExchange}. *

By default this is set to * {@link org.springframework.web.server.i18n.AcceptHeaderLocaleContextResolver}. - * @param localeContextResolver the locale context resolver to use + * @param resolver the locale context resolver to use */ - public void setLocaleContextResolver(LocaleContextResolver localeContextResolver) { - this.localeContextResolver = localeContextResolver; + public void setLocaleContextResolver(LocaleContextResolver resolver) { + Assert.notNull(resolver, "LocaleContextResolver is required"); + this.localeContextResolver = resolver; } /** * Return the configured {@link LocaleContextResolver}. */ public LocaleContextResolver getLocaleContextResolver() { - return (this.localeContextResolver != null ? - this.localeContextResolver : new AcceptHeaderLocaleContextResolver()); + return this.localeContextResolver; } /** @@ -177,12 +189,36 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa return this.applicationContext; } + /** + * This method must be invoked after all properties have been set to + * complete initialization. + */ + public void afterPropertiesSet() { + if (logger.isDebugEnabled() || logger.isTraceEnabled()) { + if (this.disableLoggingRequestDetails) { + logger.debug("Logging query, form data, multipart data, and headers is OFF."); + } + else { + logger.warn("\n\n" + + "!!!!!!!!!!!!!!!!!!!\n" + + "Logging query, form and multipart data (DEBUG), and headers (TRACE) may show sensitive data.\n" + + "If not in development, set \"disableLoggingRequestDetails(true)\" on ServerCodecConfigurer,\n" + + "or lower the log level.\n" + + "!!!!!!!!!!!!!!!!!!!\n"); + } + } + } + @Override public Mono handle(ServerHttpRequest request, ServerHttpResponse response) { + ServerWebExchange exchange = createExchange(request, response); + logExchange(exchange); + return getDelegate().handle(exchange) - .onErrorResume(ex -> handleFailure(request, response, ex)) + .doOnSuccess(aVoid -> logResponse(response)) + .onErrorResume(ex -> handleUnresolvedError(request, response, ex)) .then(Mono.defer(response::setComplete)); } @@ -191,27 +227,65 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa getCodecConfigurer(), getLocaleContextResolver(), this.applicationContext); } - private Mono handleFailure(ServerHttpRequest request, ServerHttpResponse response, Throwable ex) { + private void logExchange(ServerWebExchange exchange) { + if (logger.isDebugEnabled() || logger.isTraceEnabled()) { + ServerHttpRequest request = exchange.getRequest(); + if (logger.isTraceEnabled()) { + String headers = this.disableLoggingRequestDetails ? "" : ", headers=" + request.getHeaders(); + logger.trace(formatRequest(request) + headers); + } + else { + logger.debug(formatRequest(request)); + } + } + } + + private String formatRequest(ServerHttpRequest request) { + String query = ""; + if (!this.disableLoggingRequestDetails) { + String rawQuery = request.getURI().getRawQuery(); + query = StringUtils.hasText(rawQuery) ? "?" + rawQuery : ""; + } + return "HTTP " + request.getMethod() + " " + request.getPath() + query; + } + + private void logResponse(ServerHttpResponse response) { + if (logger.isDebugEnabled() || logger.isTraceEnabled()) { + HttpStatus status = response.getStatusCode(); + String message = "Completed " + (status != null ? status : "200 OK"); + + if (logger.isTraceEnabled()) { + String headers = this.disableLoggingRequestDetails ? "" : ", headers=" + response.getHeaders(); + logger.trace(message + headers); + } + else { + logger.debug(message); + } + } + } + + private Mono handleUnresolvedError(ServerHttpRequest request, ServerHttpResponse response, Throwable ex) { + if (isDisconnectedClientError(ex)) { - if (disconnectedClientLogger.isTraceEnabled()) { - disconnectedClientLogger.trace("Looks like the client has gone away", ex); + if (lostClientLogger.isTraceEnabled()) { + lostClientLogger.trace("Client went away", ex); } - else if (disconnectedClientLogger.isDebugEnabled()) { - disconnectedClientLogger.debug("Looks like the client has gone away: " + ex + - " (For a full stack trace, set the log category '" + DISCONNECTED_CLIENT_LOG_CATEGORY + - "' to TRACE level.)"); + else if (lostClientLogger.isDebugEnabled()) { + lostClientLogger.debug("Client went away: " + ex + + " (stacktrace at TRACE level for '" + DISCONNECTED_CLIENT_LOG_CATEGORY + "')"); } return Mono.empty(); } - if (response.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR)) { - logger.error("Failed to handle request [" + request.getMethod() + " " - + request.getURI() + "]", ex); + else if (response.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR)) { + logger.error("500 Server Error for " + formatRequest(request), ex); return Mono.empty(); } - // After the response is committed, propagate errors to the server.. - HttpStatus status = response.getStatusCode(); - logger.error("Unhandled failure: " + ex.getMessage() + ", response already set (status=" + status + ")"); - return Mono.error(ex); + else { + // After the response is committed, propagate errors to the server.. + logger.error("Error [" + ex + "] for " + formatRequest(request) + + ", but ServerHttpResponse already committed (" + response.getStatusCode() + ")"); + return Mono.error(ex); + } } private boolean isDisconnectedClientError(Throwable ex) { diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java b/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java index 530c71eef3c..f7764d0cb0c 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java @@ -274,11 +274,11 @@ public class WebHttpHandlerBuilder { public HttpHandler build() { WebHandler decorated; - decorated = new FilteringWebHandler(this.webHandler, this.filters); decorated = new ExceptionHandlingWebHandler(decorated, this.exceptionHandlers); HttpWebHandlerAdapter adapted = new HttpWebHandlerAdapter(decorated); + if (this.sessionManager != null) { adapted.setSessionManager(this.sessionManager); } @@ -292,6 +292,8 @@ public class WebHttpHandlerBuilder { adapted.setApplicationContext(this.applicationContext); } + adapted.afterPropertiesSet(); + return adapted; } diff --git a/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java b/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java index 5093a28be8c..a88847a5a52 100644 --- a/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java +++ b/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java @@ -30,6 +30,10 @@ import org.springframework.web.server.WebExceptionHandler; /** * Handle {@link ResponseStatusException} by setting the response status. * + *

By default exception stack traces are not shown for successfully resolved + * exceptions. Use {@link #setWarnLogCategory(String)} to enable logging with + * stack traces. + * * @author Rossen Stoyanchev * @author Sebastien Deleuze * @since 5.0 @@ -39,26 +43,48 @@ public class ResponseStatusExceptionHandler implements WebExceptionHandler { private static final Log logger = LogFactory.getLog(ResponseStatusExceptionHandler.class); + @Nullable + private Log warnLogger; + + + /** + * Set the log category for warn logging. + *

Default is no warn logging. Specify this setting to activate warn + * logging into a specific category. + * @see org.apache.commons.logging.LogFactory#getLog(String) + * @see java.util.logging.Logger#getLogger(String) + * @see 5.1 + */ + public void setWarnLogCategory(String loggerName) { + this.warnLogger = LogFactory.getLog(loggerName); + } + + @Override public Mono handle(ServerWebExchange exchange, Throwable ex) { + HttpStatus status = resolveStatus(ex); - if (status != null && exchange.getResponse().setStatusCode(status)) { - if (status.is5xxServerError()) { - logger.error(buildMessage(exchange.getRequest(), ex)); - } - else if (status == HttpStatus.BAD_REQUEST) { - logger.warn(buildMessage(exchange.getRequest(), ex)); - } - else { - logger.trace(buildMessage(exchange.getRequest(), ex)); - } - return exchange.getResponse().setComplete(); + if (status == null || !exchange.getResponse().setStatusCode(status)) { + return Mono.error(ex); } - return Mono.error(ex); + + // Mirrors AbstractHandlerExceptionResolver in spring-webmvc.. + + if (this.warnLogger != null && this.warnLogger.isWarnEnabled()) { + this.warnLogger.warn(formatError(ex, exchange.getRequest()), ex); + } + else if (logger.isDebugEnabled()) { + logger.debug(formatError(ex, exchange.getRequest())); + } + + return exchange.getResponse().setComplete(); } - private String buildMessage(ServerHttpRequest request, Throwable ex) { - return "Failed to handle request [" + request.getMethod() + " " + request.getURI() + "]: " + ex.getMessage(); + + private String formatError(Throwable ex, ServerHttpRequest request) { + String reason = ex.getClass().getSimpleName() + ": " + ex.getMessage(); + String path = request.getURI().getRawPath(); + return "Resolved [" + reason + "] for HTTP " + request.getMethod() + " " + path; } @Nullable diff --git a/spring-web/src/main/java/org/springframework/web/util/CookieGenerator.java b/spring-web/src/main/java/org/springframework/web/util/CookieGenerator.java index ce526a3dccc..ba6bd078f45 100644 --- a/spring-web/src/main/java/org/springframework/web/util/CookieGenerator.java +++ b/spring-web/src/main/java/org/springframework/web/util/CookieGenerator.java @@ -195,8 +195,8 @@ public class CookieGenerator { cookie.setHttpOnly(true); } response.addCookie(cookie); - if (logger.isDebugEnabled()) { - logger.debug("Added cookie with name [" + getCookieName() + "] and value [" + cookieValue + "]"); + if (logger.isTraceEnabled()) { + logger.trace("Added cookie [" + getCookieName() + "=" + cookieValue + "]"); } } @@ -220,8 +220,8 @@ public class CookieGenerator { cookie.setHttpOnly(true); } response.addCookie(cookie); - if (logger.isDebugEnabled()) { - logger.debug("Removed cookie with name [" + getCookieName() + "]"); + if (logger.isTraceEnabled()) { + logger.trace("Removed cookie '" + getCookieName() + "'"); } } diff --git a/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java b/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java index d1e0cd3a4dc..105766fa802 100644 --- a/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java @@ -153,7 +153,7 @@ public class InvocableHandlerMethodTests { assertTrue(ex.getCause() instanceof IllegalArgumentException); assertTrue(ex.getMessage().contains("Controller [")); assertTrue(ex.getMessage().contains("Method [")); - assertTrue(ex.getMessage().contains("Resolved arguments: ")); + assertTrue(ex.getMessage().contains("with argument values:")); assertTrue(ex.getMessage().contains("[0] [type=java.lang.String] [value=__invalid__]")); assertTrue(ex.getMessage().contains("[1] [type=java.lang.String] [value=value")); } @@ -192,7 +192,7 @@ public class InvocableHandlerMethodTests { catch (IllegalStateException actual) { assertNotNull(actual.getCause()); assertSame(expected, actual.getCause()); - assertTrue(actual.getMessage().contains("Failed to invoke handler method")); + assertTrue(actual.getMessage().contains("Invocation failure")); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java index 0655f3a684c..7b835887341 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java @@ -33,10 +33,13 @@ import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.HttpStatus; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.lang.Nullable; -import org.springframework.web.server.adapter.WebHttpHandlerBuilder; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; +import org.springframework.util.StringUtils; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; +import org.springframework.web.server.adapter.WebHttpHandlerBuilder; /** * Central dispatcher for HTTP request handlers/controllers. Dispatches to @@ -146,10 +149,6 @@ public class DispatcherHandler implements WebHandler, ApplicationContextAware { @Override public Mono handle(ServerWebExchange exchange) { - if (logger.isDebugEnabled()) { - ServerHttpRequest request = exchange.getRequest(); - logger.debug("Processing " + request.getMethodValue() + " request for [" + request.getURI() + "]"); - } if (this.handlerMappings == null) { return Mono.error(HANDLER_NOT_FOUND_EXCEPTION); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/FixedContentTypeResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/FixedContentTypeResolver.java index 95019daf0fa..2b71e2453e1 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/FixedContentTypeResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/FixedContentTypeResolver.java @@ -19,9 +19,6 @@ package org.springframework.web.reactive.accept; import java.util.Collections; import java.util.List; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - import org.springframework.http.MediaType; import org.springframework.util.Assert; import org.springframework.web.server.ServerWebExchange; @@ -36,9 +33,6 @@ import org.springframework.web.server.ServerWebExchange; */ public class FixedContentTypeResolver implements RequestedContentTypeResolver { - private static final Log logger = LogFactory.getLog(FixedContentTypeResolver.class); - - private final List contentTypes; @@ -71,9 +65,6 @@ public class FixedContentTypeResolver implements RequestedContentTypeResolver { @Override public List resolveMediaTypes(ServerWebExchange exchange) { - if (logger.isDebugEnabled()) { - logger.debug("Requested media types: " + this.contentTypes); - } return this.contentTypes; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistration.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistration.java index 37ee84058dd..9d74ffa52c7 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistration.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistration.java @@ -17,6 +17,7 @@ package org.springframework.web.reactive.config; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.springframework.cache.Cache; @@ -39,7 +40,7 @@ public class ResourceHandlerRegistration { private final String[] pathPatterns; - private final List locations = new ArrayList<>(); + private final List locationValues = new ArrayList<>(); @Nullable private CacheControl cacheControl; @@ -77,9 +78,7 @@ public class ResourceHandlerRegistration { * chained method invocation */ public ResourceHandlerRegistration addResourceLocations(String... resourceLocations) { - for (String location : resourceLocations) { - this.locations.add(this.resourceLoader.getResource(location)); - } + this.locationValues.addAll(Arrays.asList(resourceLocations)); return this; } @@ -145,11 +144,12 @@ public class ResourceHandlerRegistration { */ protected ResourceWebHandler getRequestHandler() { ResourceWebHandler handler = new ResourceWebHandler(); + handler.setLocationValues(this.locationValues); + handler.setResourceLoader(this.resourceLoader); if (this.resourceChainRegistration != null) { handler.setResourceResolvers(this.resourceChainRegistration.getResourceResolvers()); handler.setResourceTransformers(this.resourceChainRegistration.getResourceTransformers()); } - handler.setLocations(this.locations); if (this.cacheControl != null) { handler.setCacheControl(this.cacheControl); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java index 027fdd5c99a..9909d5d23ae 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java @@ -25,6 +25,8 @@ import reactor.core.publisher.Mono; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.client.reactive.ClientHttpConnector; +import org.springframework.http.client.reactive.ClientHttpResponse; +import org.springframework.http.codec.LoggingCodecSupport; import org.springframework.util.Assert; /** @@ -69,12 +71,22 @@ public abstract class ExchangeFunctions { private final ExchangeStrategies strategies; + private boolean disableLoggingRequestDetails; + public DefaultExchangeFunction(ClientHttpConnector connector, ExchangeStrategies strategies) { Assert.notNull(connector, "ClientHttpConnector must not be null"); Assert.notNull(strategies, "ExchangeStrategies must not be null"); this.connector = connector; this.strategies = strategies; + + strategies.messageWriters().stream() + .filter(LoggingCodecSupport.class::isInstance) + .forEach(reader -> { + if (((LoggingCodecSupport) reader).isDisableLoggingRequestDetails()) { + this.disableLoggingRequestDetails = true; + } + }); } @@ -87,19 +99,39 @@ public abstract class ExchangeFunctions { return this.connector .connect(httpMethod, url, httpRequest -> request.writeTo(httpRequest, this.strategies)) - .doOnSubscribe(subscription -> logger.debug("Subscriber present")) - .doOnRequest(n -> logger.debug("Demand signaled")) - .doOnCancel(() -> logger.debug("Cancelling request")) + .doOnRequest(n -> logRequest(request)) + .doOnCancel(() -> logger.debug("Cancel signal (to close connection)")) .map(response -> { - if (logger.isDebugEnabled()) { - int code = response.getRawStatusCode(); - HttpStatus status = HttpStatus.resolve(code); - String reason = status != null ? " " + status.getReasonPhrase() : ""; - logger.debug("Response received, status: " + code + reason); - } + logResponse(response); return new DefaultClientResponse(response, this.strategies); }); } + + private void logRequest(ClientRequest request) { + if (logger.isDebugEnabled()) { + String formatted = request.url().toString(); + if (this.disableLoggingRequestDetails) { + int index = formatted.indexOf("?"); + formatted = index != -1 ? formatted.substring(0, index) : formatted; + } + logger.debug("HTTP " + request.method() + " " + formatted); + } + } + + private void logResponse(ClientHttpResponse response) { + if (logger.isDebugEnabled() || logger.isTraceEnabled()) { + int code = response.getRawStatusCode(); + HttpStatus status = HttpStatus.resolve(code); + String message = "Response " + (status != null ? status : code); + if (logger.isTraceEnabled()) { + String headers = this.disableLoggingRequestDetails ? "" : ", headers=" + response.getHeaders(); + logger.trace(message + headers); + } + else { + logger.debug(message); + } + } + } } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequest.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequest.java index aaea8fce8c0..3fea5d8aa11 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequest.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequest.java @@ -216,7 +216,7 @@ class DefaultServerRequest implements ServerRequest { @Override public String toString() { - return String.format("%s %s", method(), path()); + return String.format("HTTP %s %s", method(), path()); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java index 03bd27294ae..e8529e39d03 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java @@ -328,9 +328,8 @@ public abstract class RequestPredicates { private static void traceMatch(String prefix, Object desired, @Nullable Object actual, boolean match) { if (logger.isTraceEnabled()) { - String message = String.format("%s \"%s\" %s against value \"%s\"", - prefix, desired, match ? "matches" : "does not match", actual); - logger.trace(message); + logger.trace(String.format("%s \"%s\" %s against value \"%s\"", + prefix, desired, match ? "matches" : "does not match", actual)); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java index 8b241efbd6c..6b64140ab9a 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java @@ -421,8 +421,8 @@ public abstract class RouterFunctions { @Override public Mono> route(ServerRequest request) { if (this.predicate.test(request)) { - if (logger.isDebugEnabled()) { - logger.debug(String.format("Predicate \"%s\" matches against \"%s\"", this.predicate, request)); + if (logger.isTraceEnabled()) { + logger.trace(String.format("Matched %s", this.predicate)); } return Mono.just(this.handlerFunction); } @@ -456,19 +456,15 @@ public abstract class RouterFunctions { public Mono> route(ServerRequest serverRequest) { return this.predicate.nest(serverRequest) .map(nestedRequest -> { - if (logger.isDebugEnabled()) { - logger.debug( - String.format( - "Nested predicate \"%s\" matches against \"%s\"", - this.predicate, serverRequest)); + if (logger.isTraceEnabled()) { + logger.trace(String.format("Matched nested %s", this.predicate)); } return this.routerFunction.route(nestedRequest) .doOnNext(match -> { mergeTemplateVariables(serverRequest, nestedRequest.pathVariables()); }); } - ) - .orElseGet(Mono::empty); + ).orElseGet(Mono::empty); } @SuppressWarnings("unchecked") diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java index 66a2e2a65fc..275966ba212 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java @@ -105,28 +105,37 @@ public class RouterFunctionMapping extends AbstractHandlerMapping implements Ini * Initialized the router functions by detecting them in the application context. */ protected void initRouterFunctions() { - if (logger.isDebugEnabled()) { - logger.debug("Looking for router functions in application context: " + - getApplicationContext()); - } - List> routerFunctions = routerFunctions(); - if (!CollectionUtils.isEmpty(routerFunctions) && logger.isInfoEnabled()) { - routerFunctions.forEach(routerFunction -> logger.info("Mapped " + routerFunction)); - } - this.routerFunction = routerFunctions.stream() - .reduce(RouterFunction::andOther) - .orElse(null); + this.routerFunction = routerFunctions.stream().reduce(RouterFunction::andOther).orElse(null); + logRouterFunctions(routerFunctions); } private List> routerFunctions() { SortedRouterFunctionsContainer container = new SortedRouterFunctionsContainer(); obtainApplicationContext().getAutowireCapableBeanFactory().autowireBean(container); + List> functions = container.routerFunctions; + return CollectionUtils.isEmpty(functions) ? Collections.emptyList() : functions; + } - return CollectionUtils.isEmpty(container.routerFunctions) ? Collections.emptyList() : - container.routerFunctions; + private void logRouterFunctions(List> routerFunctions) { + if (logger.isDebugEnabled() || logger.isTraceEnabled()) { + int total = routerFunctions.size(); + String message = total + " RouterFunction(s) in " + formatMappingName(); + if (logger.isTraceEnabled()) { + if (total > 0) { + routerFunctions.forEach(routerFunction -> logger.trace("Mapped " + routerFunction)); + } + else { + logger.trace(message); + } + } + else if (total > 0) { + logger.debug(message); + } + } } + @Override protected Mono getHandlerInternal(ServerWebExchange exchange) { if (this.routerFunction != null) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java index 2783640711e..aa6c56634c0 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java @@ -20,6 +20,7 @@ import java.util.Map; import reactor.core.publisher.Mono; +import org.springframework.beans.factory.BeanNameAware; import org.springframework.context.support.ApplicationObjectSupport; import org.springframework.core.Ordered; import org.springframework.lang.Nullable; @@ -44,7 +45,8 @@ import org.springframework.web.util.pattern.PathPatternParser; * @author Brian Clozel * @since 5.0 */ -public abstract class AbstractHandlerMapping extends ApplicationObjectSupport implements HandlerMapping, Ordered { +public abstract class AbstractHandlerMapping extends ApplicationObjectSupport + implements HandlerMapping, Ordered, BeanNameAware { private static final WebHandler REQUEST_HANDLED_HANDLER = exchange -> Mono.empty(); @@ -57,6 +59,9 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport im private int order = Ordered.LOWEST_PRECEDENCE; // default: same as non-Ordered + @Nullable + private String beanName; + public AbstractHandlerMapping() { this.patternParser = new PathPatternParser(); @@ -141,10 +146,22 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport im return this.order; } + @Override + public void setBeanName(String name) { + this.beanName = name; + } + + protected String formatMappingName() { + return this.beanName != null ? "'" + this.beanName + "'" : ""; + } + @Override public Mono getHandler(ServerWebExchange exchange) { return getHandlerInternal(exchange).map(handler -> { + if (logger.isDebugEnabled()) { + logger.debug("Mapped to " + handler); + } if (CorsUtils.isCorsRequest(exchange.getRequest())) { CorsConfiguration configA = this.globalCorsConfigSource.getCorsConfiguration(exchange); CorsConfiguration configB = getCorsConfiguration(handler, exchange); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java index 6eeec5f335a..9118354a0ad 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java @@ -18,7 +18,9 @@ package org.springframework.web.reactive.handler; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import reactor.core.publisher.Mono; @@ -90,14 +92,6 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { catch (Exception ex) { return Mono.error(ex); } - - if (handler != null && logger.isDebugEnabled()) { - logger.debug("Mapping [" + lookupPath + "] to " + handler); - } - else if (handler == null && logger.isTraceEnabled()) { - logger.trace("No handler mapping found for [" + lookupPath + "]"); - } - return Mono.justOrEmpty(handler); } @@ -113,20 +107,25 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { */ @Nullable protected Object lookupHandler(PathContainer lookupPath, ServerWebExchange exchange) throws Exception { - return this.handlerMap.entrySet().stream() - .filter(entry -> entry.getKey().matches(lookupPath)) - .sorted((entry1, entry2) -> - PathPattern.SPECIFICITY_COMPARATOR.compare(entry1.getKey(), entry2.getKey())) - .findFirst() - .map(entry -> { - PathPattern pattern = entry.getKey(); - if (logger.isDebugEnabled()) { - logger.debug("Matching pattern for request [" + lookupPath + "] is " + pattern); - } - PathContainer pathWithinMapping = pattern.extractPathWithinPattern(lookupPath); - return handleMatch(entry.getValue(), pattern, pathWithinMapping, exchange); - }) - .orElse(null); + + List matches = this.handlerMap.keySet().stream() + .filter(key -> key.matches(lookupPath)) + .collect(Collectors.toList()); + + if (matches.isEmpty()) { + return null; + } + + if (matches.size() > 1) { + matches.sort(PathPattern.SPECIFICITY_COMPARATOR); + if (logger.isTraceEnabled()) { + logger.debug("Matching patterns " + matches); + } + } + + PathPattern pattern = matches.get(0); + PathContainer pathWithinMapping = pattern.extractPathWithinPattern(lookupPath); + return handleMatch(this.handlerMap.get(pattern), pattern, pathWithinMapping, exchange); } private Object handleMatch(Object handler, PathPattern bestMatch, PathContainer pathWithinMapping, @@ -207,14 +206,13 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { // Register resolved handler this.handlerMap.put(pattern, resolvedHandler); - if (logger.isInfoEnabled()) { - logger.info("Mapped URL path [" + urlPath + "] onto " + getHandlerDescription(handler)); + if (logger.isTraceEnabled()) { + logger.trace("Mapped [" + urlPath + "] onto " + getHandlerDescription(handler)); } } private String getHandlerDescription(Object handler) { - return "handler " + (handler instanceof String ? - "'" + handler + "'" : "of type [" + handler.getClass() + "]"); + return (handler instanceof String ? "'" + handler + "'" : handler.toString()); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMapping.java index 1297a58683d..1725dc3f7cc 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMapping.java @@ -16,7 +16,9 @@ package org.springframework.web.reactive.handler; +import java.util.ArrayList; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Properties; @@ -110,7 +112,7 @@ public class SimpleUrlHandlerMapping extends AbstractUrlHandlerMapping { */ protected void registerHandlers(Map urlMap) throws BeansException { if (urlMap.isEmpty()) { - logger.warn("Neither 'urlMap' nor 'mappings' set on SimpleUrlHandlerMapping"); + logger.trace("No patterns in " + formatMappingName()); } else { for (Map.Entry entry : urlMap.entrySet()) { @@ -126,6 +128,9 @@ public class SimpleUrlHandlerMapping extends AbstractUrlHandlerMapping { } registerHandler(url, handler); } + if (logger.isDebugEnabled()) { + logger.debug("Patterns " + getHandlerMap().keySet() + " in " + formatMappingName()); + } } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/AbstractResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/AbstractResourceResolver.java index 14e88f0fc8f..d5f9005e70e 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/AbstractResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/AbstractResourceResolver.java @@ -41,9 +41,6 @@ public abstract class AbstractResourceResolver implements ResourceResolver { public Mono resolveResource(@Nullable ServerWebExchange exchange, String requestPath, List locations, ResourceResolverChain chain) { - if (logger.isTraceEnabled()) { - logger.trace("Resolving resource for request path \"" + requestPath + "\""); - } return resolveResourceInternal(exchange, requestPath, locations, chain); } @@ -51,10 +48,6 @@ public abstract class AbstractResourceResolver implements ResourceResolver { public Mono resolveUrlPath(String resourceUrlPath, List locations, ResourceResolverChain chain) { - if (logger.isTraceEnabled()) { - logger.trace("Resolving public URL for resource path \"" + resourceUrlPath + "\""); - } - return resolveUrlPathInternal(resourceUrlPath, locations, chain); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/AppCacheManifestTransformer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/AppCacheManifestTransformer.java index ac0b6c00abb..c8564e8c843 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/AppCacheManifestTransformer.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/AppCacheManifestTransformer.java @@ -127,13 +127,10 @@ public class AppCacheManifestTransformer extends ResourceTransformerSupport { if (!content.startsWith(MANIFEST_HEADER)) { if (logger.isTraceEnabled()) { - logger.trace("Manifest should start with 'CACHE MANIFEST', skip: " + resource); + logger.trace("Skipping " + resource + ": Manifest does not start with 'CACHE MANIFEST'"); } return Mono.just(resource); } - if (logger.isTraceEnabled()) { - logger.trace("Transforming resource: " + resource); - } return Flux.generate(new LineInfoGenerator(content)) .concatMap(info -> processLine(info, exchange, resource, chain)) .reduce(new ByteArrayOutputStream(), (out, line) -> { @@ -143,9 +140,6 @@ public class AppCacheManifestTransformer extends ResourceTransformerSupport { .map(out -> { String hash = DigestUtils.md5DigestAsHex(out.toByteArray()); writeToByteArrayOutputStream(out, "\n" + "# Hash: " + hash); - if (logger.isTraceEnabled()) { - logger.trace("AppCache file: [" + resource.getFilename()+ "] hash: [" + hash + "]"); - } return new TransformedResource(resource, out.toByteArray()); }); } @@ -168,12 +162,7 @@ public class AppCacheManifestTransformer extends ResourceTransformerSupport { } String link = toAbsolutePath(info.getLine(), exchange); - return resolveUrlPath(link, exchange, resource, chain) - .doOnNext(path -> { - if (logger.isTraceEnabled()) { - logger.trace("Link modified: " + path + " (original: " + info.getLine() + ")"); - } - }); + return resolveUrlPath(link, exchange, resource, chain); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceResolver.java index 64ddec4ac84..49854044ef0 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceResolver.java @@ -109,19 +109,12 @@ public class CachingResourceResolver extends AbstractResourceResolver { Resource cachedResource = this.cache.get(key, Resource.class); if (cachedResource != null) { - if (logger.isTraceEnabled()) { - logger.trace("Found match: " + cachedResource); - } + logger.trace("Resource resolved from cache"); return Mono.just(cachedResource); } return chain.resolveResource(exchange, requestPath, locations) - .doOnNext(resource -> { - if (logger.isTraceEnabled()) { - logger.trace("Putting resolved resource in cache: " + resource); - } - this.cache.put(key, resource); - }); + .doOnNext(resource -> this.cache.put(key, resource)); } protected String computeKey(@Nullable ServerWebExchange exchange, String requestPath) { @@ -160,19 +153,12 @@ public class CachingResourceResolver extends AbstractResourceResolver { String cachedUrlPath = this.cache.get(key, String.class); if (cachedUrlPath != null) { - if (logger.isTraceEnabled()) { - logger.trace("Found match: \"" + cachedUrlPath + "\""); - } + logger.trace("Path resolved from cache"); return Mono.just(cachedUrlPath); } return chain.resolveUrlPath(resourceUrlPath, locations) - .doOnNext(resolvedPath -> { - if (logger.isTraceEnabled()) { - logger.trace("Putting resolved resource URL path in cache: \"" + resolvedPath + "\""); - } - this.cache.put(key, resolvedPath); - }); + .doOnNext(resolvedPath -> this.cache.put(key, resolvedPath)); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceTransformer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceTransformer.java index 0c5bcf9d7fb..37457096a4d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceTransformer.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CachingResourceTransformer.java @@ -69,19 +69,12 @@ public class CachingResourceTransformer implements ResourceTransformer { Resource cachedResource = this.cache.get(resource, Resource.class); if (cachedResource != null) { - if (logger.isTraceEnabled()) { - logger.trace("Found match: " + cachedResource); - } + logger.trace("Resource resolved from cache"); return Mono.just(cachedResource); } return transformerChain.transform(exchange, resource) - .doOnNext(transformed -> { - if (logger.isTraceEnabled()) { - logger.trace("Putting transformed resource in cache: " + transformed); - } - this.cache.put(resource, transformed); - }); + .doOnNext(transformed -> this.cache.put(resource, transformed)); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CssLinkResourceTransformer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CssLinkResourceTransformer.java index 8261bafe208..82c344ac563 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CssLinkResourceTransformer.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/CssLinkResourceTransformer.java @@ -84,10 +84,6 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport { return Mono.just(ouptputResource); } - if (logger.isTraceEnabled()) { - logger.trace("Transforming resource: " + ouptputResource); - } - DataBufferFactory bufferFactory = exchange.getResponse().bufferFactory(); Flux flux = DataBufferUtils .read(ouptputResource, bufferFactory, StreamUtils.BUFFER_SIZE); @@ -106,9 +102,6 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport { List contentChunkInfos = parseContent(cssContent); if (contentChunkInfos.isEmpty()) { - if (logger.isTraceEnabled()) { - logger.trace("No links found."); - } return Mono.just(resource); } @@ -228,8 +221,8 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport { if (content.substring(position, position + 4).equals("url(")) { // Ignore, UrlFunctionContentParser will take care } - else if (logger.isErrorEnabled()) { - logger.error("Unexpected syntax for @import link at index " + position); + else if (logger.isTraceEnabled()) { + logger.trace("Unexpected syntax for @import link at index " + position); } return position; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/GzipResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/GzipResourceResolver.java index c9fbb560a5f..f30ab32cd9f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/GzipResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/GzipResourceResolver.java @@ -59,7 +59,7 @@ public class GzipResourceResolver extends AbstractResourceResolver { } } catch (IOException ex) { - logger.trace("No gzipped resource for [" + resource.getFilename() + "]", ex); + logger.trace("No gzip resource for [" + resource.getFilename() + "]", ex); } } return resource; 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 5b754d9b656..260783dc597 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 @@ -111,27 +111,27 @@ public class PathResourceResolver extends AbstractResourceResolver { Resource resource = location.createRelative(resourcePath); if (resource.isReadable()) { if (checkResource(resource, location)) { - if (logger.isTraceEnabled()) { - logger.trace("Found match: " + resource); - } return Mono.just(resource); } - else if (logger.isTraceEnabled()) { + else if (logger.isWarnEnabled()) { Resource[] allowedLocations = getAllowedLocations(); - logger.trace("Resource path \"" + resourcePath + "\" was successfully resolved " + + 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) : "[]")); } } - else if (logger.isTraceEnabled()) { - logger.trace("No match for location: " + location); - } return Mono.empty(); } catch (IOException ex) { - if (logger.isTraceEnabled()) { - logger.trace("Failure checking for relative resource under location + " + location, ex); + if (logger.isDebugEnabled() || logger.isTraceEnabled()) { + String error = "Skip location [" + location + "] due to error"; + if (logger.isTraceEnabled()) { + logger.trace(error, ex); + } + else { + logger.debug(error + ": " + ex.getMessage()); + } } return Mono.error(ex); } @@ -194,9 +194,7 @@ public class PathResourceResolver extends AbstractResourceResolver { try { String decodedPath = URLDecoder.decode(resourcePath, "UTF-8"); if (decodedPath.contains("../") || decodedPath.contains("..\\")) { - if (logger.isTraceEnabled()) { - logger.trace("Resolved resource path contains encoded \"../\" or \"..\\\": " + resourcePath); - } + logger.warn("Resolved resource path contains encoded \"../\" or \"..\\\": " + resourcePath); return true; } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java index 3a54aeaf9ca..fd338658baa 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java @@ -87,15 +87,10 @@ public class ResourceUrlProvider implements ApplicationListener beans = context.getBeansOfType(SimpleUrlHandlerMapping.class); List mappings = new ArrayList<>(beans.values()); AnnotationAwareOrderComparator.sort(mappings); @@ -104,14 +99,13 @@ public class ResourceUrlProvider implements ApplicationListener { if (handler instanceof ResourceWebHandler) { ResourceWebHandler resourceHandler = (ResourceWebHandler) handler; - if (logger.isDebugEnabled()) { - logger.debug("Found resource handler mapping: URL pattern=\"" + pattern + "\", " + - "locations=" + resourceHandler.getLocations() + ", " + - "resolvers=" + resourceHandler.getResourceResolvers()); - } this.handlerMap.put(pattern, resourceHandler); } })); + + if (this.handlerMap.isEmpty()) { + logger.trace("No resource handling mappings found"); + } } @@ -124,17 +118,11 @@ public class ResourceUrlProvider implements ApplicationListener getForUriString(String uriString, ServerWebExchange exchange) { - if (logger.isTraceEnabled()) { - logger.trace("Getting resource URL for request URL \"" + uriString + "\""); - } ServerHttpRequest request = exchange.getRequest(); int queryIndex = getQueryIndex(uriString); String lookupPath = uriString.substring(0, queryIndex); String query = uriString.substring(queryIndex); PathContainer parsedLookupPath = PathContainer.parsePath(lookupPath); - if (logger.isTraceEnabled()) { - logger.trace("Getting resource URL for lookup path \"" + lookupPath + "\""); - } return resolveResourceUrl(parsedLookupPath).map(resolvedPath -> request.getPath().contextPath().value() + resolvedPath + query); } @@ -162,23 +150,21 @@ public class ResourceUrlProvider implements ApplicationListener resolvers = handler.getResourceResolvers(); ResourceResolverChain chain = new DefaultResourceResolverChain(resolvers); return chain.resolveUrlPath(path.value(), handler.getLocations()) .map(resolvedPath -> { - if (logger.isTraceEnabled()) { - logger.trace("Resolved public resource URL path \"" + resolvedPath + "\""); - } return mapping.value() + resolvedPath; }); }) - .orElse(Mono.empty()); + .orElseGet(() ->{ + if (logger.isTraceEnabled()) { + logger.trace("No match for \"" + lookupPath + "\""); + } + return Mono.empty(); + }); } 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 818fb93e2ca..b522f6812c0 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 @@ -25,6 +25,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -33,6 +34,7 @@ import reactor.core.publisher.Mono; import org.springframework.beans.factory.InitializingBean; import org.springframework.core.ResolvableType; import org.springframework.core.io.Resource; +import org.springframework.core.io.ResourceLoader; import org.springframework.http.CacheControl; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; @@ -90,6 +92,8 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { private static final Log logger = LogFactory.getLog(ResourceWebHandler.class); + private final List locationValues = new ArrayList<>(4); + private final List locations = new ArrayList<>(4); private final List resourceResolvers = new ArrayList<>(4); @@ -108,6 +112,28 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { @Nullable private ResourceHttpMessageWriter resourceHttpMessageWriter; + @Nullable + private ResourceLoader resourceLoader; + + + /** + * Accepts a list of String-based location values to be resolved into + * {@link Resource} locations. + * @since 5.1 + */ + public void setLocationValues(List locationValues) { + Assert.notNull(locationValues, "Location values list must not be null"); + this.locationValues.clear(); + this.locationValues.addAll(locationValues); + } + + /** + * Return the configured location values. + * @since 5.1 + */ + public List getLocationValues() { + return this.locationValues; + } /** * Set the {@code List} of {@code Resource} paths to use as sources @@ -123,6 +149,11 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { /** * Return the {@code List} of {@code Resource} paths to use as sources * for serving static resources. + *

Note that if {@link #setLocationValues(List) locationValues} are provided, + * instead of loaded Resource-based locations, this method will return + * empty until after initialization via {@link #afterPropertiesSet()}. + * @see #setLocationValues + * @see #setLocations */ public List getLocations() { return this.locations; @@ -198,9 +229,25 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { return this.resourceHttpMessageWriter; } + /** + * Provide the ResourceLoader to load {@link #setLocationValues(List) + * location values} with. + * @since 5.1 + */ + public void setResourceLoader(ResourceLoader resourceLoader) { + this.resourceLoader = resourceLoader; + } + @Override public void afterPropertiesSet() throws Exception { + resolveResourceLocations(); + + if (logger.isWarnEnabled() && CollectionUtils.isEmpty(this.locations)) { + logger.warn("Locations list is empty. No resources will be served unless a " + + "custom ResourceResolver is configured as an alternative to PathResourceResolver."); + } + if (this.resourceResolvers.isEmpty()) { this.resourceResolvers.add(new PathResourceResolver()); } @@ -216,6 +263,24 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { this.transformerChain = new DefaultResourceTransformerChain(this.resolverChain, this.resourceTransformers); } + private void resolveResourceLocations() { + if (CollectionUtils.isEmpty(this.locationValues)) { + return; + } + else if (!CollectionUtils.isEmpty(this.locations)) { + throw new IllegalArgumentException("Please set either Resource-based \"locations\" or " + + "String-based \"locationValues\", but not both."); + } + + Assert.notNull(this.resourceLoader, + "ResourceLoader is required when \"locationValues\" are configured."); + + for (String location : this.locationValues) { + Resource resource = this.resourceLoader.getResource(location); + this.locations.add(resource); + } + } + /** * Look for a {@code PathResourceResolver} among the configured resource * resolvers and set its {@code allowedLocations} property (if empty) to @@ -257,7 +322,7 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { public Mono handle(ServerWebExchange exchange) { return getResource(exchange) .switchIfEmpty(Mono.defer(() -> { - logger.trace("No matching resource found - returning 404"); + logger.debug("Resource not found"); return Mono.error(NOT_FOUND_EXCEPTION); })) .flatMap(resource -> { @@ -276,7 +341,7 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { // Header phase if (exchange.checkNotModified(Instant.ofEpochMilli(resource.lastModified()))) { - logger.trace("Resource not modified - returning 304"); + logger.trace("Resource not modified"); return Mono.empty(); } @@ -290,23 +355,11 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { // Check the media type for the resource MediaType mediaType = MediaTypeFactory.getMediaType(resource).orElse(null); - if (mediaType != null) { - if (logger.isTraceEnabled()) { - logger.trace("Determined media type '" + mediaType + "' for " + resource); - } - } - else { - if (logger.isTraceEnabled()) { - logger.trace("No media type found " + - "for " + resource + " - not sending a content-type header"); - } - } // Content phase if (HttpMethod.HEAD.matches(exchange.getRequest().getMethodValue())) { setHeaders(exchange, resource, mediaType); exchange.getResponse().getHeaders().set(HttpHeaders.ACCEPT_RANGES, "bytes"); - logger.trace("HEAD request - skipping content"); return Mono.empty(); } @@ -329,15 +382,9 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { String path = processPath(pathWithinHandler.value()); if (!StringUtils.hasText(path) || isInvalidPath(path)) { - if (logger.isTraceEnabled()) { - logger.trace("Ignoring invalid resource path [" + path + "]"); - } return Mono.empty(); } if (isInvalidEncodedPath(path)) { - if (logger.isTraceEnabled()) { - logger.trace("Ignoring invalid resource path with escape sequences [" + path + "]"); - } return Mono.empty(); } @@ -399,11 +446,7 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { if (i == 0 || (i == 1 && slash)) { return path; } - path = slash ? "/" + path.substring(i) : path.substring(i); - if (logger.isTraceEnabled()) { - logger.trace("Path after trimming leading '/' and control characters: " + path); - } - return path; + return slash ? "/" + path.substring(i) : path.substring(i); } } return (slash ? "/" : ""); @@ -450,30 +493,21 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { * @return {@code true} if the path is invalid, {@code false} otherwise */ protected boolean isInvalidPath(String path) { - if (logger.isTraceEnabled()) { - logger.trace("Applying \"invalid path\" checks to path: " + path); - } if (path.contains("WEB-INF") || path.contains("META-INF")) { - if (logger.isTraceEnabled()) { - logger.trace("Path contains \"WEB-INF\" or \"META-INF\"."); - } + logger.warn("Path contains \"WEB-INF\" or \"META-INF\"."); return true; } if (path.contains(":/")) { String relativePath = (path.charAt(0) == '/' ? path.substring(1) : path); if (ResourceUtils.isUrl(relativePath) || relativePath.startsWith("url:")) { - if (logger.isTraceEnabled()) { - logger.trace("Path represents URL or has \"url:\" prefix."); - } + logger.warn("Path represents URL or has \"url:\" prefix."); return true; } } if (path.contains("..")) { path = StringUtils.cleanPath(path); if (path.contains("../")) { - if (logger.isTraceEnabled()) { - logger.trace("Path contains \"../\" after call to StringUtils#cleanPath."); - } + logger.warn("Path contains \"../\" after call to StringUtils#cleanPath."); return true; } } @@ -506,7 +540,16 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { @Override public String toString() { - return "ResourceWebHandler [locations=" + getLocations() + ", resolvers=" + getResourceResolvers() + "]"; + return "ResourceWebHandler " + formatLocations(); } + private Object formatLocations() { + if (!this.locationValues.isEmpty()) { + return this.locationValues.stream().collect(Collectors.joining("\", \"", "[\"", "\"]")); + } + else if (!this.locations.isEmpty()) { + return "[" + this.locations + "]"; + } + return Collections.emptyList(); + } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java index 03094a56347..8b0faf8422c 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/VersionResourceResolver.java @@ -173,30 +173,20 @@ public class VersionResourceResolver extends AbstractResourceResolver { String candidate = versionStrategy.extractVersion(requestPath); if (StringUtils.isEmpty(candidate)) { - if (logger.isTraceEnabled()) { - logger.trace("No version found in path \"" + requestPath + "\""); - } return Mono.empty(); } String simplePath = versionStrategy.removeVersion(requestPath, candidate); - if (logger.isTraceEnabled()) { - logger.trace("Extracted version from path, re-resolving without version: \"" + simplePath + "\""); - } - return chain.resolveResource(exchange, simplePath, locations) .filterWhen(resource -> versionStrategy.getResourceVersion(resource) .map(actual -> { if (candidate.equals(actual)) { - if (logger.isTraceEnabled()) { - logger.trace("Resource matches extracted version [" + candidate + "]"); - } return true; } else { if (logger.isTraceEnabled()) { - logger.trace("Potential resource found for \"" + requestPath + "\", " + - "but version [" + candidate + "] does not match"); + logger.trace("Found resource for \"" + requestPath + "\", but version [" + + candidate + "] does not match"); } return false; } @@ -215,16 +205,9 @@ public class VersionResourceResolver extends AbstractResourceResolver { if (strategy == null) { return Mono.just(baseUrl); } - if (logger.isTraceEnabled()) { - logger.trace("Getting the original resource to determine version " + - "for path \"" + resourceUrlPath + "\""); - } return chain.resolveResource(null, baseUrl, locations) .flatMap(resource -> strategy.getResourceVersion(resource) .map(version -> { - if (logger.isTraceEnabled()) { - logger.trace("Determined version [" + version + "] for " + resource); - } return strategy.addVersion(baseUrl, version); })); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/WebJarsResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/WebJarsResourceResolver.java index de6fd3be4e6..0becb9500d9 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/WebJarsResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/WebJarsResourceResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -18,7 +18,6 @@ package org.springframework.web.reactive.resource; import java.util.List; -import org.webjars.MultipleMatchesException; import org.webjars.WebJarAssetLocator; import reactor.core.publisher.Mono; @@ -105,26 +104,16 @@ public class WebJarsResourceResolver extends AbstractResourceResolver { @Nullable protected String findWebJarResourcePath(String path) { - try { - int startOffset = (path.startsWith("/") ? 1 : 0); - int endOffset = path.indexOf('/', 1); - if (endOffset != -1) { - String webjar = path.substring(startOffset, endOffset); - String partialPath = path.substring(endOffset); - String webJarPath = webJarAssetLocator.getFullPath(webjar, partialPath); + int startOffset = (path.startsWith("/") ? 1 : 0); + int endOffset = path.indexOf('/', 1); + if (endOffset != -1) { + String webjar = path.substring(startOffset, endOffset); + String partialPath = path.substring(endOffset + 1); + String webJarPath = webJarAssetLocator.getFullPathExact(webjar, partialPath); + if (webJarPath != null) { return webJarPath.substring(WEBJARS_LOCATION_LENGTH); } } - catch (MultipleMatchesException ex) { - if (logger.isWarnEnabled()) { - logger.warn("WebJar version conflict for \"" + path + "\"", ex); - } - } - catch (IllegalArgumentException ex) { - if (logger.isTraceEnabled()) { - logger.trace("No WebJar resource found for \"" + path + "\""); - } - } return null; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/HandlerResultHandlerSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/HandlerResultHandlerSupport.java index 506a1ff201a..b2d9288d2aa 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/HandlerResultHandlerSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/HandlerResultHandlerSupport.java @@ -23,6 +23,9 @@ import java.util.List; import java.util.Set; import java.util.function.Supplier; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.core.Ordered; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; @@ -47,6 +50,8 @@ public abstract class HandlerResultHandlerSupport implements Ordered { private static final MediaType MEDIA_TYPE_APPLICATION_ALL = new MediaType("application"); + protected final Log logger = LogFactory.getLog(getClass()); + private final RequestedContentTypeResolver contentTypeResolver; private final ReactiveAdapterRegistry adapterRegistry; @@ -117,6 +122,9 @@ public abstract class HandlerResultHandlerSupport implements Ordered { MediaType contentType = exchange.getResponse().getHeaders().getContentType(); if (contentType != null && contentType.isConcrete()) { + if (logger.isDebugEnabled()) { + logger.debug("Found 'Content-Type:" + contentType + "' in response"); + } return contentType; } @@ -137,13 +145,24 @@ public abstract class HandlerResultHandlerSupport implements Ordered { for (MediaType mediaType : result) { if (mediaType.isConcrete()) { + if (logger.isDebugEnabled()) { + logger.debug("Using '" + mediaType + "' given " + acceptableTypes); + } return mediaType; } else if (mediaType.equals(MediaType.ALL) || mediaType.equals(MEDIA_TYPE_APPLICATION_ALL)) { - return MediaType.APPLICATION_OCTET_STREAM; + mediaType = MediaType.APPLICATION_OCTET_STREAM; + if (logger.isDebugEnabled()) { + logger.debug("Using '" + mediaType + "' given " + acceptableTypes); + } + return mediaType; } } + if (logger.isDebugEnabled()) { + logger.debug("No match for " + acceptableTypes + ", supported: " + producibleTypes); + } + return null; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java index 8efebf79335..b996aa6fa71 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java @@ -34,6 +34,7 @@ import reactor.core.publisher.Mono; import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.InitializingBean; import org.springframework.core.MethodIntrospector; +import org.springframework.http.server.RequestPath; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -122,6 +123,9 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap * @param method the method */ public void registerMapping(T mapping, Object handler, Method method) { + if (logger.isTraceEnabled()) { + logger.trace("Register \"" + mapping + "\" to " + method.toGenericString()); + } this.mappingRegistry.register(mapping, handler, method); } @@ -131,6 +135,9 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap * @param mapping the mapping to unregister */ public void unregisterMapping(T mapping) { + if (logger.isTraceEnabled()) { + logger.trace("Unregister mapping \"" + mapping); + } this.mappingRegistry.unregister(mapping); } @@ -142,7 +149,15 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap */ @Override public void afterPropertiesSet() { + initHandlerMethods(); + + // Total includes detected mappings + explicit registrations via registerMapping.. + int total = this.getHandlerMethods().size(); + + if ((logger.isTraceEnabled() && total == 0) || (logger.isDebugEnabled() && total > 0) ) { + logger.debug(total + " mappings in " + formatMappingName()); + } } /** @@ -152,9 +167,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap * @see #handlerMethodsInitialized(Map) */ protected void initHandlerMethods() { - if (logger.isDebugEnabled()) { - logger.debug("Looking for request mappings in application context: " + getApplicationContext()); - } String[] beanNames = obtainApplicationContext().getBeanNamesForType(Object.class); for (String beanName : beanNames) { @@ -165,8 +177,8 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } catch (Throwable ex) { // An unresolvable bean type, probably from a lazy bean - let's ignore it. - if (logger.isDebugEnabled()) { - logger.debug("Could not resolve target class for bean with name '" + beanName + "'", ex); + if (logger.isTraceEnabled()) { + logger.trace("Could not resolve type for bean '" + beanName + "'", ex); } } if (beanType != null && isHandler(beanType)) { @@ -189,8 +201,8 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap final Class userType = ClassUtils.getUserClass(handlerType); Map methods = MethodIntrospector.selectMethods(userType, (MethodIntrospector.MetadataLookup) method -> getMappingForMethod(method, userType)); - if (logger.isDebugEnabled()) { - logger.debug(methods.size() + " request handler methods found on " + userType + ": " + methods); + if (logger.isTraceEnabled()) { + logger.trace("Mapped " + methods.size() + " handler method(s) for " + userType + ": " + methods); } methods.forEach((key, mapping) -> { Method invocableMethod = AopUtils.selectInvocableMethod(key, userType); @@ -255,10 +267,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap */ @Override public Mono getHandlerInternal(ServerWebExchange exchange) { - if (logger.isDebugEnabled()) { - logger.debug("Looking up handler method for path " + - exchange.getRequest().getPath().value()); - } this.mappingRegistry.acquireReadLock(); try { HandlerMethod handlerMethod; @@ -268,15 +276,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap catch (Exception ex) { return Mono.error(ex); } - if (logger.isDebugEnabled()) { - if (handlerMethod != null) { - logger.debug("Returning handler method [" + handlerMethod + "]"); - } - else { - logger.debug("Did not find handler method for " + - "[" + exchange.getRequest().getPath().value() + "]"); - } - } if (handlerMethod != null) { handlerMethod = handlerMethod.createWithResolvedBean(); } @@ -303,12 +302,11 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap if (!matches.isEmpty()) { Comparator comparator = new MatchComparator(getMappingComparator(exchange)); matches.sort(comparator); - if (logger.isTraceEnabled()) { - logger.trace("Found " + matches.size() + " matching mapping(s) for [" + - exchange.getRequest().getPath() + "] : " + matches); - } Match bestMatch = matches.get(0); if (matches.size() > 1) { + if (logger.isTraceEnabled()) { + logger.trace(matches.size() + " matching mappings: " + matches); + } if (CorsUtils.isPreFlightRequest(exchange.getRequest())) { return PREFLIGHT_AMBIGUOUS_MATCH; } @@ -316,8 +314,9 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap if (comparator.compare(bestMatch, secondBestMatch) == 0) { Method m1 = bestMatch.handlerMethod.getMethod(); Method m2 = secondBestMatch.handlerMethod.getMethod(); - throw new IllegalStateException("Ambiguous handler methods mapped for HTTP path '" + - exchange.getRequest().getPath() + "': {" + m1 + ", " + m2 + "}"); + RequestPath path = exchange.getRequest().getPath(); + throw new IllegalStateException( + "Ambiguous handler methods mapped for '" + path + "': {" + m1 + ", " + m2 + "}"); } } handleMatch(bestMatch.mapping, bestMatch.handlerMethod, exchange); @@ -464,9 +463,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap HandlerMethod handlerMethod = createHandlerMethod(handler, method); assertUniqueMethodMapping(handlerMethod, mapping); - if (logger.isInfoEnabled()) { - logger.info("Mapped \"" + mapping + "\" onto " + handlerMethod); - } this.mappingLookup.put(mapping, handlerMethod); CorsConfiguration corsConfig = initCorsConfiguration(handler, method, mapping); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/HandlerMethodArgumentResolverSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/HandlerMethodArgumentResolverSupport.java index db3f60344cd..ba0dd4edf39 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/HandlerMethodArgumentResolverSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/HandlerMethodArgumentResolverSupport.java @@ -20,6 +20,9 @@ import java.lang.annotation.Annotation; import java.util.function.BiPredicate; import java.util.function.Predicate; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.core.MethodParameter; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; @@ -35,6 +38,8 @@ import org.springframework.util.Assert; */ public abstract class HandlerMethodArgumentResolverSupport implements HandlerMethodArgumentResolver { + protected final Log logger = LogFactory.getLog(getClass()); + private final ReactiveAdapterRegistry adapterRegistry; diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java index f043ab9c9ac..43f8436019f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -38,9 +38,9 @@ import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.http.HttpStatus; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.lang.Nullable; -import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; +import org.springframework.util.StringUtils; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.BindingContext; import org.springframework.web.reactive.HandlerResult; @@ -134,31 +134,38 @@ public class InvocableHandlerMethod extends HandlerMethod { Object... providedArgs) { return resolveArguments(exchange, bindingContext, providedArgs).flatMap(args -> { + Object value; try { - Object value = doInvoke(args); - - HttpStatus status = getResponseStatus(); - if (status != null) { - exchange.getResponse().setStatusCode(status); - } - - MethodParameter returnType = getReturnType(); - ReactiveAdapter adapter = this.reactiveAdapterRegistry.getAdapter(returnType.getParameterType()); - boolean asyncVoid = isAsyncVoidReturnType(returnType, adapter); - if ((value == null || asyncVoid) && isResponseHandled(args, exchange)) { - logger.debug("Response fully handled in controller method"); - return asyncVoid ? Mono.from(adapter.toPublisher(value)) : Mono.empty(); - } - - HandlerResult result = new HandlerResult(this, value, returnType, bindingContext); - return Mono.just(result); + ReflectionUtils.makeAccessible(getBridgedMethod()); + value = getBridgedMethod().invoke(getBean(), args); + } + catch (IllegalArgumentException ex) { + assertTargetBean(getBridgedMethod(), getBean(), args); + String text = (ex.getMessage() != null ? ex.getMessage() : "Illegal argument"); + throw new IllegalStateException(formatInvokeError(text, args), ex); } catch (InvocationTargetException ex) { return Mono.error(ex.getTargetException()); } catch (Throwable ex) { - return Mono.error(new IllegalStateException(getInvocationErrorMessage(args))); + // Unlikely to ever get here, but it must be handled... + return Mono.error(new IllegalStateException(formatInvokeError("Invocation failure", args), ex)); + } + + HttpStatus status = getResponseStatus(); + if (status != null) { + exchange.getResponse().setStatusCode(status); + } + + MethodParameter returnType = getReturnType(); + ReactiveAdapter adapter = this.reactiveAdapterRegistry.getAdapter(returnType.getParameterType()); + boolean asyncVoid = isAsyncVoidReturnType(returnType, adapter); + if ((value == null || asyncVoid) && isResponseHandled(args, exchange)) { + return asyncVoid ? Mono.from(adapter.toPublisher(value)) : Mono.empty(); } + + HandlerResult result = new HandlerResult(this, value, returnType, bindingContext); + return Mono.just(result); }); } @@ -203,8 +210,8 @@ public class InvocableHandlerMethod extends HandlerMethod { private HandlerMethodArgumentResolver findResolver(MethodParameter param) { return this.resolvers.stream() .filter(r -> r.supportsParameter(param)) - .findFirst() - .orElseThrow(() -> getArgumentError("No suitable resolver for", param, null)); + .findFirst().orElseThrow(() -> + new IllegalStateException(formatArgumentError(param, "No suitable resolver"))); } private Mono resolveArg(HandlerMethodArgumentResolver resolver, MethodParameter parameter, @@ -213,49 +220,60 @@ public class InvocableHandlerMethod extends HandlerMethod { try { return resolver.resolveArgument(parameter, bindingContext, exchange) .defaultIfEmpty(NO_ARG_VALUE) - .doOnError(cause -> { - if (logger.isDebugEnabled()) { - logger.debug(getDetailedErrorMessage("Failed to resolve", parameter), cause); - } - }); + .doOnError(cause -> logArgumentErrorIfNecessary(parameter, cause)); } catch (Exception ex) { - throw getArgumentError("Failed to resolve", parameter, ex); + logArgumentErrorIfNecessary(parameter, ex); + return Mono.error(ex); } } - private IllegalStateException getArgumentError(String text, MethodParameter parameter, @Nullable Throwable ex) { - return new IllegalStateException(getDetailedErrorMessage(text, parameter), ex); + private void logArgumentErrorIfNecessary(MethodParameter parameter, Throwable cause) { + // Leave stack trace for later, if error is not handled.. + String message = cause.getMessage(); + if (!message.contains(parameter.getExecutable().toGenericString())) { + if (logger.isDebugEnabled()) { + logger.debug(formatArgumentError(parameter, message)); + } + } } - private String getDetailedErrorMessage(String text, MethodParameter param) { - return text + " argument " + param.getParameterIndex() + " of type '" + - param.getParameterType().getName() + "' on " + getBridgedMethod().toGenericString(); + private static String formatArgumentError(MethodParameter param, String message) { + return "Could not resolve parameter [" + param.getParameterIndex() + "] in " + + param.getExecutable().toGenericString() + (StringUtils.hasText(message) ? ": " + message : ""); } - @Nullable - private Object doInvoke(Object[] args) throws Exception { - if (logger.isTraceEnabled()) { - logger.trace("Invoking '" + ClassUtils.getQualifiedMethodName(getMethod(), getBeanType()) + - "' with arguments " + Arrays.toString(args)); - } - ReflectionUtils.makeAccessible(getBridgedMethod()); - Object returnValue = getBridgedMethod().invoke(getBean(), args); - if (logger.isTraceEnabled()) { - logger.trace("Method [" + ClassUtils.getQualifiedMethodName(getMethod(), getBeanType()) + - "] returned [" + returnValue + "]"); + /** + * Assert that the target bean class is an instance of the class where the given + * method is declared. In some cases the actual controller instance at request- + * processing time may be a JDK dynamic proxy (lazy initialization, prototype + * beans, and others). {@code @Controller}'s that require proxying should prefer + * class-based proxy mechanisms. + */ + private void assertTargetBean(Method method, Object targetBean, Object[] args) { + Class methodDeclaringClass = method.getDeclaringClass(); + Class targetBeanClass = targetBean.getClass(); + if (!methodDeclaringClass.isAssignableFrom(targetBeanClass)) { + String text = "The mapped handler method class '" + methodDeclaringClass.getName() + + "' is not an instance of the actual controller bean class '" + + targetBeanClass.getName() + "'. If the controller requires proxying " + + "(e.g. due to @Transactional), please use class-based proxying."; + throw new IllegalStateException(formatInvokeError(text, args)); } - return returnValue; } - private String getInvocationErrorMessage(Object[] args) { - String argumentDetails = IntStream.range(0, args.length) + private String formatInvokeError(String text, Object[] args) { + + String formattedArgs = IntStream.range(0, args.length) .mapToObj(i -> (args[i] != null ? - "[" + i + "][type=" + args[i].getClass().getName() + "][value=" + args[i] + "]" : - "[" + i + "][null]")) - .collect(Collectors.joining(",", " ", " ")); - return "Failed to invoke handler method with resolved arguments:" + argumentDetails + - "on " + getBridgedMethod().toGenericString(); + "[" + i + "] [type=" + args[i].getClass().getName() + "] [value=" + args[i] + "]" : + "[" + i + "] [null]")) + .collect(Collectors.joining(",\n", " ", " ")); + + return text + "\n" + + "Controller [" + getBeanType().getName() + "]\n" + + "Method [" + getBridgedMethod().toGenericString() + "]\n" + + "with argument values:\n" + formattedArgs; } private boolean isAsyncVoidReturnType(MethodParameter returnType, diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageReaderArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageReaderArgumentResolver.java index 637a1b31d4d..2ac34b7b7ef 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageReaderArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageReaderArgumentResolver.java @@ -152,10 +152,18 @@ public abstract class AbstractMessageReaderArgumentResolver extends HandlerMetho MediaType contentType = request.getHeaders().getContentType(); MediaType mediaType = (contentType != null ? contentType : MediaType.APPLICATION_OCTET_STREAM); + if (logger.isDebugEnabled()) { + logger.debug(contentType != null ? "Content-Type:" + contentType : + "No Content-Type, using " + MediaType.APPLICATION_OCTET_STREAM); + } + for (HttpMessageReader reader : getMessageReaders()) { if (reader.canRead(elementType, mediaType)) { Map readHints = Collections.emptyMap(); if (adapter != null && adapter.isMultiValue()) { + if (logger.isDebugEnabled()) { + logger.debug("0..N [" + elementType + "]"); + } Flux flux = reader.read(actualType, elementType, request, response, readHints); flux = flux.onErrorResume(ex -> Flux.error(handleReadError(bodyParam, ex))); if (isBodyRequired) { @@ -170,6 +178,9 @@ public abstract class AbstractMessageReaderArgumentResolver extends HandlerMetho } else { // Single-value (with or without reactive type wrapper) + if (logger.isDebugEnabled()) { + logger.debug("0..1 [" + elementType + "]"); + } Mono mono = reader.readMono(actualType, elementType, request, response, readHints); mono = mono.onErrorResume(ex -> Mono.error(handleReadError(bodyParam, ex))); if (isBodyRequired) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java index 42b7436807c..f4f90ac3465 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java @@ -141,6 +141,9 @@ public abstract class AbstractMessageWriterResultHandler extends HandlerResultHa ServerHttpResponse response = exchange.getResponse(); MediaType bestMediaType = selectMediaType(exchange, () -> getMediaTypesFor(elementType)); if (bestMediaType != null) { + if (logger.isDebugEnabled()) { + logger.debug((publisher instanceof Mono ? "0..1" : "0..N") + " [" + elementType + "]"); + } for (HttpMessageWriter writer : getMessageWriters()) { if (writer.canWrite(elementType, bestMediaType)) { return writer.write((Publisher) publisher, actualType, elementType, diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java index 00bc01eb779..80854ef2c92 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java @@ -205,10 +205,6 @@ class ControllerMethodResolver { private void initControllerAdviceCaches(ApplicationContext applicationContext) { - if (logger.isInfoEnabled()) { - logger.info("Looking for @ControllerAdvice: " + applicationContext); - } - List beans = ControllerAdviceBean.findAnnotatedBeans(applicationContext); AnnotationAwareOrderComparator.sort(beans); @@ -218,26 +214,30 @@ class ControllerMethodResolver { Set attrMethods = selectMethods(beanType, ATTRIBUTE_METHODS); if (!attrMethods.isEmpty()) { this.modelAttributeAdviceCache.put(bean, attrMethods); - if (logger.isInfoEnabled()) { - logger.info("Detected @ModelAttribute methods in " + bean); - } } Set binderMethods = selectMethods(beanType, BINDER_METHODS); if (!binderMethods.isEmpty()) { this.initBinderAdviceCache.put(bean, binderMethods); - if (logger.isInfoEnabled()) { - logger.info("Detected @InitBinder methods in " + bean); - } } ExceptionHandlerMethodResolver resolver = new ExceptionHandlerMethodResolver(beanType); if (resolver.hasExceptionMappings()) { this.exceptionHandlerAdviceCache.put(bean, resolver); - if (logger.isInfoEnabled()) { - logger.info("Detected @ExceptionHandler methods in " + bean); - } } } } + + if (logger.isDebugEnabled()) { + int modelSize = this.modelAttributeAdviceCache.size(); + int binderSize = this.initBinderAdviceCache.size(); + int handlerSize = this.exceptionHandlerAdviceCache.size(); + if (modelSize == 0 && binderSize == 0 && handlerSize == 0) { + logger.debug("ControllerAdvice beans: none"); + } + else { + logger.debug("ControllerAdvice beans: " + modelSize + " @ModelAttribute, " + binderSize + + " @InitBinder, " + handlerSize + " @ExceptionHandler"); + } + } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java index d8081fed40f..6a1c5f70001 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java @@ -214,7 +214,7 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, Application if (invocable != null) { try { if (logger.isDebugEnabled()) { - logger.debug("Invoking @ExceptionHandler method: " + invocable.getMethod()); + logger.debug("Using @ExceptionHandler " + invocable); } bindingContext.getModel().asMap().clear(); Throwable cause = exception.getCause(); @@ -227,7 +227,7 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, Application } catch (Throwable invocationEx) { if (logger.isWarnEnabled()) { - logger.warn("Failed to invoke: " + invocable.getMethod(), invocationEx); + logger.warn("Failure in @ExceptionHandler " + invocable, invocationEx); } } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractView.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractView.java index 5841b899a9a..bf0c1e9f8d8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractView.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/AbstractView.java @@ -29,6 +29,7 @@ import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import org.springframework.beans.factory.BeanNameAware; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.core.ReactiveAdapter; @@ -44,7 +45,7 @@ import org.springframework.web.server.ServerWebExchange; * @author Rossen Stoyanchev * @since 5.0 */ -public abstract class AbstractView implements View, ApplicationContextAware { +public abstract class AbstractView implements View, BeanNameAware, ApplicationContextAware { /** Well-known name for the RequestDataValueProcessor in the bean factory */ public static final String REQUEST_DATA_VALUE_PROCESSOR_BEAN_NAME = "requestDataValueProcessor"; @@ -65,6 +66,9 @@ public abstract class AbstractView implements View, ApplicationContextAware { @Nullable private String requestContextAttribute; + @Nullable + private String beanName; + @Nullable private ApplicationContext applicationContext; @@ -131,6 +135,24 @@ public abstract class AbstractView implements View, ApplicationContextAware { return this.requestContextAttribute; } + /** + * Set the view's name. Helpful for traceability. + *

Framework code must call this when constructing views. + */ + @Override + public void setBeanName(@Nullable String beanName) { + this.beanName = beanName; + } + + /** + * Return the view's name. Should never be {@code null}, if the view was + * correctly configured. + */ + @Nullable + public String getBeanName() { + return this.beanName; + } + @Override public void setApplicationContext(@Nullable ApplicationContext applicationContext) { this.applicationContext = applicationContext; @@ -166,8 +188,9 @@ public abstract class AbstractView implements View, ApplicationContextAware { public Mono render(@Nullable Map model, @Nullable MediaType contentType, ServerWebExchange exchange) { - if (logger.isTraceEnabled()) { - logger.trace("Rendering view with model " + model); + if (logger.isDebugEnabled()) { + logger.debug("View " + formatViewName() + + ", model " + (model != null ? model : Collections.emptyMap())); } if (contentType != null) { @@ -298,7 +321,12 @@ public abstract class AbstractView implements View, ApplicationContextAware { @Override public String toString() { - return getClass().getName(); + return getClass().getName() + ": " + formatViewName(); + } + + protected String formatViewName() { + return (getBeanName() != null ? + "name '" + getBeanName() + "'" : "[" + getClass().getSimpleName() + "]"); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/HttpMessageWriterView.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/HttpMessageWriterView.java index 1a7cbe62f24..c182e3f561f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/HttpMessageWriterView.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/HttpMessageWriterView.java @@ -23,6 +23,8 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; import reactor.core.publisher.Mono; @@ -43,6 +45,9 @@ import org.springframework.web.server.ServerWebExchange; */ public class HttpMessageWriterView implements View { + private static final Log logger = LogFactory.getLog(HttpMessageWriter.class); + + private final HttpMessageWriter writer; private final Set modelKeys = new HashSet<>(4); @@ -118,8 +123,7 @@ public class HttpMessageWriterView implements View { Object value = getObjectToRender(model); return (value != null) ? - write(value, contentType, exchange) : - exchange.getResponse().setComplete(); + write(value, contentType, exchange) : exchange.getResponse().setComplete(); } @Nullable diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerView.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerView.java index ba7f0aab3f6..88e51884756 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerView.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerView.java @@ -167,9 +167,7 @@ public class FreeMarkerView extends AbstractUrlBasedView { return true; } catch (FileNotFoundException ex) { - if (logger.isDebugEnabled()) { - logger.debug("No FreeMarker view found for URL: " + getUrl()); - } + // Allow for ViewResolver chaining... return false; } catch (ParseException ex) { @@ -188,8 +186,9 @@ public class FreeMarkerView extends AbstractUrlBasedView { // Expose all standard FreeMarker hash models. SimpleHash freeMarkerModel = getTemplateModel(renderAttributes, exchange); + if (logger.isDebugEnabled()) { - logger.debug("Rendering FreeMarker template [" + getUrl() + "]."); + logger.debug("Rendering [" + getUrl() + "]"); } Locale locale = LocaleContextHolder.getLocale(exchange.getLocaleContext()); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/AbstractListenerWebSocketSession.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/AbstractListenerWebSocketSession.java index 0ea78234a20..2df51138682 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/AbstractListenerWebSocketSession.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/AbstractListenerWebSocketSession.java @@ -227,11 +227,11 @@ public abstract class AbstractListenerWebSocketSession extends AbstractWebSoc protected void checkOnDataAvailable() { resumeReceiving(); if (!this.pendingMessages.isEmpty()) { - logger.trace("checkOnDataAvailable, processing pending messages"); + logger.trace("checkOnDataAvailable, " + this.pendingMessages.size() + " pending messages"); onDataAvailable(); } else { - logger.trace("checkOnDataAvailable, no pending messages"); + logger.trace("checkOnDataAvailable, 0 pending messages"); } } @@ -248,11 +248,11 @@ public abstract class AbstractListenerWebSocketSession extends AbstractWebSoc void handleMessage(WebSocketMessage message) { if (logger.isTraceEnabled()) { - logger.trace("Received message: " + message); + logger.trace("Received " + message); } if (!this.pendingMessages.offer(message)) { - throw new IllegalStateException("Too many messages received. " + - "Please ensure WebSocketSession.receive() is subscribed to."); + throw new IllegalStateException( + "Too many messages. Please ensure WebSocketSession.receive() is subscribed to."); } onDataAvailable(); } @@ -266,7 +266,7 @@ public abstract class AbstractListenerWebSocketSession extends AbstractWebSoc @Override protected boolean write(WebSocketMessage message) throws IOException { if (logger.isTraceEnabled()) { - logger.trace("Sending message " + message); + logger.trace("Sending " + message); } return sendMessage(message); } @@ -288,7 +288,7 @@ public abstract class AbstractListenerWebSocketSession extends AbstractWebSoc */ public void setReadyToSend(boolean ready) { if (ready) { - logger.trace("Send succeeded, ready to send again"); + logger.trace("Ready to send again"); } this.isReady = ready; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/WebSocketClientSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/WebSocketClientSupport.java index dfdb36f2949..fa3c8f5d3b8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/WebSocketClientSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/client/WebSocketClientSupport.java @@ -43,14 +43,14 @@ public class WebSocketClientSupport { protected List beforeHandshake(URI url, HttpHeaders requestHeaders, WebSocketHandler handler) { if (logger.isDebugEnabled()) { - logger.debug("Executing handshake to " + url); + logger.debug("Connecting to " + url); } return handler.getSubProtocols(); } protected HandshakeInfo afterHandshake(URI url, HttpHeaders responseHeaders) { if (logger.isDebugEnabled()) { - logger.debug("Handshake response: " + url + ", " + responseHeaders); + logger.debug("Connected to " + url + ", " + responseHeaders); } String protocol = responseHeaders.getFirst(SEC_WEBSOCKET_PROTOCOL); return new HandshakeInfo(url, responseHeaders, Mono.empty(), protocol); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java index 91910ade4d4..4e712bf5d04 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java @@ -208,10 +208,6 @@ public class HandshakeWebSocketService implements WebSocketService, Lifecycle { HttpMethod method = request.getMethod(); HttpHeaders headers = request.getHeaders(); - if (logger.isDebugEnabled()) { - logger.debug("Handling " + request.getURI() + " with headers: " + headers); - } - if (HttpMethod.GET != method) { return Mono.error(new MethodNotAllowedException( request.getMethodValue(), Collections.singleton(HttpMethod.GET))); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java index 24c123dd29f..6751cdad483 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java @@ -89,7 +89,7 @@ public class DispatcherHandlerErrorTests { .consumeErrorWith(error -> { assertThat(error, instanceOf(ResponseStatusException.class)); assertThat(error.getMessage(), - is("Response status 404 NOT_FOUND with reason \"No matching handler\"")); + is("404 NOT_FOUND \"No matching handler\"")); }) .verify(); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java index b74fdcd2ab5..322a8779de5 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java @@ -41,10 +41,7 @@ import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.UnsupportedMediaTypeStatusException; import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -160,8 +157,8 @@ public class InvocableHandlerMethodTests { fail("Expected IllegalStateException"); } catch (IllegalStateException ex) { - assertThat(ex.getMessage(), is("No suitable resolver for argument 0 of type 'java.lang.String' " + - "on " + method.toGenericString())); + assertThat(ex.getMessage(), is("Could not resolve parameter [0] in " + + method.toGenericString() + ": No suitable resolver")); } } @@ -176,12 +173,12 @@ public class InvocableHandlerMethodTests { fail("Expected UnsupportedMediaTypeStatusException"); } catch (UnsupportedMediaTypeStatusException ex) { - assertThat(ex.getMessage(), is("Response status 415 UNSUPPORTED_MEDIA_TYPE with reason \"boo\"")); + assertThat(ex.getMessage(), is("415 UNSUPPORTED_MEDIA_TYPE \"boo\"")); } } @Test - public void illegalArgumentExceptionIsWrappedWithInvocationDetails() throws Exception { + public void illegalArgumentException() throws Exception { Mono resolvedValue = Mono.just(1); Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method(); Mono mono = invoke(new TestController(), method, resolverFor(resolvedValue)); @@ -191,9 +188,12 @@ public class InvocableHandlerMethodTests { fail("Expected IllegalStateException"); } catch (IllegalStateException ex) { - assertThat(ex.getMessage(), is("Failed to invoke handler method with resolved arguments: " + - "[0][type=java.lang.Integer][value=1] " + - "on " + method.toGenericString())); + assertNotNull("Exception not wrapped", ex.getCause()); + assertTrue(ex.getCause() instanceof IllegalArgumentException); + assertTrue(ex.getMessage().contains("Controller [")); + assertTrue(ex.getMessage().contains("Method [")); + assertTrue(ex.getMessage().contains("with argument values:")); + assertTrue(ex.getMessage().contains("[0] [type=java.lang.Integer] [value=1]")); } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java index 330274c64f7..96a581279b5 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java @@ -170,7 +170,7 @@ public class RequestMappingInfoHandlerMappingTests { Mono mono = this.handlerMapping.getHandler(exchange); assertError(mono, UnsupportedMediaTypeStatusException.class, - ex -> assertEquals("Response status 415 UNSUPPORTED_MEDIA_TYPE with reason " + + ex -> assertEquals("415 UNSUPPORTED_MEDIA_TYPE " + "\"Invalid mime type \"bogus\": does not contain '/'\"", ex.getMessage())); } diff --git a/spring-webflux/src/test/resources/log4j2-test.xml b/spring-webflux/src/test/resources/log4j2-test.xml index c2d3b8c40f4..2cdd748b9cb 100644 --- a/spring-webflux/src/test/resources/log4j2-test.xml +++ b/spring-webflux/src/test/resources/log4j2-test.xml @@ -6,8 +6,9 @@ + - + 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 8ff57127ab2..d47138d7492 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 @@ -537,9 +537,9 @@ public class DispatcherServlet extends FrameworkServlet { else { logger.warn("\n\n" + "!!!!!!!!!!!!!!!!!!!\n" + - "Logging of request parameters (DEBUG level) and headers (TRACE level) may log sensitive data.\n" + - "If not in development, lower the log level for \"org.springframework.web.servlet.DispatcherServlet\", or\n" + - "set the DispatcherServlet property \"disableLoggingRequestDetails\" to 'true'.\n" + + "Logging request parameters (DEBUG) and headers (TRACE) may show sensitive data.\n" + + "If not in development, use the DispatcherServlet property \"disableLoggingRequestDetails=true\",\n" + + "or lower the log level.\n" + "!!!!!!!!!!!!!!!!!!!\n"); } } @@ -1278,8 +1278,7 @@ public class DispatcherServlet extends FrameworkServlet { */ protected void noHandlerFound(HttpServletRequest request, HttpServletResponse response) throws Exception { if (pageNotFoundLogger.isWarnEnabled()) { - pageNotFoundLogger.warn("No mapping for " + request.getMethod() + " " + - getRequestUri(request) + " in DispatcherServlet '" + getServletName() + "'"); + pageNotFoundLogger.warn("No mapping for " + request.getMethod() + " " + getRequestUri(request)); } if (this.throwExceptionIfNoHandlerFound) { throw new NoHandlerFoundException(request.getMethod(), getRequestUri(request), diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java index 88cfc0fabd0..933d56b7f97 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java @@ -997,9 +997,9 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic if (asyncManager.isConcurrentHandlingStarted()) { logger.debug("Exiting, but response remains open for further handling"); } - else { + else if (logger.isDebugEnabled()) { HttpStatus status = HttpStatus.resolve(response.getStatus()); - this.logger.debug("Completed " + (status != null ? status : response.getStatus())); + logger.debug("Completed " + (status != null ? status : response.getStatus())); } } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerExceptionResolver.java index 9899fc9f253..e1ac6568995 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerExceptionResolver.java @@ -135,10 +135,12 @@ public abstract class AbstractHandlerExceptionResolver implements HandlerExcepti prepareResponse(ex, response); ModelAndView result = doResolveException(request, response, handler, ex); if (result != null) { - // One-liner at debug level.. - if (logger.isDebugEnabled()) { + + // Print debug message, when warn logger is not enabled.. + if (logger.isDebugEnabled() && (this.warnLogger == null || !this.warnLogger.isWarnEnabled())) { logger.debug("Resolved [" + ex + "]" + (result.isEmpty() ? "" : " to " + result)); } + // warnLogger with full stack trace (requires explicit config).. logException(ex, request); } @@ -202,7 +204,7 @@ public abstract class AbstractHandlerExceptionResolver implements HandlerExcepti * @return the log message to use */ protected String buildLogMessage(Exception ex, HttpServletRequest request) { - return "Resolved exception caused by Handler execution: " + ex; + return "Resolved [" + ex + "]"; } /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java index 9e2a4d61656..3f21a073f20 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java @@ -197,10 +197,11 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap initHandlerMethods(); - if (logger.isDebugEnabled()) { - // Total includes detected mappings + explicit registrations via registerMapping.. - int total = this.getHandlerMethods().size(); - logger.debug("Detected " + total + " mappings in " + formatMappingName()); + // Total includes detected mappings + explicit registrations via registerMapping.. + int total = this.getHandlerMethods().size(); + + if ((logger.isTraceEnabled() && total == 0) || (logger.isDebugEnabled() && total > 0) ) { + logger.debug(total + " mappings in " + formatMappingName()); } } @@ -359,7 +360,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap Match bestMatch = matches.get(0); if (matches.size() > 1) { if (logger.isTraceEnabled()) { - logger.trace(matches.size() + " matching mapppings: " + matches); + logger.trace(matches.size() + " matching mappings: " + matches); } if (CorsUtils.isPreFlightRequest(request)) { return PREFLIGHT_AMBIGUOUS_MATCH; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java index 0341d84fb2e..7d83a2cad07 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java @@ -363,7 +363,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping i else { this.handlerMap.put(urlPath, resolvedHandler); if (logger.isTraceEnabled()) { - logger.trace("Mapped URL path [" + urlPath + "] onto " + getHandlerDescription(handler)); + logger.trace("Mapped [" + urlPath + "] onto " + getHandlerDescription(handler)); } } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMapping.java index 5013ce91aea..ed5f8c9eb49 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMapping.java @@ -114,7 +114,7 @@ public class SimpleUrlHandlerMapping extends AbstractUrlHandlerMapping { */ protected void registerHandlers(Map urlMap) throws BeansException { if (urlMap.isEmpty()) { - logger.warn("Neither 'urlMap' nor 'mappings' set, " + formatMappingName()); + logger.trace("No patterns in " + formatMappingName()); } else { urlMap.forEach((url, handler) -> { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index 35f71abb706..eb6d34c4c2a 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -215,6 +215,9 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe MediaType contentType = outputMessage.getHeaders().getContentType(); if (contentType != null && contentType.isConcrete()) { + if (logger.isDebugEnabled()) { + logger.debug("Found 'Content-Type:" + contentType + "' in response"); + } mediaTypesToUse = Collections.singletonList(contentType); } else { @@ -234,6 +237,9 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } } } + if (logger.isDebugEnabled()) { + logger.debug("No match for " + requestedMediaTypes + ", supported: " + producibleMediaTypes); + } if (mediaTypesToUse.isEmpty()) { if (body != null) { throw new HttpMediaTypeNotAcceptableException(producibleMediaTypes); @@ -255,6 +261,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } } + if (logger.isDebugEnabled()) { + logger.debug("Using '" + selectedMediaType + "' given " + mediaTypesToUse); + } + if (selectedMediaType != null) { selectedMediaType = selectedMediaType.removeQualityValue(); for (HttpMessageConverter converter : this.messageConverters) { @@ -267,6 +277,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe (Class>) converter.getClass(), inputMessage, outputMessage); if (body != null) { + if (logger.isDebugEnabled()) { + Object formatted = body instanceof CharSequence ? "\"" + body + "\"" : body; + logger.debug("Writing [" + formatted + "]"); + } addContentDispositionHeader(inputMessage, outputMessage); if (genericConverter != null) { genericConverter.write(body, declaredType, selectedMediaType, outputMessage); @@ -274,9 +288,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe else { ((HttpMessageConverter) converter).write(body, selectedMediaType, outputMessage); } + } + else { if (logger.isDebugEnabled()) { - logger.debug("Written \"" + selectedMediaType + "\" from " + - "[" + (body instanceof CharSequence ? "\"" + body + "\"" : body) + "]"); + logger.debug("Nothing to write: null body"); } } return; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java index 0b19fffd612..297ed6c75da 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java @@ -121,7 +121,7 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod { } catch (Exception ex) { if (logger.isTraceEnabled()) { - logger.trace(formatErrorForReturnValue("Error handling return value", returnValue), ex); + logger.trace(formatErrorForReturnValue(returnValue), ex); } throw ex; } @@ -160,13 +160,10 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod { return webRequest.isNotModified(); } - private String formatErrorForReturnValue(String message, @Nullable Object returnValue) { - StringBuilder sb = new StringBuilder(message); - if (returnValue != null) { - sb.append(" [type=").append(returnValue.getClass().getName()).append("]"); - } - sb.append(" [value=").append(returnValue).append("]"); - return getDetailedErrorMessage(sb.toString()); + private String formatErrorForReturnValue(@Nullable Object returnValue) { + return "Error handling return value=[" + returnValue + "]" + + (returnValue != null ? ", type=" + returnValue.getClass().getName() : "") + + " in " + toString(); } /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CssLinkResourceTransformer.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CssLinkResourceTransformer.java index 5f0483e58ec..29c519522ad 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CssLinkResourceTransformer.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CssLinkResourceTransformer.java @@ -182,8 +182,8 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport { if (content.substring(index, index + 4).equals("url(")) { // Ignore, UrlLinkParser will take care } - else if (logger.isWarnEnabled()) { - logger.warn("Unexpected syntax for @import link at index " + index); + else if (logger.isTraceEnabled()) { + logger.trace("Unexpected syntax for @import link at index " + index); } return index; } 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 8bea6d97fb7..69c3007e813 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 @@ -157,7 +157,15 @@ public class PathResourceResolver extends AbstractResourceResolver { } } catch (IOException ex) { - logger.trace("Failed to get resource, skipping location: " + location, ex); + if (logger.isDebugEnabled() || logger.isTraceEnabled()) { + String error = "Skip location [" + location + "] due to error"; + if (logger.isTraceEnabled()) { + logger.trace(error, ex); + } + else { + logger.debug(error + ": " + ex.getMessage()); + } + } } } return null; @@ -276,9 +284,7 @@ public class PathResourceResolver extends AbstractResourceResolver { try { String decodedPath = URLDecoder.decode(resourcePath, "UTF-8"); if (decodedPath.contains("../") || decodedPath.contains("..\\")) { - if (logger.isTraceEnabled()) { - logger.trace("Resolved resource path contains encoded \"../\" or \"..\\\": " + resourcePath); - } + logger.warn("Resolved resource path contains encoded \"../\" or \"..\\\": " + resourcePath); 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 98ae0cc60a1..bb17ad1bc1a 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 @@ -450,7 +450,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator // For very general mappings (e.g. "/") we need to check 404 first Resource resource = getResource(request); if (resource == null) { - logger.trace("Resource not found"); + logger.debug("Resource not found"); response.sendError(HttpServletResponse.SC_NOT_FOUND); return; } @@ -514,15 +514,9 @@ public class ResourceHttpRequestHandler extends WebContentGenerator path = processPath(path); if (!StringUtils.hasText(path) || isInvalidPath(path)) { - if (logger.isTraceEnabled()) { - logger.trace("Ignoring invalid resource path [" + path + "]"); - } return null; } if (isInvalidEncodedPath(path)) { - if (logger.isTraceEnabled()) { - logger.trace("Ignoring invalid resource path with escape sequences [" + path + "]"); - } return null; } @@ -587,8 +581,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator if (i == 0 || (i == 1 && slash)) { return path; } - path = (slash ? "/" + path.substring(i) : path.substring(i)); - return path; + return (slash ? "/" + path.substring(i) : path.substring(i)); } } return (slash ? "/" : ""); @@ -637,20 +630,20 @@ public class ResourceHttpRequestHandler extends WebContentGenerator */ protected boolean isInvalidPath(String path) { if (path.contains("WEB-INF") || path.contains("META-INF")) { - logger.warn("Path contains \"WEB-INF\" or \"META-INF\"."); + logger.warn("Path with \"WEB-INF\" or \"META-INF\": [" + path + "]"); return true; } if (path.contains(":/")) { String relativePath = (path.charAt(0) == '/' ? path.substring(1) : path); if (ResourceUtils.isUrl(relativePath) || relativePath.startsWith("url:")) { - logger.warn("Path represents URL or has \"url:\" prefix."); + logger.warn("Path represents URL or has \"url:\" prefix: [" + path + "]"); return true; } } if (path.contains("..")) { path = StringUtils.cleanPath(path); if (path.contains("../")) { - logger.warn("Path contains \"../\" after call to StringUtils#cleanPath."); + logger.warn("Invalid Path contains \"../\" after call to StringUtils#cleanPath."); return true; } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractView.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractView.java index 3a34bf1a69d..8d1fa081c42 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractView.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractView.java @@ -497,7 +497,8 @@ public abstract class AbstractView extends WebApplicationObjectSupport implement } protected String formatViewName() { - return (getBeanName() != null ? "name '" + getBeanName() + "'" : "[unnamed]"); + return (getBeanName() != null ? + "name '" + getBeanName() + "'" : "[" + getClass().getSimpleName() + "]"); } }