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
This commit is contained in:
Stephane Nicoll 2020-03-21 15:11:50 +01:00
parent 1bf7d25141
commit 8593270b5b
4 changed files with 37 additions and 6 deletions

View File

@ -107,7 +107,7 @@ public final class WebFluxTags {
PathPattern pathPattern = exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
if (pathPattern != null) {
String patternString = pathPattern.getPatternString();
if (ignoreTrailingSlash) {
if (ignoreTrailingSlash && patternString.length() > 1) {
patternString = TRAILING_SLASH_PATTERN.matcher(patternString).replaceAll("");
}
return Tag.of("uri", patternString);

View File

@ -112,7 +112,7 @@ public final class WebMvcTags {
if (request != null) {
String pattern = getMatchingPattern(request);
if (pattern != null) {
if (ignoreTrailingSlash) {
if (ignoreTrailingSlash && pattern.length() > 1) {
pattern = TRAILING_SLASH_PATTERN.matcher(pattern).replaceAll("");
}
return Tag.of("uri", pattern);

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2020 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.
@ -52,12 +52,26 @@ class WebMvcTagsTests {
@Test
void uriTagValueIsBestMatchingPatternWhenAvailable() {
this.request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/spring");
this.request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/spring/");
this.response.setStatus(301);
Tag tag = WebMvcTags.uri(this.request, this.response);
assertThat(tag.getValue()).isEqualTo("/spring/");
}
@Test
void uriTagValueWithBestMatchingPatternAndIgnoreTrailingSlashRemoveTrailingSlash() {
this.request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/spring/");
Tag tag = WebMvcTags.uri(this.request, this.response, true);
assertThat(tag.getValue()).isEqualTo("/spring");
}
@Test
void uriTagValueWithBestMatchingPatternAndIgnoreTrailingSlashKeepSingleSlash() {
this.request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/");
Tag tag = WebMvcTags.uri(this.request, this.response, true);
assertThat(tag.getValue()).isEqualTo("/");
}
@Test
void uriTagValueIsRootWhenRequestHasNoPatternOrPathInfo() {
assertThat(WebMvcTags.uri(this.request, null).getValue()).isEqualTo("root");

View File

@ -39,12 +39,13 @@ import static org.mockito.Mockito.mock;
* @author Brian Clozel
* @author Michael McFadyen
* @author Madhura Bhave
* @author Stephane Nicoll
*/
class WebFluxTagsTests {
private MockServerWebExchange exchange;
private PathPatternParser parser = new PathPatternParser();
private final PathPatternParser parser = new PathPatternParser();
@BeforeEach
void setup() {
@ -53,12 +54,28 @@ class WebFluxTagsTests {
@Test
void uriTagValueIsBestMatchingPatternWhenAvailable() {
this.exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, this.parser.parse("/spring"));
this.exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE,
this.parser.parse("/spring/"));
this.exchange.getResponse().setStatusCode(HttpStatus.MOVED_PERMANENTLY);
Tag tag = WebFluxTags.uri(this.exchange);
assertThat(tag.getValue()).isEqualTo("/spring/");
}
@Test
void uriTagValueWithBestMatchingPatternAndIgnoreTrailingSlashRemoveTrailingSlash() {
this.exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE,
this.parser.parse("/spring/"));
Tag tag = WebFluxTags.uri(this.exchange, true);
assertThat(tag.getValue()).isEqualTo("/spring");
}
@Test
void uriTagValueWithBestMatchingPatternAndIgnoreTrailingSlashKeepSingleSlash() {
this.exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, this.parser.parse("/"));
Tag tag = WebFluxTags.uri(this.exchange, true);
assertThat(tag.getValue()).isEqualTo("/");
}
@Test
void uriTagValueIsRedirectionWhenResponseStatusIs3xx() {
this.exchange.getResponse().setStatusCode(HttpStatus.MOVED_PERMANENTLY);