DATAREST-815 - Tweaked HalBrowser controller implementation to consider proxied requests.

We now use the APIs in place in Spring MVC to make sure we create proper redirects for requests that carry proxy headers so that the redirect target is still going through the proxy.

Author: odrotbohm

spring-data-rest-hal-browser/src/main/java/org/springframework/data/rest/webmvc/halbrowser/HalBrowser.java

@@ -17,15 +17,14 @@
 
 import javax.servlet.http.HttpServletRequest;
 
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
 import org.springframework.data.rest.webmvc.BasePathAwareController;
 import org.springframework.http.MediaType;
-import org.springframework.util.Assert;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.servlet.View;
+import org.springframework.web.servlet.support.ServletUriComponentsBuilder;
 import org.springframework.web.servlet.view.RedirectView;
+import org.springframework.web.util.UriComponents;
 
 /**
  * Controller with a few convenience redirects to expose the HAL browser shipped as static content.
@@ -37,22 +36,7 @@
 public class HalBrowser {
 
 	private static String BROWSER = "/browser";
-	public static String BROWSER_INDEX = BROWSER.concat("/index.html");
-
-	private final RepositoryRestConfiguration configuration;
-
-	/**
-	 * Creates a new {@link HalBrowser} for the given {@link RepositoryRestConfiguration}.
-	 * 
-	 * @param configuration must not be {@literal null}.
-	 */
-	@Autowired
-	public HalBrowser(RepositoryRestConfiguration configuration) {
-
-		Assert.notNull(configuration, "RepositoryRestConfiguration must not be null!");
-
-		this.configuration = configuration;
-	}
+	private static String INDEX = "/index.html";
 
 	/**
 	 * Redirects requests to the API root asking for HTML to the HAL browser.
@@ -61,7 +45,7 @@ public HalBrowser(RepositoryRestConfiguration configuration) {
 	 */
 	@RequestMapping(value = { "/", "" }, method = RequestMethod.GET, produces = MediaType.TEXT_HTML_VALUE)
 	public View index(HttpServletRequest request) {
-		return browser(request);
+		return getRedirectView(request, false);
 	}
 
 	/**
@@ -71,10 +55,30 @@ public View index(HttpServletRequest request) {
 	 */
 	@RequestMapping(value = "/browser", method = RequestMethod.GET)
 	public View browser(HttpServletRequest request) {
+		return getRedirectView(request, request.getRequestURI().endsWith("/browser"));
+	}
+
+	/**
+	 * Returns the View to redirect to to access the HAL browser.
+	 * 
+	 * @param request must not be {@literal null}.
+	 * @param browserRelative
+	 * @return
+	 */
+	private View getRedirectView(HttpServletRequest request, boolean browserRelative) {
+
+		ServletUriComponentsBuilder builder = ServletUriComponentsBuilder.fromRequest(request);
+
+		UriComponents components = builder.build();
+		String path = components.getPath() == null ? "" : components.getPath();
+
+		if (!browserRelative) {
+			builder.path(BROWSER);
+		}
 
-		String contextPath = request.getContextPath();
-		String basePath = configuration.getBasePath().toString();
+		builder.path(INDEX);
+		builder.fragment(browserRelative ? path.substring(0, path.lastIndexOf("/browser")) : path);
 
-		return new RedirectView(basePath.concat(BROWSER_INDEX).concat("#").concat(contextPath.concat(basePath)), true);
+		return new RedirectView(builder.build().toUriString());
 	}
 }

spring-data-rest-hal-browser/src/test/java/org/springframework/data/rest/webmvc/halbrowser/HalBrowserUnitTests.java

@@ -15,18 +15,12 @@
  */
 package org.springframework.data.rest.webmvc.halbrowser;
 
-import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
-import static org.mockito.Mockito.*;
 
 import java.util.Collections;
 
-import org.hamcrest.Matchers;
 import org.junit.Test;
-import org.springframework.data.rest.core.config.EnumTranslationConfiguration;
-import org.springframework.data.rest.core.config.MetadataConfiguration;
-import org.springframework.data.rest.core.config.ProjectionDefinitionConfiguration;
-import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
 import org.springframework.http.HttpHeaders;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
@@ -51,22 +45,39 @@ public class HalBrowserUnitTests {
 	@Test
 	public void createsContextRelativeRedirectForBrowser() throws Exception {
 
-		RepositoryRestConfiguration configuration = new RepositoryRestConfiguration(new ProjectionDefinitionConfiguration(),
-				new MetadataConfiguration(), mock(EnumTranslationConfiguration.class));
-
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.setRequestURI("/context");
 		request.setContextPath("/context");
 
-		View view = new HalBrowser(configuration).browser(request);
+		View view = new HalBrowser().browser(request);
 
 		assertThat(view, is(instanceOf(RedirectView.class)));
 
 		((AbstractView) view).render(Collections.<String, Object> emptyMap(), request, response);
 
 		UriComponents components = UriComponentsBuilder.fromUriString(response.getHeader(HttpHeaders.LOCATION)).build();
 
-		assertThat(components.getPath(), Matchers.startsWith("/context"));
+		assertThat(components.getPath(), startsWith("/context"));
 		assertThat(components.getFragment(), is("/context"));
 	}
+
+	@Test
+	public void producesProxyRelativeRedirectIfNecessary() {
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/browser");
+		request.addHeader("X-Forwarded-Host", "somehost");
+		request.addHeader("X-Forwarded-Port", "4711");
+		request.addHeader("X-Forwarded-Proto", "https");
+		request.addHeader("X-Forwarded-Prefix", "/prefix");
+
+		View view = new HalBrowser().browser(request);
+
+		assertThat(view, is(instanceOf(RedirectView.class)));
+
+		String url = ((RedirectView) view).getUrl();
+
+		assertThat(url, startsWith("https://somehost:4711/prefix"));
+		assertThat(url, endsWith("/prefix"));
+	}
 }