Skip to content

Commit

Permalink
Fix an off-by-one bug in ForwardedHeaderTransformer
Browse files Browse the repository at this point in the history
Consistently with what is done in ForwardedHeaderFilter.

Closes spring-projectsgh-33465
  • Loading branch information
sdeleuze committed Sep 4, 2024
1 parent 83be0f2 commit 4b7292f
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 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.
Expand Down Expand Up @@ -53,6 +53,7 @@
* in which case it removes but does not use the headers.
*
* @author Rossen Stoyanchev
* @author Sebastien Deleuze
* @since 5.1
* @see <a href="https://tools.ietf.org/html/rfc7239">https://tools.ietf.org/html/rfc7239</a>
* @see <a href="https://docs.spring.io/spring-framework/reference/web/webflux/reactive-spring.html#webflux-forwarded-headers">Forwarded Headers</a>
Expand Down Expand Up @@ -165,7 +166,7 @@ private static String getForwardedPrefix(ServerHttpRequest request) {
String[] rawPrefixes = StringUtils.tokenizeToStringArray(header, ",");
for (String rawPrefix : rawPrefixes) {
int endIndex = rawPrefix.length();
while (endIndex > 1 && rawPrefix.charAt(endIndex - 1) == '/') {
while (endIndex > 0 && rawPrefix.charAt(endIndex - 1) == '/') {
endIndex--;
}
prefix.append((endIndex != rawPrefix.length() ? rawPrefix.substring(0, endIndex) : rawPrefix));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
* @author Eddú Meléndez
* @author Rob Winch
* @author Brian Clozel
* @author Sebastien Deleuze
*/
class ForwardedHeaderFilterTests {

Expand Down Expand Up @@ -442,6 +443,15 @@ void shouldConcatenatePrefixesWithTrailingSlashes() throws Exception {
assertThat(actual.getRequestURL().toString()).isEqualTo("http://localhost/first/second/mvc-showcase");
}

@Test
void shouldRemoveSingleTrailingSlash() throws Exception {
request.addHeader(X_FORWARDED_PREFIX, "/prefix,/");
request.setRequestURI("/mvc-showcase");

HttpServletRequest actual = filterAndGetWrappedRequest();
assertThat(actual.getRequestURL().toString()).isEqualTo("http://localhost/prefix/mvc-showcase");
}

@Test
void requestURLNewStringBuffer() throws Exception {
request.addHeader(X_FORWARDED_PREFIX, "/prefix/");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
* Tests for {@link ForwardedHeaderTransformer}.
*
* @author Rossen Stoyanchev
* @author Sebastien Deleuze
*/
class ForwardedHeaderTransformerTests {

Expand Down Expand Up @@ -170,6 +171,17 @@ void shouldConcatenatePrefixesWithTrailingSlashes() {
assertForwardedHeadersRemoved(request);
}

@Test // gh-33465
void shouldRemoveSingleTrailingSlash() {
HttpHeaders headers = new HttpHeaders();
headers.add("X-Forwarded-Prefix", "/prefix,/");
ServerHttpRequest request = this.requestMutator.apply(getRequest(headers));

assertThat(request.getURI()).isEqualTo(URI.create("https://example.com/prefix/path"));
assertThat(request.getPath().value()).isEqualTo("/prefix/path");
assertForwardedHeadersRemoved(request);
}

@Test
void forwardedForNotPresent() {
HttpHeaders headers = new HttpHeaders();
Expand Down

0 comments on commit 4b7292f

Please sign in to comment.