From 9874c22e2352371464e0d9f025381d01606a8778 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 30 Aug 2016 15:52:37 +0100 Subject: [PATCH] Fix HAL browser endpoint redirect and entry point with custom servlet path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the HAL browser endpoint did not consider the dispatcher servlet’s path (server.servlet-path) when redirecting to browser.html or when updating the API entry point in the served HTML. This commit moves to using ServletUriComponentsBuilder to build the URI for the redirect and the path for the entry point. In the interests of simplicity the logic that sometimes redirected and sometimes forwarded the request has been changed so that it will always perform a redirect. Closes gh-6586 --- .../endpoint/mvc/HalBrowserMvcEndpoint.java | 27 ++-- ...vcEndpointBrowserPathIntegrationTests.java | 19 ++- ...erMvcEndpointDisabledIntegrationTests.java | 13 +- ...ManagementContextPathIntegrationTests.java | 7 +- ...ointServerContextPathIntegrationTests.java | 8 +- ...MvcEndpointServerPortIntegrationTests.java | 8 +- ...ointServerServletPathIntegrationTests.java | 131 ++++++++++++++++++ ...serMvcEndpointVanillaIntegrationTests.java | 11 +- 8 files changed, 179 insertions(+), 45 deletions(-) create mode 100644 spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointServerServletPathIntegrationTests.java diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpoint.java index fce09c56148..de5edd25711 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpoint.java @@ -18,6 +18,8 @@ package org.springframework.boot.actuate.endpoint.mvc; import java.io.IOException; import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.List; import javax.servlet.http.HttpServletRequest; @@ -26,11 +28,13 @@ import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; import org.springframework.http.MediaType; import org.springframework.util.FileCopyUtils; +import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistry; import org.springframework.web.servlet.resource.ResourceTransformer; import org.springframework.web.servlet.resource.ResourceTransformerChain; import org.springframework.web.servlet.resource.TransformedResource; +import org.springframework.web.servlet.support.ServletUriComponentsBuilder; /** * {@link MvcEndpoint} to expose a HAL browser. @@ -61,12 +65,12 @@ public class HalBrowserMvcEndpoint extends HalJsonMvcEndpoint @RequestMapping(produces = MediaType.TEXT_HTML_VALUE) public String browse(HttpServletRequest request) { - String contextPath = getManagementServletContext().getContextPath() - + (getPath().endsWith("/") ? getPath() : getPath() + "/"); - if (request.getRequestURI().endsWith("/")) { - return "forward:" + contextPath + this.location.getHtmlFile(); - } - return "redirect:" + contextPath; + ServletUriComponentsBuilder builder = ServletUriComponentsBuilder + .fromRequest(request); + String uriString = builder.build().toUriString(); + + return "redirect:" + uriString + (uriString.endsWith("/") ? "" : "/") + + this.location.getHtmlFile(); } @Override @@ -143,17 +147,20 @@ public class HalBrowserMvcEndpoint extends HalJsonMvcEndpoint resource = transformerChain.transform(request, resource); if (resource.getFilename().equalsIgnoreCase( HalBrowserMvcEndpoint.this.location.getHtmlFile())) { - return replaceInitialLink(request.getContextPath(), resource); + return replaceInitialLink(request, resource); } return resource; } - private Resource replaceInitialLink(String contextPath, Resource resource) + private Resource replaceInitialLink(HttpServletRequest request, Resource resource) throws IOException { byte[] bytes = FileCopyUtils.copyToByteArray(resource.getInputStream()); String content = new String(bytes, DEFAULT_CHARSET); - String initial = contextPath + getManagementServletContext().getContextPath() - + getPath(); + List pathSegments = new ArrayList(ServletUriComponentsBuilder + .fromRequest(request).build().getPathSegments()); + pathSegments.remove(pathSegments.size() - 1); + String initial = "/" + + StringUtils.collectionToDelimitedString(pathSegments, "/"); content = content.replace("entryPoint: '/'", "entryPoint: '" + initial + "'"); return new TransformedResource(resource, content.getBytes(DEFAULT_CHARSET)); } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointBrowserPathIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointBrowserPathIntegrationTests.java index 73265902f53..20ea1a36269 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointBrowserPathIntegrationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointBrowserPathIntegrationTests.java @@ -24,16 +24,15 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.autoconfigure.MinimalActuatorHypermediaApplication; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.MockMvc; -import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; -import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -61,19 +60,17 @@ public class HalBrowserMvcEndpointBrowserPathIntegrationTests { } @Test - public void browser() throws Exception { - MvcResult response = this.mockMvc - .perform(get("/actuator/").accept(MediaType.TEXT_HTML)) - .andExpect(status().isOk()).andReturn(); - assertThat(response.getResponse().getForwardedUrl()) - .isEqualTo("/actuator/browser.html"); + public void requestWithTrailingSlashIsRedirectedToBrowserHtml() throws Exception { + this.mockMvc.perform(get("/actuator/").accept(MediaType.TEXT_HTML)) + .andExpect(status().isFound()).andExpect(header().string( + HttpHeaders.LOCATION, "http://localhost/actuator/browser.html")); } @Test - public void redirect() throws Exception { + public void requestWithoutTrailingSlashIsRedirectedToBrowserHtml() throws Exception { this.mockMvc.perform(get("/actuator").accept(MediaType.TEXT_HTML)) - .andExpect(status().isFound()) - .andExpect(header().string("location", "/actuator/")); + .andExpect(status().isFound()).andExpect(header().string("location", + "http://localhost/actuator/browser.html")); } @MinimalActuatorHypermediaApplication diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointDisabledIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointDisabledIntegrationTests.java index ab6683b671b..e7772532b94 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointDisabledIntegrationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointDisabledIntegrationTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.endpoint.mvc; +import com.google.common.net.HttpHeaders; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -29,11 +30,9 @@ import org.springframework.http.MediaType; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.MockMvc; -import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; -import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ -71,12 +70,10 @@ public class HalBrowserMvcEndpointDisabledIntegrationTests { } @Test - public void browser() throws Exception { - MvcResult response = this.mockMvc - .perform(get("/actuator/").accept(MediaType.TEXT_HTML)) - .andExpect(status().isOk()).andReturn(); - assertThat(response.getResponse().getForwardedUrl()) - .isEqualTo("/actuator/browser.html"); + public void browserRedirect() throws Exception { + this.mockMvc.perform(get("/actuator/").accept(MediaType.TEXT_HTML)) + .andExpect(status().isFound()).andExpect(header().string( + HttpHeaders.LOCATION, "http://localhost/actuator/browser.html")); } @Test diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointManagementContextPathIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointManagementContextPathIntegrationTests.java index a53c2f4ee86..ea29faa176b 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointManagementContextPathIntegrationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointManagementContextPathIntegrationTests.java @@ -24,6 +24,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.autoconfigure.MinimalActuatorHypermediaApplication; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.hateoas.ResourceSupport; +import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.TestPropertySource; @@ -38,7 +39,7 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.springframework.hateoas.mvc.ControllerLinkBuilder.linkTo; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.forwardedUrl; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -83,8 +84,8 @@ public class HalBrowserMvcEndpointManagementContextPathIntegrationTests { @Test public void actuatorHomeHtml() throws Exception { this.mockMvc.perform(get("/admin/").accept(MediaType.TEXT_HTML)) - .andExpect(status().isOk()) - .andExpect(forwardedUrl("/admin/browser.html")); + .andExpect(status().isFound()).andExpect(header().string( + HttpHeaders.LOCATION, "http://localhost/admin/browser.html")); } @Test diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointServerContextPathIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointServerContextPathIntegrationTests.java index 3748f8cc3c8..8b0f689e6c6 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointServerContextPathIntegrationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointServerContextPathIntegrationTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.endpoint.mvc; +import java.net.URI; import java.util.Arrays; import org.junit.Test; @@ -69,14 +70,15 @@ public class HalBrowserMvcEndpointServerContextPathIntegrationTests { } @Test - public void actuatorBrowser() throws Exception { + public void actuatorBrowserRedirect() throws Exception { HttpHeaders headers = new HttpHeaders(); headers.setAccept(Arrays.asList(MediaType.TEXT_HTML)); ResponseEntity entity = new TestRestTemplate().exchange( "http://localhost:" + this.port + "/spring/actuator/", HttpMethod.GET, new HttpEntity(null, headers), String.class); - assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); - assertThat(entity.getBody()).contains(" entity = new TestRestTemplate().exchange( "http://localhost:" + this.port + "/actuator/", HttpMethod.GET, new HttpEntity(null, headers), String.class); - assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); - assertThat(entity.getBody()).contains(" entity = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/spring/", HttpMethod.GET, + new HttpEntity(null, headers), String.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(entity.getBody()).contains("\"_links\":"); + } + + @Test + public void actuatorBrowserRedirect() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.setAccept(Arrays.asList(MediaType.TEXT_HTML)); + ResponseEntity entity = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/spring/actuator/", HttpMethod.GET, + new HttpEntity(null, headers), String.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.FOUND); + assertThat(entity.getHeaders().getLocation()).isEqualTo(URI.create( + "http://localhost:" + this.port + "/spring/actuator/browser.html")); + } + + @Test + public void actuatorBrowserEntryPoint() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.setAccept(Arrays.asList(MediaType.TEXT_HTML)); + ResponseEntity entity = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/spring/actuator/browser.html", + HttpMethod.GET, new HttpEntity(null, headers), String.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(entity.getBody()).contains("entryPoint: '/spring/actuator'"); + } + + @Test + public void actuatorLinks() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.setAccept(Arrays.asList(MediaType.APPLICATION_JSON)); + ResponseEntity entity = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/spring/actuator", HttpMethod.GET, + new HttpEntity(null, headers), String.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(entity.getBody()).contains("\"_links\":"); + } + + @Test + public void actuatorLinksWithTrailingSlash() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.setAccept(Arrays.asList(MediaType.APPLICATION_JSON)); + ResponseEntity entity = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/spring/actuator/", HttpMethod.GET, + new HttpEntity(null, headers), String.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(entity.getBody()).contains("\"_links\":"); + } + + @MinimalActuatorHypermediaApplication + @RestController + public static class SpringBootHypermediaApplication { + + @RequestMapping("") + public ResourceSupport home() { + ResourceSupport resource = new ResourceSupport(); + resource.add(linkTo(SpringBootHypermediaApplication.class).slash("/") + .withSelfRel()); + return resource; + } + + } + +} diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointVanillaIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointVanillaIntegrationTests.java index 209170a139e..4afe550a14a 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointVanillaIntegrationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HalBrowserMvcEndpointVanillaIntegrationTests.java @@ -28,16 +28,15 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.autoconfigure.MinimalActuatorHypermediaApplication; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.MockMvc; -import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; -import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ -84,11 +83,9 @@ public class HalBrowserMvcEndpointVanillaIntegrationTests { @Test public void browser() throws Exception { - MvcResult response = this.mockMvc - .perform(get("/actuator/").accept(MediaType.TEXT_HTML)) - .andExpect(status().isOk()).andReturn(); - assertThat(response.getResponse().getForwardedUrl()) - .isEqualTo("/actuator/browser.html"); + this.mockMvc.perform(get("/actuator/").accept(MediaType.TEXT_HTML)) + .andExpect(status().isFound()).andExpect(header().string( + HttpHeaders.LOCATION, "http://localhost/actuator/browser.html")); } @Test