Resolve #3490 ApacheProxyAddressStrategy should support Forward header from (#3491)

* #3490 ApacheProxyAddressStrategy should support Forward header from
Rfc7239

* #3490 ApacheProxyAddressStrategy should support Forward header from
Rfc7239

Remove one copy & paste error

* #3490 ApacheProxyAddressStrategy should support Forward header from
Rfc7239

Fix failing test

* #3490 ApacheProxyAddressStrategy should support Forward header from
Rfc7239

Fix failing test

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/ApacheProxyAddressStrategy.java

@@ -1,7 +1,5 @@
 package ca.uhn.fhir.rest.server;
 
-import java.net.URI;
-
 /*
  * #%L
  * HAPI FHIR - Server Framework
@@ -24,18 +22,21 @@ import java.net.URI;
 
 import java.util.Optional;
 
-import javax.servlet.ServletContext;
-import javax.servlet.http.HttpServletRequest;
-
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.server.ServletServerHttpRequest;
+import org.springframework.web.util.UriComponents;
+import org.springframework.web.util.UriComponentsBuilder;
+
+import javax.servlet.ServletContext;
+import javax.servlet.http.HttpServletRequest;
+
+
 
 import static java.util.Optional.ofNullable;
 
-import ca.uhn.fhir.rest.server.IncomingRequestAddressStrategy;
 
 /**
  * Works like the normal
@@ -77,7 +78,7 @@ public class ApacheProxyAddressStrategy extends IncomingRequestAddressStrategy {
 	private static final String X_FORWARDED_PREFIX = "x-forwarded-prefix";
 	private static final String X_FORWARDED_PROTO = "x-forwarded-proto";
 	private static final String X_FORWARDED_HOST = "x-forwarded-host";
-	private static final String X_FORWARDED_PORT = "x-forwarded-port";
+
 
 	private static final Logger LOG = LoggerFactory
 			.getLogger(ApacheProxyAddressStrategy.class);
@@ -99,62 +100,48 @@ public class ApacheProxyAddressStrategy extends IncomingRequestAddressStrategy {
 		String serverBase = super.determineServerBase(servletContext, request);
 		ServletServerHttpRequest requestWrapper = new ServletServerHttpRequest(
 				request);
+		UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromHttpRequest(requestWrapper);
+		uriBuilder.replaceQuery(null);
 		HttpHeaders headers = requestWrapper.getHeaders();
-		Optional<String> forwardedHost = headers
+		adjustSchemeWithDefault(uriBuilder, headers);
-				.getValuesAsList(X_FORWARDED_HOST).stream().findFirst();
+		return forwardedServerBase(serverBase, headers, uriBuilder);
-		return forwardedHost
+	}
-				.map(s -> forwardedServerBase(serverBase, headers, s))
+
-				.orElse(serverBase);
+	/**
+	 * If forward host ist defined, but no forward protocol, use the configured default.
+	 * 
+	 * @param uriBuilder
+	 * @param headers
+	 */
+	private void adjustSchemeWithDefault(UriComponentsBuilder uriBuilder,
+			HttpHeaders headers) {
+		if (headers.getFirst(X_FORWARDED_HOST) != null
+				&& headers.getFirst(X_FORWARDED_PROTO) == null) {
+			uriBuilder.scheme(useHttps ? "https" : "http");
+		}
 	}
 
 	private String forwardedServerBase(String originalServerBase,
-			HttpHeaders headers, String forwardedHost) {
+			HttpHeaders headers, UriComponentsBuilder uriBuilder) {
 		Optional<String> forwardedPrefix = getForwardedPrefix(headers);
-		LOG.debug("serverBase: {}, forwardedHost: {}, forwardedPrefix: {}",
+		LOG.debug("serverBase: {}, forwardedPrefix: {}", originalServerBase, forwardedPrefix);
-				originalServerBase, forwardedHost, forwardedPrefix);
 		LOG.debug("request header: {}", headers);
 
-		String host = protocol(headers) + "://" + forwardedHost;
-		String hostWithOptionalPort = port(headers).map(p -> (host + ":" + p))
-				.orElse(host);
-
 		String path = forwardedPrefix
 				.orElseGet(() -> pathFrom(originalServerBase));
-		return joinStringsWith(hostWithOptionalPort, path, "/");
+		uriBuilder.replacePath(path);
-	}
+		return uriBuilder.build().toUriString();
-
-	private Optional<String> port(HttpHeaders headers) {
-		return ofNullable(headers.getFirst(X_FORWARDED_PORT));
 	}
 
 	private String pathFrom(String serverBase) {
-		String serverBasePath = URI.create(serverBase).getPath();
+		UriComponents build = UriComponentsBuilder.fromHttpUrl(serverBase).build();
-		return StringUtils.defaultIfBlank(serverBasePath, "");
+		return StringUtils.defaultIfBlank(build.getPath(), "");
-	}
-
-	private static String joinStringsWith(String left, String right,
-			String joiner) {
-		if (left.endsWith(joiner) && right.startsWith(joiner)) {
-			return left + right.substring(1);
-		} else if (left.endsWith(joiner) || right.startsWith(joiner)) {
-			return left + right;
-		} else {
-			return left + joiner + right;
-		}
 	}
 
 	private Optional<String> getForwardedPrefix(HttpHeaders headers) {
 		return ofNullable(headers.getFirst(X_FORWARDED_PREFIX));
 	}
 
-	private String protocol(HttpHeaders headers) {
-		String protocol = headers.getFirst(X_FORWARDED_PROTO);
-		if (protocol != null) {
-			return protocol;
-		}
-		return useHttps ? "https" : "http";
-	}
-
 	/**
 	 * Static factory for instance using <code>http://</code>
 	 */

hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/ApacheProxyAddressStrategyTest.java

@@ -87,6 +87,32 @@ public class ApacheProxyAddressStrategyTest {
 				serverBase);
 	}
 	
+	@Test
+	public void testWithForwardedHostAndUnsetPort() {
+		ApacheProxyAddressStrategy addressStrategy = new ApacheProxyAddressStrategy(
+				true);
+		MockHttpServletRequest request = prepareRequest();
+
+		request.addHeader("X-Forwarded-Host", "my.example.host");
+		request.addHeader("X-Forwarded-Port", "-1");
+		String serverBase = addressStrategy.determineServerBase(null, request);
+		assertEquals("https://my.example.host/imagingstudy/fhir",
+				serverBase);
+	}
+	
+	@Test
+	public void testWithRfc7239 () {
+		ApacheProxyAddressStrategy addressStrategy = new ApacheProxyAddressStrategy(
+				true);
+		MockHttpServletRequest request = prepareRequest();
+
+		request.addHeader("Forwarded", "Forwarded: for=192.0.2.43,"
+				+ " for=198.51.100.17;by=203.0.113.60;proto=http;host=example.com");
+		String serverBase = addressStrategy.determineServerBase(null, request);
+		assertEquals("http://example.com/imagingstudy/fhir",
+				serverBase);
+	}
+
 	private MockHttpServletRequest prepareRequest() {
 		MockHttpServletRequest request = new MockHttpServletRequest();
 		request.setMethod("POST");