Spring/spring-hateoas
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

#758 - Centralize all Forward header handling.

Browse Source 
Spring 5.1 is centralizing all Forward header handling. This moves critical bits into one location, making it easy to completely remove once we rebaseline 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 267ff45043
commit bb4a4a615c
6 changed files with 134 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.travis.yml
View File

@@ -5,6 +5,7 @@ env:
matrix:
- PROFILE=non-existant
- PROFILE=spring5-next
- PROFILE=spring51-next
addons:
apt:
packages:

14
pom.xml
View File

@@ -97,6 +97,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 -->

43
src/main/java/org/springframework/hateoas/mvc/ControllerLinkBuilder.java
View File

@@ -15,7 +15,7 @@
*/
package org.springframework.hateoas.mvc;
import static org.springframework.util.StringUtils.*;
import static org.springframework.hateoas.mvc.ForwardedHeader.handleXForwardedSslHeader;
import lombok.RequiredArgsConstructor;
import lombok.experimental.Delegate;
@@ -28,6 +28,7 @@ import java.util.Map;
import javax.servlet.http.HttpServletRequest;
import org.springframework.context.ApplicationContext;
import org.springframework.core.io.support.SpringFactoriesLoader;
import org.springframework.hateoas.Affordance;
import org.springframework.hateoas.Link;
@@ -306,27 +307,37 @@ public class ControllerLinkBuilder extends LinkBuilderSupport<ControllerLinkBuil
*
* @return
*/
public static UriComponentsBuilder getBuilder() {
public static UriComponentsBuilder getBuilder() {
if (RequestContextHolder.getRequestAttributes() == null) {
return UriComponentsBuilder.fromPath("/");
}
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);
}
/**
@@ -361,13 +372,13 @@ public class ControllerLinkBuilder extends LinkBuilderSupport<ControllerLinkBuil
private static class CachingAnnotationMappingDiscoverer implements MappingDiscoverer {
private final @Delegate AnnotationMappingDiscoverer delegate;
private final Map<String, UriTemplate> templates = new ConcurrentReferenceHashMap<>();
private final Map<String, UriTemplate> templates = new ConcurrentReferenceHashMap<>();
public UriTemplate getMappingAsUriTemplate(Class<?> type, Method method) {
String mapping = delegate.getMapping(type, method);
return templates.computeIfAbsent(mapping, UriTemplate::new);
return templates.computeIfAbsent(mapping, UriTemplate::new);
}
}

42
src/main/java/org/springframework/hateoas/mvc/ForwardedHeader.java
View File

@@ -15,38 +15,70 @@
*/
package org.springframework.hateoas.mvc;
import static org.springframework.util.StringUtils.hasText;
import java.util.Arrays;
import java.util.Collections;
import java.util.Map;
import java.util.stream.Collectors;
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.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;
@@ -67,7 +99,7 @@ class ForwardedHeader {
*
* @return
*/
public String getProto() {
String getProto() {
return elements.get("proto");
}
@@ -76,7 +108,7 @@ class ForwardedHeader {
*
* @return
*/
public String getHost() {
String getHost() {
return elements.get("host");
}
}

26
src/test/java/org/springframework/hateoas/TestUtils.java
View File

@@ -17,10 +17,18 @@ package org.springframework.hateoas;
import static org.assertj.core.api.Assertions.*;
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 testing.
@@ -43,6 +51,24 @@ 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 | 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).isEqualTo(right);

31
src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java
View File

@@ -20,11 +20,14 @@ import static org.hamcrest.MatcherAssert.assertThat;
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.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
@@ -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"));
}