From 954c49be892f564a51bf3b82d66b44cef475c92d Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 27 Apr 2016 15:33:40 +0200 Subject: [PATCH] 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. --- .../rest/webmvc/halbrowser/HalBrowser.java | 50 ++++++++++--------- .../halbrowser/HalBrowserUnitTests.java | 35 ++++++++----- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/spring-data-rest-hal-browser/src/main/java/org/springframework/data/rest/webmvc/halbrowser/HalBrowser.java b/spring-data-rest-hal-browser/src/main/java/org/springframework/data/rest/webmvc/halbrowser/HalBrowser.java index 2037eaa71..992a13f8a 100644 --- a/spring-data-rest-hal-browser/src/main/java/org/springframework/data/rest/webmvc/halbrowser/HalBrowser.java +++ b/spring-data-rest-hal-browser/src/main/java/org/springframework/data/rest/webmvc/halbrowser/HalBrowser.java @@ -17,15 +17,14 @@ package org.springframework.data.rest.webmvc.halbrowser; 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 @@ import org.springframework.web.servlet.view.RedirectView; 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 class HalBrowser { */ @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 class HalBrowser { */ @RequestMapping(value = "/browser", method = RequestMethod.GET) public View browser(HttpServletRequest request) { + return getRedirectView(request, request.getRequestURI().endsWith("/browser")); + } - String contextPath = request.getContextPath(); - String basePath = configuration.getBasePath().toString(); + /** + * 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) { - return new RedirectView(basePath.concat(BROWSER_INDEX).concat("#").concat(contextPath.concat(basePath)), true); + ServletUriComponentsBuilder builder = ServletUriComponentsBuilder.fromRequest(request); + + UriComponents components = builder.build(); + String path = components.getPath() == null ? "" : components.getPath(); + + if (!browserRelative) { + builder.path(BROWSER); + } + + builder.path(INDEX); + builder.fragment(browserRelative ? path.substring(0, path.lastIndexOf("/browser")) : path); + + return new RedirectView(builder.build().toUriString()); } } diff --git a/spring-data-rest-hal-browser/src/test/java/org/springframework/data/rest/webmvc/halbrowser/HalBrowserUnitTests.java b/spring-data-rest-hal-browser/src/test/java/org/springframework/data/rest/webmvc/halbrowser/HalBrowserUnitTests.java index 2b4aa943f..311c85514 100644 --- a/spring-data-rest-hal-browser/src/test/java/org/springframework/data/rest/webmvc/halbrowser/HalBrowserUnitTests.java +++ b/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,14 +45,12 @@ 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))); @@ -66,7 +58,26 @@ public class HalBrowserUnitTests { 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")); + } }