SpringCloud/spring-cloud-gateway
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Updates NettyRoutingFilter to set request headers early.

Browse Source 
By setting them early, it doesn't overwrite the Transfer-Encoding:chunked header automatically set by the Reactor Netty HttpClient.

This caused bad requests to be sent downstream.

fixes gh-1181
This commit is contained in:
Spencer Gibb
2019-09-19 18:57:42 -04:00
parent d534f71a48
commit 0d2b87af30
2 changed files with 98 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

136
spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java
View File

@@ -114,93 +114,81 @@ public class NettyRoutingFilter implements GlobalFilter, Ordered {
final DefaultHttpHeaders httpHeaders = new DefaultHttpHeaders(); final DefaultHttpHeaders httpHeaders = new DefaultHttpHeaders();
filtered.forEach(httpHeaders::set); filtered.forEach(httpHeaders::set);
String transferEncoding = request.getHeaders()
.getFirst(HttpHeaders.TRANSFER_ENCODING);
boolean chunkedTransfer = "chunked".equalsIgnoreCase(transferEncoding);
boolean preserveHost = exchange boolean preserveHost = exchange
.getAttributeOrDefault(PRESERVE_HOST_HEADER_ATTRIBUTE, false); .getAttributeOrDefault(PRESERVE_HOST_HEADER_ATTRIBUTE, false);
Flux<HttpClientResponse> responseFlux = this.httpClient Flux<HttpClientResponse> responseFlux = this.httpClient.headers(headers -> {
.chunkedTransfer(chunkedTransfer).request(method).uri(url) headers.add(httpHeaders);
.send((req, nettyOutbound) -> { if (preserveHost) {
req.headers(httpHeaders); String host = request.getHeaders().getFirst(HttpHeaders.HOST);
headers.add(HttpHeaders.HOST, host);
}
}).request(method).uri(url).send((req, nettyOutbound) -> {
if (log.isTraceEnabled()) {
nettyOutbound.withConnection(connection -> log.trace(
"outbound route: " + connection.channel().id().asShortText()
+ ", inbound: " + exchange.getLogPrefix()));
}
return nettyOutbound.options(NettyPipeline.SendOptions::flushOnEach).send(
request.getBody().map(dataBuffer -> ((NettyDataBuffer) dataBuffer)
.getNativeBuffer()));
}).responseConnection((res, connection) -> {
if (preserveHost) { // Defer committing the response until all route filters have run
String host = request.getHeaders().getFirst(HttpHeaders.HOST); // Put client response as ServerWebExchange attribute and write
req.header(HttpHeaders.HOST, host); // response later NettyWriteResponseFilter
} exchange.getAttributes().put(CLIENT_RESPONSE_ATTR, res);
if (log.isTraceEnabled()) { exchange.getAttributes().put(CLIENT_RESPONSE_CONN_ATTR, connection);
nettyOutbound
.withConnection(connection -> log.trace("outbound route: "
+ connection.channel().id().asShortText()
+ ", inbound: " + exchange.getLogPrefix()));
}
return nettyOutbound.options(NettyPipeline.SendOptions::flushOnEach)
.send(request.getBody()
.map(dataBuffer -> ((NettyDataBuffer) dataBuffer)
.getNativeBuffer()));
}).responseConnection((res, connection) -> {
// Defer committing the response until all route filters have run ServerHttpResponse response = exchange.getResponse();
// Put client response as ServerWebExchange attribute and write // put headers and status so filters can modify the response
// response later NettyWriteResponseFilter HttpHeaders headers = new HttpHeaders();
exchange.getAttributes().put(CLIENT_RESPONSE_ATTR, res);
exchange.getAttributes().put(CLIENT_RESPONSE_CONN_ATTR, connection);
ServerHttpResponse response = exchange.getResponse(); res.responseHeaders()
// put headers and status so filters can modify the response .forEach(entry -> headers.add(entry.getKey(), entry.getValue()));
HttpHeaders headers = new HttpHeaders();
res.responseHeaders().forEach( String contentTypeValue = headers.getFirst(HttpHeaders.CONTENT_TYPE);
entry -> headers.add(entry.getKey(), entry.getValue())); if (StringUtils.hasLength(contentTypeValue)) {
exchange.getAttributes().put(ORIGINAL_RESPONSE_CONTENT_TYPE_ATTR,
contentTypeValue);
}
String contentTypeValue = headers.getFirst(HttpHeaders.CONTENT_TYPE); HttpStatus status = HttpStatus.resolve(res.status().code());
if (StringUtils.hasLength(contentTypeValue)) { if (status != null) {
exchange.getAttributes().put(ORIGINAL_RESPONSE_CONTENT_TYPE_ATTR, response.setStatusCode(status);
contentTypeValue); }
} else if (response instanceof AbstractServerHttpResponse) {
// https://jira.spring.io/browse/SPR-16748
((AbstractServerHttpResponse) response)
.setStatusCodeValue(res.status().code());
}
else {
// TODO: log warning here, not throw error?
throw new IllegalStateException("Unable to set status code on response: "
+ res.status().code() + ", " + response.getClass());
}
HttpStatus status = HttpStatus.resolve(res.status().code()); // make sure headers filters run after setting status so it is
if (status != null) { // available in response
response.setStatusCode(status); HttpHeaders filteredResponseHeaders = HttpHeadersFilter
} .filter(getHeadersFilters(), headers, exchange, Type.RESPONSE);
else if (response instanceof AbstractServerHttpResponse) {
// https://jira.spring.io/browse/SPR-16748
((AbstractServerHttpResponse) response)
.setStatusCodeValue(res.status().code());
}
else {
// TODO: log warning here, not throw error?
throw new IllegalStateException(
"Unable to set status code on response: "
+ res.status().code() + ", "
+ response.getClass());
}
// make sure headers filters run after setting status so it is if (!filteredResponseHeaders.containsKey(HttpHeaders.TRANSFER_ENCODING)
// available in response && filteredResponseHeaders.containsKey(HttpHeaders.CONTENT_LENGTH)) {
HttpHeaders filteredResponseHeaders = HttpHeadersFilter.filter( // It is not valid to have both the transfer-encoding header and
getHeadersFilters(), headers, exchange, Type.RESPONSE); // the content-length header.
// Remove the transfer-encoding header in the response if the
// content-length header is present.
response.getHeaders().remove(HttpHeaders.TRANSFER_ENCODING);
}
if (!filteredResponseHeaders exchange.getAttributes().put(CLIENT_RESPONSE_HEADER_NAMES,
.containsKey(HttpHeaders.TRANSFER_ENCODING) filteredResponseHeaders.keySet());
&& filteredResponseHeaders
.containsKey(HttpHeaders.CONTENT_LENGTH)) {
// It is not valid to have both the transfer-encoding header and
// the content-length header.
// Remove the transfer-encoding header in the response if the
// content-length header is present.
response.getHeaders().remove(HttpHeaders.TRANSFER_ENCODING);
}
exchange.getAttributes().put(CLIENT_RESPONSE_HEADER_NAMES, response.getHeaders().putAll(filteredResponseHeaders);
filteredResponseHeaders.keySet());
response.getHeaders().putAll(filteredResponseHeaders); return Mono.just(res);
});
return Mono.just(res);
});
if (properties.getResponseTimeout() != null) { if (properties.getResponseTimeout() != null) {
responseFlux = responseFlux.timeout(properties.getResponseTimeout(), responseFlux = responseFlux.timeout(properties.getResponseTimeout(),

51
spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/test/FormIntegrationTests.java
View File

@@ -29,10 +29,10 @@ import org.springframework.boot.test.web.client.TestRestTemplate;
import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Import;
import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.ClassPathResource;
import org.springframework.http.HttpEntity; import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.http.client.MultipartBodyBuilder;
import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.context.junit4.SpringRunner;
import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.LinkedMultiValueMap;
@@ -43,6 +43,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
import static org.springframework.cloud.gateway.test.TestUtils.getMap; import static org.springframework.cloud.gateway.test.TestUtils.getMap;
import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED; import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED;
import static org.springframework.http.MediaType.MULTIPART_FORM_DATA;
@RunWith(SpringRunner.class) @RunWith(SpringRunner.class)
@SpringBootTest(webEnvironment = RANDOM_PORT) @SpringBootTest(webEnvironment = RANDOM_PORT)
@@ -59,35 +60,55 @@ public class FormIntegrationTests extends BaseWebClientTests {
formData.add("foo", "bar"); formData.add("foo", "bar");
formData.add("baz", "bam"); formData.add("baz", "bam");
// @formatter:off
testClient.post().uri("/post").contentType(FORM_URL_ENCODED_CONTENT_TYPE) testClient.post().uri("/post").contentType(FORM_URL_ENCODED_CONTENT_TYPE)
.body(BodyInserters.fromFormData(formData)).exchange().expectStatus() .body(BodyInserters.fromFormData(formData))
.isOk().expectBody(Map.class).consumeWith(result -> { .exchange()
.expectStatus().isOk()
.expectBody(Map.class).consumeWith(result -> {
Map map = result.getResponseBody(); Map map = result.getResponseBody();
Map<String, Object> form = getMap(map, "form"); Map<String, Object> form = getMap(map, "form");
assertThat(form).containsEntry("foo", "bar"); assertThat(form).containsEntry("foo", "bar");
assertThat(form).containsEntry("baz", "bam"); assertThat(form).containsEntry("baz", "bam");
}); });
// @formatter:on
} }
@Test @Test
public void multipartFormDataWorks() { public void multipartFormDataWorksWebClient() {
ClassPathResource img = new ClassPathResource("1x1.png"); MultiValueMap<String, HttpEntity<?>> formData = createMultipartData();
// @formatter:off
testClient.post().uri("/post").contentType(MULTIPART_FORM_DATA)
.syncBody(formData)
.exchange()
.expectStatus().isOk()
.expectBody(Map.class)
.consumeWith(result -> assertMultipartData(result.getResponseBody()));
// @formatter:on
}
@Test
public void multipartFormDataWorksRestTemplate() {
MultiValueMap<String, HttpEntity<?>> formData = createMultipartData();
TestRestTemplate rest = new TestRestTemplate(); TestRestTemplate rest = new TestRestTemplate();
HttpHeaders headers = new HttpHeaders(); ResponseEntity<Map> response = rest.postForEntity(baseUri + "/post", formData,
headers.setContentType(MediaType.IMAGE_PNG);
HttpEntity<ClassPathResource> entity = new HttpEntity<>(img, headers);
MultiValueMap<String, Object> parts = new LinkedMultiValueMap<>();
parts.add("imgpart", entity);
ResponseEntity<Map> response = rest.postForEntity(baseUri + "/post", parts,
Map.class); Map.class);
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK);
Map<String, Object> files = getMap(response.getBody(), "files"); assertMultipartData(response.getBody());
}
private MultiValueMap<String, HttpEntity<?>> createMultipartData() {
ClassPathResource part = new ClassPathResource("1x1.png");
MultipartBodyBuilder builder = new MultipartBodyBuilder();
builder.part("imgpart", part, MediaType.IMAGE_PNG);
return builder.build();
}
private void assertMultipartData(Map responseBody) {
Map<String, Object> files = getMap(responseBody, "files");
assertThat(files).containsKey("imgpart"); assertThat(files).containsKey("imgpart");
String file = (String) files.get("imgpart"); String file = (String) files.get("imgpart");
assertThat(file).startsWith("data:").contains(";base64,"); assertThat(file).startsWith("data:").contains(";base64,");