#757 - Centralize all Forward header handling.

Spring 5.1 is centralizing all Forward header handling. This moves critical bits into one location, making it easy to completely remove once we re-baseline against this version.

Also adds a test profile to ensure Spring 5.1 doesn't break anything.

Original pull request: #717.
This commit is contained in:
Greg Turnquist
2018-05-22 14:04:09 -05:00
committed by Oliver Drotbohm
parent 9147b327f4
commit d885cb01dd
6 changed files with 136 additions and 20 deletions

View File

@@ -7,6 +7,7 @@ env:
- PROFILE=spring43-next
- PROFILE=spring5
- PROFILE=spring5-next
- PROFILE=spring51-next
addons:
apt:
packages:

14
pom.xml
View File

@@ -125,6 +125,20 @@
</repositories>
</profile>
<profile>
<id>spring51-next</id>
<properties>
<spring.version>5.1.0.BUILD-SNAPSHOT</spring.version>
<jackson.version>2.9.2</jackson.version>
</properties>
<repositories>
<repository>
<id>spring-libs-snapshot</id>
<url>http://repo.spring.io/libs-snapshot</url>
</repository>
</repositories>
</profile>
<profile>
<!-- Profile to be run on the CI server, JARs JavaDocs -->

View File

@@ -15,7 +15,7 @@
*/
package org.springframework.hateoas.mvc;
import static org.springframework.util.StringUtils.*;
import static org.springframework.hateoas.mvc.ForwardedHeader.*;
import lombok.RequiredArgsConstructor;
import lombok.experimental.Delegate;
@@ -26,6 +26,7 @@ import java.util.Map;
import javax.servlet.http.HttpServletRequest;
import org.springframework.context.ApplicationContext;
import org.springframework.hateoas.Link;
import org.springframework.hateoas.TemplateVariables;
import org.springframework.hateoas.core.AnnotationMappingDiscoverer;
@@ -269,20 +270,30 @@ public class ControllerLinkBuilder extends LinkBuilderSupport<ControllerLinkBuil
}
HttpServletRequest request = getCurrentRequest();
UriComponentsBuilder builder = ServletUriComponentsBuilder.fromServletMapping(request);
ServletUriComponentsBuilder builder = ServletUriComponentsBuilder.fromServletMapping(request);
// special case handling for X-Forwarded-Ssl:
// apply it, but only if X-Forwarded-Proto is unset.
String forwardedSsl = request.getHeader("X-Forwarded-Ssl");
ForwardedHeader forwarded = ForwardedHeader.of(request.getHeader(ForwardedHeader.NAME));
String proto = hasText(forwarded.getProto()) ? forwarded.getProto() : request.getHeader("X-Forwarded-Proto");
if (!hasText(proto) && hasText(forwardedSsl) && forwardedSsl.equalsIgnoreCase("on")) {
builder.scheme("https");
// Spring 5.1 can handle X-Forwarded-Ssl headers...
if (isSpringAtLeast5_1()) {
return builder;
} else {
return handleXForwardedSslHeader(request, builder);
}
}
return builder;
/**
* Check if the current version of Spring Framework is 5.1 or higher.
*
* @return
*/
private static boolean isSpringAtLeast5_1() {
String versionOfSpringFramework = ApplicationContext.class.getPackage().getImplementationVersion();
String[] parts = versionOfSpringFramework.split("\\.");
int majorVersion = Integer.parseInt(parts[0]);
int minorVersion = Integer.parseInt(parts[1]);
return (majorVersion >= 5 && minorVersion >= 1) || (majorVersion > 5);
}
/**

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014 the original author or authors.
* Copyright 2014-2018 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.
@@ -15,37 +15,70 @@
*/
package org.springframework.hateoas.mvc;
import static org.springframework.util.StringUtils.*;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import javax.servlet.http.HttpServletRequest;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
import org.springframework.web.util.UriComponentsBuilder;
/**
* Value object to partially implement the {@literal Forwarded} header defined in RFC 7239.
*
* @author Oliver Gierke
* @see http://tools.ietf.org/html/rfc7239
* @deprecated In Spring 5.1, all Forwarded headers will by handled by Spring MVC.
*/
@Deprecated
class ForwardedHeader {
public static String NAME = "Forwarded";
private static final ForwardedHeader NO_HEADER = new ForwardedHeader(Collections.<String, String> emptyMap());
private final Map<String, String> elements;
private ForwardedHeader(Map<String, String> elements) {
this.elements = elements;
}
/**
* Utility method to pull handling of {@literal X-Forwarded-Ssl} into a class that will be removed when rebaselined
* against Spring 5.1
*
* @param request
* @param builder
* @return
* @deprecated No longer needed with Spring 5.1
*/
@Deprecated
public static UriComponentsBuilder handleXForwardedSslHeader(HttpServletRequest request,
UriComponentsBuilder builder) {
// special case handling for X-Forwarded-Ssl:
// apply it, but only if X-Forwarded-Proto is unset.
String forwardedSsl = request.getHeader("X-Forwarded-Ssl");
ForwardedHeader forwarded = ForwardedHeader.of(request.getHeader("Forwarded"));
String proto = hasText(forwarded.getProto()) ? forwarded.getProto() : request.getHeader("X-Forwarded-Proto");
if (!hasText(proto) && hasText(forwardedSsl) && forwardedSsl.equalsIgnoreCase("on")) {
builder.scheme("https");
}
return builder;
}
/**
* Creates a new {@link ForwardedHeader} from the given source.
*
* @param source can be {@literal null}.
* @return
*/
public static ForwardedHeader of(String source) {
static ForwardedHeader of(String source) {
if (!StringUtils.hasText(source)) {
return NO_HEADER;
@@ -75,7 +108,7 @@ class ForwardedHeader {
*
* @return
*/
public String getProto() {
String getProto() {
return elements.get("proto");
}
@@ -84,7 +117,7 @@ class ForwardedHeader {
*
* @return
*/
public String getHost() {
String getHost() {
return elements.get("host");
}
}

View File

@@ -18,10 +18,18 @@ package org.springframework.hateoas;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import java.io.IOException;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import org.junit.Before;
import org.springframework.mock.web.MockFilterChain;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletRequestAttributes;
import org.springframework.web.filter.ForwardedHeaderFilter;
/**
* Utility class to ease tesing.
@@ -44,6 +52,28 @@ public class TestUtils {
assertThat(link.getHref(), startsWith("http://localhost"));
}
/**
* Provide a mechanism to simulate inserting a {@link ForwardedHeaderFilter} into the servlet filter chain, so
* {@literal Forwarded} headers are properly inserted into the test web request.
*
* @see https://jira.spring.io/browse/SPR-16668
*/
protected void adaptRequestFromForwardedHeaders() {
MockFilterChain chain = new MockFilterChain();
try {
new ForwardedHeaderFilter().doFilter(this.request, new MockHttpServletResponse(), chain);
} catch (ServletException e) {
throw new RuntimeException(e);
} catch (IOException e) {
throw new RuntimeException(e);
}
HttpServletRequest adaptedRequest = (HttpServletRequest) chain.getRequest();
RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(adaptedRequest));
}
public static void assertEqualAndSameHashCode(Object left, Object right) {
assertThat(left, is(right));

View File

@@ -19,11 +19,14 @@ import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.*;
import static org.springframework.hateoas.mvc.ControllerLinkBuilder.*;
import java.io.IOException;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import javax.servlet.ServletException;
import org.hamcrest.Matchers;
import org.junit.Rule;
import org.junit.Test;
@@ -162,6 +165,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
request.addHeader("X-Forwarded-Host", "somethingDifferent");
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith("http://somethingDifferent"));
}
@@ -174,6 +179,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
request.addHeader("X-Forwarded-Ssl", "on");
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith("https://"));
}
@@ -186,6 +193,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
request.addHeader("X-Forwarded-Ssl", "off");
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith("http://"));
}
@@ -199,6 +208,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
request.addHeader("X-Forwarded-Host", "somethingDifferent");
request.addHeader("X-Forwarded-Ssl", "on");
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith("https://somethingDifferent"));
}
@@ -270,6 +281,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
request.addHeader("X-Forwarded-Host", "foobar:8088");
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith("http://foobar:8088"));
}
@@ -282,6 +295,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
request.addHeader("X-Forwarded-Host", "barfoo:8888, localhost:8088");
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith("http://barfoo:8888"));
}
@@ -335,6 +350,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
request.addHeader("X-Forwarded-Port", "9090");
request.setServerPort(8080);
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith("http://foobarhost:9090/"));
@@ -349,6 +366,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
request.addHeader("X-Forwarded-Host", "foobarhost");
request.setServerPort(8080);
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith("http://foobarhost/"));
}
@@ -410,12 +429,14 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
* @see #257, #107
*/
@Test
public void usesXForwardedProtoHeaderAsLinkSchema() {
public void usesXForwardedProtoHeaderAsLinkSchema() throws ServletException, IOException {
for (String proto : Arrays.asList("http", "https")) {
setUp();
request.addHeader("X-Forwarded-Proto", proto);
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith(proto + "://"));
@@ -433,6 +454,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
setUp();
request.addHeader("Forwarded", new String[] { "proto=" + proto });
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith(proto.concat("://")));
}
@@ -445,7 +468,9 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
public void favorsStandardForwardHeaderOverXForwardedProto() {
request.addHeader("X-Forwarded-Proto", "foo");
request.addHeader(ForwardedHeader.NAME, "proto=bar");
request.addHeader("Forwarded", "proto=bar");
adaptRequestFromForwardedHeaders();
Link link = linkTo(PersonControllerImpl.class).withSelfRel();
assertThat(link.getHref(), startsWith("bar://"));
@@ -527,6 +552,8 @@ public class ControllerLinkBuilderUnitTest extends TestUtils {
request.addHeader("X-Forwarded-Port", "1443,8443");
request.addHeader("X-Forwarded-Host", "proxy1,proxy2");
adaptRequestFromForwardedHeaders();
assertThat(linkTo(PersonControllerImpl.class).withSelfRel().getHref(), startsWith("http://proxy1:1443"));
}