Skip to content

Commit 8593270

Browse files
committed
Only remove trailing slash from URI value
This commit upgrades the algorithm when trailing slash are to be ignored. Previously a root URI (i.e. "/") would result to to empty string which is an issue for monitoring system that requires tag values to be non empty. If the URI is a single character, the trailing is not applied and "/" is left as is. Closes gh-20536
1 parent 1bf7d25 commit 8593270

File tree

4 files changed

+37
-6
lines changed

4 files changed

+37
-6
lines changed

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/WebFluxTags.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public static Tag uri(ServerWebExchange exchange, boolean ignoreTrailingSlash) {
107107
PathPattern pathPattern = exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
108108
if (pathPattern != null) {
109109
String patternString = pathPattern.getPatternString();
110-
if (ignoreTrailingSlash) {
110+
if (ignoreTrailingSlash && patternString.length() > 1) {
111111
patternString = TRAILING_SLASH_PATTERN.matcher(patternString).replaceAll("");
112112
}
113113
return Tag.of("uri", patternString);

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public static Tag uri(HttpServletRequest request, HttpServletResponse response,
112112
if (request != null) {
113113
String pattern = getMatchingPattern(request);
114114
if (pattern != null) {
115-
if (ignoreTrailingSlash) {
115+
if (ignoreTrailingSlash && pattern.length() > 1) {
116116
pattern = TRAILING_SLASH_PATTERN.matcher(pattern).replaceAll("");
117117
}
118118
return Tag.of("uri", pattern);

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcTagsTests.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -52,12 +52,26 @@ void uriTagIsDataRestsEffectiveRepositoryLookupPathWhenAvailable() {
5252

5353
@Test
5454
void uriTagValueIsBestMatchingPatternWhenAvailable() {
55-
this.request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/spring");
55+
this.request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/spring/");
5656
this.response.setStatus(301);
5757
Tag tag = WebMvcTags.uri(this.request, this.response);
58+
assertThat(tag.getValue()).isEqualTo("/spring/");
59+
}
60+
61+
@Test
62+
void uriTagValueWithBestMatchingPatternAndIgnoreTrailingSlashRemoveTrailingSlash() {
63+
this.request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/spring/");
64+
Tag tag = WebMvcTags.uri(this.request, this.response, true);
5865
assertThat(tag.getValue()).isEqualTo("/spring");
5966
}
6067

68+
@Test
69+
void uriTagValueWithBestMatchingPatternAndIgnoreTrailingSlashKeepSingleSlash() {
70+
this.request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/");
71+
Tag tag = WebMvcTags.uri(this.request, this.response, true);
72+
assertThat(tag.getValue()).isEqualTo("/");
73+
}
74+
6175
@Test
6276
void uriTagValueIsRootWhenRequestHasNoPatternOrPathInfo() {
6377
assertThat(WebMvcTags.uri(this.request, null).getValue()).isEqualTo("root");

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/WebFluxTagsTests.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,13 @@
3939
* @author Brian Clozel
4040
* @author Michael McFadyen
4141
* @author Madhura Bhave
42+
* @author Stephane Nicoll
4243
*/
4344
class WebFluxTagsTests {
4445

4546
private MockServerWebExchange exchange;
4647

47-
private PathPatternParser parser = new PathPatternParser();
48+
private final PathPatternParser parser = new PathPatternParser();
4849

4950
@BeforeEach
5051
void setup() {
@@ -53,12 +54,28 @@ void setup() {
5354

5455
@Test
5556
void uriTagValueIsBestMatchingPatternWhenAvailable() {
56-
this.exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, this.parser.parse("/spring"));
57+
this.exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE,
58+
this.parser.parse("/spring/"));
5759
this.exchange.getResponse().setStatusCode(HttpStatus.MOVED_PERMANENTLY);
5860
Tag tag = WebFluxTags.uri(this.exchange);
61+
assertThat(tag.getValue()).isEqualTo("/spring/");
62+
}
63+
64+
@Test
65+
void uriTagValueWithBestMatchingPatternAndIgnoreTrailingSlashRemoveTrailingSlash() {
66+
this.exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE,
67+
this.parser.parse("/spring/"));
68+
Tag tag = WebFluxTags.uri(this.exchange, true);
5969
assertThat(tag.getValue()).isEqualTo("/spring");
6070
}
6171

72+
@Test
73+
void uriTagValueWithBestMatchingPatternAndIgnoreTrailingSlashKeepSingleSlash() {
74+
this.exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, this.parser.parse("/"));
75+
Tag tag = WebFluxTags.uri(this.exchange, true);
76+
assertThat(tag.getValue()).isEqualTo("/");
77+
}
78+
6279
@Test
6380
void uriTagValueIsRedirectionWhenResponseStatusIs3xx() {
6481
this.exchange.getResponse().setStatusCode(HttpStatus.MOVED_PERMANENTLY);

0 commit comments

Comments
 (0)