From a4298b29e7f30715c7d8f96334c8d30a02e541a1 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 23 Jun 2016 13:10:04 +0100 Subject: [PATCH] =?UTF-8?q?Include=20request=20URI=E2=80=99s=20port,=20if?= =?UTF-8?q?=20any,=20in=20default=20Host=20header?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, OperationRequestFactory would add a Host header if one did not already exist in the request’s headers, however it did not include the port. This meant that when the request was being made to a non-standard port (a port other than 80 for an HTTP request and 443 for an HTTPS request) the Host header was incorrect. This commit updates OperationRequestFactory to check the URI for a port and, if it has one, include it in the Host header. UriModifyingOperationPreprocessor has also been updated to correctly include the port in the Host header. Closes gh-269 --- .../restdocs/operation/OperationRequestFactory.java | 9 ++++++++- .../restdocs/http/HttpRequestSnippetTests.java | 10 ++++++++++ .../MockMvcRestDocumentationIntegrationTests.java | 2 +- .../preprocess/UriModifyingOperationPreprocessor.java | 10 ++++++---- .../restassured/RestAssuredRequestConverterTests.java | 2 +- .../RestAssuredRestDocumentationIntegrationTests.java | 10 +++++----- .../UriModifyingOperationPreprocessorTests.java | 8 +++++++- 7 files changed, 38 insertions(+), 13 deletions(-) diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/operation/OperationRequestFactory.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/operation/OperationRequestFactory.java index 961987b5..198011b4 100644 --- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/operation/OperationRequestFactory.java +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/operation/OperationRequestFactory.java @@ -100,10 +100,17 @@ public class OperationRequestFactory { private HttpHeaders augmentHeaders(HttpHeaders originalHeaders, URI uri, byte[] content) { return new HttpHeadersHelper(originalHeaders) - .addIfAbsent(HttpHeaders.HOST, uri.getHost()) + .addIfAbsent(HttpHeaders.HOST, createHostHeader(uri)) .setContentLengthHeader(content).getHeaders(); } + private String createHostHeader(URI uri) { + if (uri.getPort() == -1) { + return uri.getHost(); + } + return uri.getHost() + ":" + uri.getPort(); + } + private HttpHeaders getUpdatedHeaders(HttpHeaders originalHeaders, byte[] updatedContent) { return new HttpHeadersHelper(originalHeaders) diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/http/HttpRequestSnippetTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/http/HttpRequestSnippetTests.java index 32e1e680..df1401ac 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/http/HttpRequestSnippetTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/http/HttpRequestSnippetTests.java @@ -59,6 +59,16 @@ public class HttpRequestSnippetTests extends AbstractSnippetTests { .request("http://localhost/foo").header("Alpha", "a").build()); } + @Test + public void getRequestWithPort() throws IOException { + this.snippet.expectHttpRequest("get-request") + .withContents(httpRequest(RequestMethod.GET, "/foo").header("Alpha", "a") + .header(HttpHeaders.HOST, "localhost:8080")); + + new HttpRequestSnippet().document(operationBuilder("get-request") + .request("http://localhost:8080/foo").header("Alpha", "a").build()); + } + @Test public void getRequestWithQueryString() throws IOException { this.snippet.expectHttpRequest("get-request-with-query-string") diff --git a/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java b/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java index 5d70cfb5..0094abc8 100644 --- a/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java +++ b/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java @@ -375,7 +375,7 @@ public class MockMvcRestDocumentationIntegrationTests { .header("Content-Type", "application/json") .header("Accept", MediaType.APPLICATION_JSON_VALUE) - .header("Host", "localhost") + .header("Host", "localhost:8080") .header("Content-Length", "13") .content("{\"a\":\"alpha\"}")))); String prettyPrinted = String.format("{%n \"a\" : \"<>\"%n}"); diff --git a/spring-restdocs-restassured/src/main/java/org/springframework/restdocs/restassured/operation/preprocess/UriModifyingOperationPreprocessor.java b/spring-restdocs-restassured/src/main/java/org/springframework/restdocs/restassured/operation/preprocess/UriModifyingOperationPreprocessor.java index 3c7b109d..5d156655 100644 --- a/spring-restdocs-restassured/src/main/java/org/springframework/restdocs/restassured/operation/preprocess/UriModifyingOperationPreprocessor.java +++ b/spring-restdocs-restassured/src/main/java/org/springframework/restdocs/restassured/operation/preprocess/UriModifyingOperationPreprocessor.java @@ -16,6 +16,7 @@ package org.springframework.restdocs.restassured.operation.preprocess; +import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -125,8 +126,10 @@ public final class UriModifyingOperationPreprocessor implements OperationPreproc if (this.scheme != null) { uriBuilder.scheme(this.scheme); } + HttpHeaders modifiedHeaders = modify(request.getHeaders()); if (this.host != null) { uriBuilder.host(this.host); + modifiedHeaders.set(HttpHeaders.HOST, this.host); } if (this.port != null) { if (StringUtils.hasText(this.port)) { @@ -136,10 +139,9 @@ public final class UriModifyingOperationPreprocessor implements OperationPreproc uriBuilder.port(null); } } - HttpHeaders modifiedHeaders = modify(request.getHeaders()); - if (this.host != null) { - modifiedHeaders.set(HttpHeaders.HOST, this.host); - } + URI modifiedUri = uriBuilder.build(true).toUri(); + modifiedHeaders.set(HttpHeaders.HOST, modifiedUri.getHost() + + (modifiedUri.getPort() == -1 ? "" : ":" + modifiedUri.getPort())); return this.contentModifyingDelegate.preprocess( new OperationRequestFactory().create(uriBuilder.build(true).toUri(), request.getMethod(), request.getContent(), modifiedHeaders, diff --git a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/RestAssuredRequestConverterTests.java b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/RestAssuredRequestConverterTests.java index 389a878a..3bf79098 100644 --- a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/RestAssuredRequestConverterTests.java +++ b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/RestAssuredRequestConverterTests.java @@ -142,7 +142,7 @@ public class RestAssuredRequestConverterTests { assertThat(request.getHeaders().get("Foo"), is(equalTo(Arrays.asList("bar")))); assertThat(request.getHeaders().get("Accept"), is(equalTo(Arrays.asList("*/*")))); assertThat(request.getHeaders().get("Host"), - is(equalTo(Arrays.asList("localhost")))); + is(equalTo(Arrays.asList("localhost:" + this.port)))); } @Test diff --git a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/RestAssuredRestDocumentationIntegrationTests.java b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/RestAssuredRestDocumentationIntegrationTests.java index 27c055ae..f81c91f3 100644 --- a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/RestAssuredRestDocumentationIntegrationTests.java +++ b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/RestAssuredRestDocumentationIntegrationTests.java @@ -268,9 +268,9 @@ public class RestAssuredRestDocumentationIntegrationTests { .filter(document("original-request")) .filter(document("preprocessed-request", preprocessRequest(prettyPrint(), - removeHeaders("a", HttpHeaders.HOST, - HttpHeaders.CONTENT_LENGTH), - replacePattern(pattern, "\"<>\"")))) + replacePattern(pattern, "\"<>\""), + modifyUris().removePort(), + removeHeaders("a", HttpHeaders.CONTENT_LENGTH)))) .get("/").then().statusCode(200); assertThat( new File("build/generated-snippets/original-request/http-request.adoc"), @@ -279,7 +279,7 @@ public class RestAssuredRestDocumentationIntegrationTests { .header("a", "alpha").header("b", "bravo") .header("Accept", MediaType.APPLICATION_JSON_VALUE) .header("Content-Type", "application/json; charset=UTF-8") - .header("Host", "localhost") + .header("Host", "localhost:" + this.port) .header("Content-Length", "13") .content("{\"a\":\"alpha\"}")))); String prettyPrinted = String.format("{%n \"a\" : \"<>\"%n}"); @@ -291,7 +291,7 @@ public class RestAssuredRestDocumentationIntegrationTests { .header("b", "bravo") .header("Accept", MediaType.APPLICATION_JSON_VALUE) .header("Content-Type", "application/json; charset=UTF-8") - .content(prettyPrinted)))); + .header("Host", "localhost").content(prettyPrinted)))); } @Test diff --git a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/operation/preprocess/UriModifyingOperationPreprocessorTests.java b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/operation/preprocess/UriModifyingOperationPreprocessorTests.java index 8ce53288..d8cffc40 100644 --- a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/operation/preprocess/UriModifyingOperationPreprocessorTests.java +++ b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured/operation/preprocess/UriModifyingOperationPreprocessorTests.java @@ -63,9 +63,11 @@ public class UriModifyingOperationPreprocessorTests { public void requestUriHostCanBeModified() { this.preprocessor.host("api.example.com"); OperationRequest processed = this.preprocessor - .preprocess(createRequestWithUri("http://api.example.com:12345")); + .preprocess(createRequestWithUri("http://api.foo.com:12345")); assertThat(processed.getUri(), is(equalTo(URI.create("http://api.example.com:12345")))); + assertThat(processed.getHeaders().getFirst(HttpHeaders.HOST), + is(equalTo("api.example.com:12345"))); } @Test @@ -75,6 +77,8 @@ public class UriModifyingOperationPreprocessorTests { .preprocess(createRequestWithUri("http://api.example.com:12345")); assertThat(processed.getUri(), is(equalTo(URI.create("http://api.example.com:23456")))); + assertThat(processed.getHeaders().getFirst(HttpHeaders.HOST), + is(equalTo("api.example.com:23456"))); } @Test @@ -83,6 +87,8 @@ public class UriModifyingOperationPreprocessorTests { OperationRequest processed = this.preprocessor .preprocess(createRequestWithUri("http://api.example.com:12345")); assertThat(processed.getUri(), is(equalTo(URI.create("http://api.example.com")))); + assertThat(processed.getHeaders().getFirst(HttpHeaders.HOST), + is(equalTo("api.example.com"))); } @Test