Reduce logging in WelcomePageHandlerMapping on invalid Accept headers

[jze]

Every now and then I find the InvalidMimeTypeException followed by a stack trace in my logfiles. These are apparently erroneous queries, but in the end they do not bother at all. Therefore, it is annoying that they take up so much space in the log file. You can find one of these error messages below.

I think I found the location where the InvalidMimeTypeException should be handled:

spring-boot/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/WelcomePageHandlerMapping.java

Line 82 in 3a99066

return MediaType.parseMediaTypes(acceptHeader);

This is how the exception could be handled. The the logic is reasonably preserved, an invalid Accept header is treated as not provided:

try {
   return MediaType.parseMediaTypes(acceptHeader);
} catch( InvalidMimeTypeException ignore ) {
   return MEDIA_TYPES_ALL
}

An example error message with shortened stack trace:

11:58:09.133 [http-nio-8080-exec-108] ERROR o.a.c.c.C.[.[.[.[dispatcherServlet] - Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed: org.springframework.http.InvalidMediaTypeException: Invalid mime type "*/*q=0.8": wildcard type is legal only in '*/*' (all mime types)] with root cause
 org.springframework.util.InvalidMimeTypeException: Invalid mime type "*/*q=0.8": wildcard type is legal only in '*/*' (all mime types)
 	at org.springframework.util.MimeTypeUtils.parseMimeTypeInternal(MimeTypeUtils.java:237)
 	at org.springframework.util.ConcurrentLruCache.get(ConcurrentLruCache.java:100)
 	at org.springframework.util.MimeTypeUtils.parseMimeType(MimeTypeUtils.java:213)
 	at org.springframework.http.MediaType.parseMediaType(MediaType.java:739)
 	at org.springframework.http.MediaType.parseMediaTypes(MediaType.java:768)
 	at org.springframework.boot.autoconfigure.web.servlet.WelcomePageHandlerMapping.getAcceptedMediaTypes(WelcomePageHandlerMapping.java:82)
 	at org.springframework.boot.autoconfigure.web.servlet.WelcomePageHandlerMapping.isHtmlTextAccepted(WelcomePageHandlerMapping.java:71)
 	at org.springframework.boot.autoconfigure.web.servlet.WelcomePageHandlerMapping.getHandlerInternal(WelcomePageHandlerMapping.java:67)
 	at org.springframework.web.servlet.handler.AbstractHandlerMapping.getHandler(AbstractHandlerMapping.java:505)

[mhalbritter]

Hello,

I'm not sure that we should completely suppress the error. In your case, the client has sent an erroneous Accept header with value */*q=0.8. I understand that this pollutes your log with errors where you can't do anything to solve them, so I agree that we should do something here. I'm just not sure that ignoring it and defaulting to a MEDIA_TYPES_ALL is the right thing to do.

I'll flag this as team attention to get more feedback from the team.

[wilkinsona]

How about catching the exception and then varying what we log depending on the level of logging that's enabled? Framework does this in a few places. Something like this:

private List<MediaType> getAcceptedMediaTypes(HttpServletRequest request) {
	String acceptHeader = request.getHeader(HttpHeaders.ACCEPT);
	if (StringUtils.hasText(acceptHeader)) {
		try {
			return MediaType.parseMediaTypes(acceptHeader);
		}
		catch (InvalidMediaTypeException ex) {
			logger.warn("Received invalid Accept header. Assuming all media types are accepted",
					logger.isDebugEnabled() ? ex : null);
		}
	}
	return MEDIA_TYPES_ALL;
}

[mhalbritter]

Yes, I think this improves the situation considerably. You will still have something in the log, but it's now a WARN, and the stacktrace is only there if DEBUG is enabled, too.

What do you think, @jze ?

[jze]

That's a good solution for me. Maybe it could help someone if the invalid header value would be included in the warning message.

logger.warn("Received invalid Accept header '" + acceptHeader + "'. Assuming all media types are accepted",
					logger.isDebugEnabled() ? ex : null);

[wilkinsona]

I would prefer not to include the Accept header in the log message. Logging user input brings with it the risk of a log forging attack. It's probably overly cautious as the limited set of characters that can be used in an HTTP header should make forgery impossible but I think the risk still outweighs the benefit.