From ebb2f70e0b2e8da09769badcb454480a46ef6e35 Mon Sep 17 00:00:00 2001
From: Brian Clozel <bclozel@pivotal.io>
Date: Thu, 29 Mar 2018 20:33:07 +0200
Subject: [PATCH] Make WebMvgTags use matched patterns for HTTP 404

Prior to this commit, `WebMvcTags' would always mark as "NOT_FOUND" or
"REDIRECTION" *any* exchange with responses of 404 and 3xx status, even
if those responses are actually returned by Controller handlers.

This commit checks inverts those checks and first considers if the
"BEST_MATCHING_PATTERN_ATTRIBUTE" request attribute is present and uses
it - then falls back to "NOT_FOUND" and "REDIRECTION" to avoid
cardinality explosion.

Fixes gh-12577
---
 .../metrics/web/servlet/WebMvcTags.java       | 46 ++++++++++++-------
 .../endpoint/web/servlet/WebMvcTagsTests.java | 18 +++++++-
 2 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java
index 9168249fe7..567098980e 100644
--- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java
+++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java
@@ -31,10 +31,16 @@ import org.springframework.web.servlet.HandlerMapping;
  *
  * @author Jon Schneider
  * @author Andy Wilkinson
+ * @author Brian Clozel
  * @since 2.0.0
  */
 public final class WebMvcTags {
 
+	private static final Tag URI_NOT_FOUND = Tag.of("uri", "NOT_FOUND");
+
+	private static final Tag URI_REDIRECTION = Tag.of("uri", "REDIRECTION");
+
+
 	private WebMvcTags() {
 	}
 
@@ -69,21 +75,24 @@ public final class WebMvcTags {
 	 * @return the uri tag derived from the request
 	 */
 	public static Tag uri(HttpServletRequest request, HttpServletResponse response) {
-		if (response != null) {
-			HttpStatus status = extractStatus(response);
-			if (status != null && status.is3xxRedirection()) {
-				return Tag.of("uri", "REDIRECTION");
+		if (request != null) {
+			String pattern = getMatchingPattern(request);
+			if (pattern != null) {
+				return Tag.of("uri", pattern);
 			}
-			if (status != null && status.equals(HttpStatus.NOT_FOUND)) {
-				return Tag.of("uri", "NOT_FOUND");
+			else if (response != null) {
+				HttpStatus status = extractStatus(response);
+				if (status != null && status.is3xxRedirection()) {
+					return URI_REDIRECTION;
+				}
+				if (status != null && status.equals(HttpStatus.NOT_FOUND)) {
+					return URI_NOT_FOUND;
+				}
 			}
+			String pathInfo = getPathInfo(request);
+			return Tag.of("uri", pathInfo.isEmpty() ? "root" : pathInfo);
 		}
-		if (request == null) {
-			return Tag.of("uri", "UNKNOWN");
-		}
-		String uri = getUri(request);
-		uri = uri.replaceAll("//+", "/").replaceAll("/$", "");
-		return Tag.of("uri", uri.isEmpty() ? "root" : uri);
+		return Tag.of("uri", "UNKNOWN");
 	}
 
 	private static HttpStatus extractStatus(HttpServletResponse response) {
@@ -95,11 +104,16 @@ public final class WebMvcTags {
 		}
 	}
 
-	private static String getUri(HttpServletRequest request) {
-		String uri = (String) request
+	private static String getMatchingPattern(HttpServletRequest request) {
+		return (String) request
 				.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
-		uri = (uri != null ? uri : request.getPathInfo());
-		return (StringUtils.hasText(uri) ? uri : "/");
+	}
+
+	private static String getPathInfo(HttpServletRequest request) {
+		String uri = StringUtils.hasText(request.getPathInfo()) ?
+				request.getPathInfo() : "/";
+		return uri.replaceAll("//+", "/")
+				.replaceAll("/$", "");
 	}
 
 	/**
diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcTagsTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcTagsTests.java
index 4ed229ad05..9131f04c3c 100644
--- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcTagsTests.java
+++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcTagsTests.java
@@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat;
  * Tests for {@link WebMvcTags}.
  *
  * @author Andy Wilkinson
+ * @author Brian Clozel
  */
 public class WebMvcTagsTests {
 
@@ -39,9 +40,17 @@ public class WebMvcTagsTests {
 
 	@Test
 	public void uriTrailingSlashesAreSuppressed() {
+		this.request.setPathInfo("//spring/");
+		assertThat(WebMvcTags.uri(this.request, null).getValue()).isEqualTo("/spring");
+	}
+
+	@Test
+	public void uriTagValueIsBestMatchingPatternWhenAvailable() {
 		this.request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE,
-				"//foo/");
-		assertThat(WebMvcTags.uri(this.request, null).getValue()).isEqualTo("/foo");
+				"/spring");
+		this.response.setStatus(301);
+		Tag tag = WebMvcTags.uri(this.request, this.response);
+		assertThat(tag.getValue()).isEqualTo("/spring");
 	}
 
 	@Test
@@ -65,4 +74,9 @@ public class WebMvcTagsTests {
 		assertThat(tag.getValue()).isEqualTo("root");
 	}
 
+	@Test
+	public void uriTagIsUnknownWhenRequestIsNull() {
+		Tag tag = WebMvcTags.uri(null, null);
+		assertThat(tag.getValue()).isEqualTo("UNKNOWN");
+	}
 }