#713 - 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.

Author: gregturn

.travis.yml

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

pom.xml

@@ -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 -->

src/main/java/org/springframework/hateoas/mvc/ControllerLinkBuilder.java

@@ -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 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 @@ static UriComponentsBuilder getBuilder() {
 		}
 
 		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.
+		// Spring 5.1 can handle X-Forwarded-Ssl headers...
+		if (isSpringAtLeast5_1()) {
+			return builder;
+		} else {
+			return handleXForwardedSslHeader(request, builder);
+		}
+	}
+
+	/**
+	 * Check if the current version of Spring Framework is 5.1 or higher.
+	 * 
+	 * @return
+	 */
+	private static boolean isSpringAtLeast5_1() {
 
-		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");
+		String versionOfSpringFramework = ApplicationContext.class.getPackage().getImplementationVersion();
 
-		if (!hasText(proto) && hasText(forwardedSsl) && forwardedSsl.equalsIgnoreCase("on")) {
-			builder.scheme("https");
-		}
+		String[] parts = versionOfSpringFramework.split("\\.");
+		int majorVersion = Integer.parseInt(parts[0]);
+		int minorVersion = Integer.parseInt(parts[1]);
 
-		return builder;
+		return (majorVersion >= 5 && minorVersion >= 1) || (majorVersion > 5);
 	}
 
 	/**

src/main/java/org/springframework/hateoas/mvc/ForwardedHeader.java

@@ -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 @@ public static ForwardedHeader of(String source) {
 	 * 
 	 * @return
 	 */
-	public String getProto() {
+	String getProto() {
 		return elements.get("proto");
 	}
 
@@ -84,7 +117,7 @@ public String getProto() {
 	 * 
 	 * @return
 	 */
-	public String getHost() {
+	String getHost() {
 		return elements.get("host");
 	}
 }

src/test/java/org/springframework/hateoas/TestUtils.java

@@ -18,10 +18,18 @@
 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 @@ protected void assertPointsToMockServer(Link link) {
 		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));

src/test/java/org/springframework/hateoas/mvc/ControllerLinkBuilderUnitTest.java

@@ -19,11 +19,14 @@
 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 void usesForwardedHostAsHostIfHeaderIsSet() {
 
 		request.addHeader("X-Forwarded-Host", "somethingDifferent");
 
+		adaptRequestFromForwardedHeaders();
+
 		Link link = linkTo(PersonControllerImpl.class).withSelfRel();
 		assertThat(link.getHref(), startsWith("http://somethingDifferent"));
 	}
@@ -174,6 +179,8 @@ public void usesForwardedSslIfHeaderIsSet() {
 
 		request.addHeader("X-Forwarded-Ssl", "on");
 
+		adaptRequestFromForwardedHeaders();
+
 		Link link = linkTo(PersonControllerImpl.class).withSelfRel();
 		assertThat(link.getHref(), startsWith("https://"));
 	}
@@ -186,6 +193,8 @@ public void usesForwardedSslIfHeaderIsSetOff() {
 
 		request.addHeader("X-Forwarded-Ssl", "off");
 
+		adaptRequestFromForwardedHeaders();
+
 		Link link = linkTo(PersonControllerImpl.class).withSelfRel();
 		assertThat(link.getHref(), startsWith("http://"));
 	}
@@ -199,6 +208,8 @@ public void usesForwardedSslAndHostIfHeaderIsSet() {
 		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 void usesForwardedHostAndPortFromHeader() {
 
 		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 void usesFirstHostOfXForwardedHost() {
 
 		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 void usesForwardedPortFromHeader() {
 		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 void usesForwardedHostFromHeaderWithDefaultPort() {
 		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 void usesRootMappingOfTargetClassForMethodsOfParent() throws Exception {
 	 * @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 void usesProtoValueFromForwardedHeaderAsLinkSchema() {
 			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 void usesProtoValueFromForwardedHeaderAsLinkSchema() {
 	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 void supportsTwoProxiesAddingXForwardedPort() {
 		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"));
 	}