Skip to content

Commit

Permalink
Prevent Content-Length and Host headers from being copied by default (#…
Browse files Browse the repository at this point in the history
…3313)

Also remove the bogus spring.cloud.gateway.proxy.auto-forward setting from the tests.
Fixes gh-3154
  • Loading branch information
jkuipers authored Jul 3, 2024
1 parent 17c1b5b commit b435ce3
Show file tree
Hide file tree
Showing 12 changed files with 158 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,5 @@ You can add headers to the downstream response by using the `header()` methods o
You can also manipulate response headers (and anything else you like in the response) by adding a mapper to the `get()` method (and other methods).
The mapper is a `Function` that takes the incoming `ResponseEntity` and converts it to an outgoing one.

First-class support is provided for "`sensitive`" headers (by default, `cookie` and `authorization`), which are not passed downstream, and for "`proxy`" (`x-forwarded-*`) headers.
First-class support is provided for "`sensitive`" headers (by default, `cookie` and `authorization`) and "`skipped`" headers (by default, `content-length` and `host`), which are not passed downstream, and for "`proxy`" (`x-forwarded-*`) headers. The idea behind "`skipped`" headers is that they may result in problems when copied over to the downstream request. For example: because of the way that the `ProxyExchange` calls the downstream endpoint the content's length might have changed or even use a `Transfer-Encoding: chunked` instead of a `Content-Length` header.

Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@
*
* <p>
* By default the incoming request body and headers are sent intact to the downstream
* service (with the exception of "sensitive" headers). To manipulate the downstream
* service (with the exception of "excluded" headers). To manipulate the downstream
* request there are "builder" style methods in {@link ProxyExchange}, but only the
* {@link #uri(String)} is mandatory. You can change the sensitive headers by calling the
* {@link #sensitive(String...)} method (Authorization and Cookie are sensitive by
* default).
* {@link #uri(String)} is mandatory. You can change the excluded headers by calling the
* {@link #excluded(String...)} method (the argument resolver will populate these with
* some sensible defaults).
* </p>
* <p>
* The type parameter <code>T</code> in <code>ProxyExchange&lt;T&gt;</code> is the type of
Expand Down Expand Up @@ -137,12 +137,6 @@
*/
public class ProxyExchange<T> {

/**
* Contains headers that are considered case-sensitive by default.
*/
public static Set<String> DEFAULT_SENSITIVE = Collections
.unmodifiableSet(new HashSet<>(Arrays.asList("cookie", "authorization")));

private URI uri;

private RestTemplate rest;
Expand All @@ -157,7 +151,7 @@ public class ProxyExchange<T> {

private WebDataBinderFactory binderFactory;

private Set<String> sensitive;
private Set<String> excluded;

private HttpHeaders headers = new HttpHeaders();

Expand Down Expand Up @@ -210,19 +204,19 @@ public ProxyExchange<T> headers(HttpHeaders headers) {
}

/**
* Sets the names of sensitive headers that are not passed downstream to the backend
* Sets the names of excluded headers that are not passed downstream to the backend
* service.
* @param names the names of sensitive headers
* @param names the names of excluded headers
* @return this for convenience
*/
public ProxyExchange<T> sensitive(String... names) {
if (this.sensitive == null) {
this.sensitive = new HashSet<>();
public ProxyExchange<T> excluded(String... names) {
if (this.excluded == null) {
this.excluded = new HashSet<>();
}

this.sensitive.clear();
this.excluded.clear();
for (String name : names) {
this.sensitive.add(name.toLowerCase());
this.excluded.add(name.toLowerCase());
}
return this;
}
Expand Down Expand Up @@ -369,8 +363,8 @@ private Set<String> filterHeaderKeys(HttpHeaders headers) {
}

private Set<String> filterHeaderKeys(Collection<String> headerNames) {
final Set<String> sensitiveHeaders = this.sensitive != null ? this.sensitive : DEFAULT_SENSITIVE;
return headerNames.stream().filter(header -> !sensitiveHeaders.contains(header.toLowerCase()))
final Set<String> excludedHeaders = this.excluded != null ? this.excluded : Collections.emptySet();
return headerNames.stream().filter(header -> !excludedHeaders.contains(header.toLowerCase()))
.collect(Collectors.toSet());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResol

private Set<String> autoForwardedHeaders;

private Set<String> sensitive;
private Set<String> excluded;

public ProxyExchangeArgumentResolver(RestTemplate builder) {
this.rest = builder;
Expand All @@ -62,8 +62,8 @@ public void setAutoForwardedHeaders(Set<String> autoForwardedHeaders) {
: autoForwardedHeaders.stream().map(String::toLowerCase).collect(toSet());
}

public void setSensitive(Set<String> sensitive) {
this.sensitive = sensitive;
public void setExcluded(Set<String> excluded) {
this.excluded = excluded;
}

@Override
Expand All @@ -77,7 +77,7 @@ public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer m
ProxyExchange<?> proxy = new ProxyExchange<>(rest, webRequest, mavContainer, binderFactory, type(parameter));
configureHeaders(proxy);
configureAutoForwardedHeaders(proxy, webRequest);
configureSensitive(proxy);
configureExcluded(proxy);
return proxy;
}

Expand Down Expand Up @@ -115,9 +115,9 @@ private void configureAutoForwardedHeaders(final ProxyExchange<?> proxy, final N
}
}

private void configureSensitive(final ProxyExchange<?> proxy) {
if (sensitive != null) {
proxy.sensitive(sensitive.toArray(new String[0]));
private void configureExcluded(final ProxyExchange<?> proxy) {
if (excluded != null) {
proxy.excluded(excluded.toArray(new String[0]));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
*
* @author Dave Syer
* @author Tim Ysewyn
* @author Joris Kuipers
*
*/
@ConfigurationProperties("spring.cloud.gateway.proxy")
Expand All @@ -42,14 +43,19 @@ public class ProxyProperties {
private Map<String, String> headers = new LinkedHashMap<>();

/**
* A set of header names that should be send downstream by default.
* A set of header names that should be sent downstream by default.
*/
private Set<String> autoForward = new HashSet<>();

/**
* A set of sensitive header names that will not be sent downstream by default.
*/
private Set<String> sensitive = null;
private Set<String> sensitive = Set.of("cookie", "authorization");

/**
* A set of header names that will not be sent downstream because they could be problematic.
*/
private Set<String> skipped = Set.of("content-length", "host");

public Map<String, String> getHeaders() {
return headers;
Expand All @@ -75,6 +81,14 @@ public void setSensitive(Set<String> sensitive) {
this.sensitive = sensitive;
}

public Set<String> getSkipped() {
return skipped;
}

public void setSkipped(Set<String> skipped) {
this.skipped = skipped;
}

public HttpHeaders convertHeaders() {
HttpHeaders headers = new HttpHeaders();
for (String key : this.headers.keySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
package org.springframework.cloud.gateway.mvc.config;

import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
Expand Down Expand Up @@ -70,7 +72,14 @@ public boolean supports(Class<?> clazz) {
ProxyExchangeArgumentResolver resolver = new ProxyExchangeArgumentResolver(template);
resolver.setHeaders(proxy.convertHeaders());
resolver.setAutoForwardedHeaders(proxy.getAutoForward());
resolver.setSensitive(proxy.getSensitive()); // can be null
Set<String> excludedHeaderNames = new HashSet<>();
if (proxy.getSensitive() != null) {
excludedHeaderNames.addAll(proxy.getSensitive());
}
if (proxy.getSkipped() != null) {
excludedHeaderNames.addAll(proxy.getSkipped());
}
resolver.setExcluded(excludedHeaderNames);
return resolver;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public ProxyExchangeArgumentResolver proxyExchangeArgumentResolver(final ProxyPr
generateConfiguredRestTemplate());
resolver.setHeaders(proxy.convertHeaders());
resolver.setAutoForwardedHeaders(proxy.getAutoForward());
resolver.setSensitive(proxy.getSensitive());
resolver.setExcluded(proxy.getSensitive());
return resolver;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.RequestEntity;
import org.springframework.http.ResponseEntity;
import org.springframework.http.client.SimpleClientHttpRequestFactory;
Expand All @@ -56,8 +57,7 @@

import static org.assertj.core.api.Assertions.assertThat;

@SpringBootTest(properties = { "spring.cloud.gateway.proxy.auto-forward=Baz" },
webEnvironment = WebEnvironment.RANDOM_PORT)
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
@ContextConfiguration(classes = TestApplication.class)
public class ProductionConfigurationTests {

Expand Down Expand Up @@ -112,6 +112,21 @@ public void post() {
.isEqualTo("host=localhost:" + port + ";foo");
}

@Test
public void postJsonWithWhitespace() {
var json = """
{
"foo": "bar"
}""";

var headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON);
headers.setContentLength(json.length());
var request = new HttpEntity<>(json, headers);
assertThat(rest.postForEntity("/proxy/checkContentLength", request, Void.class).getStatusCode())
.isEqualTo(HttpStatus.OK);
}

@Test
public void forward() {
assertThat(rest.getForObject("/forward/foos/0", Foo.class).getName()).isEqualTo("bye");
Expand Down Expand Up @@ -424,7 +439,7 @@ public void postForwardForgetBody(@RequestBody byte[] body, ProxyExchange<?> pro
@GetMapping("/proxy/headers")
@SuppressWarnings("Duplicates")
public ResponseEntity<Map<String, List<String>>> headers(ProxyExchange<Map<String, List<String>>> proxy) {
proxy.sensitive("foo", "hello");
proxy.excluded("foo", "hello");
proxy.header("bar", "hello");
proxy.header("abc", "123");
proxy.header("hello", "world");
Expand All @@ -440,6 +455,12 @@ public ResponseEntity<Map<String, List<String>>> defaultSensitiveHeaders(
return proxy.uri(home.toString() + "/headers").get();
}

@PostMapping("/proxy/checkContentLength")
public ResponseEntity<?> checkContentLength(
ProxyExchange<byte[]> proxy) {
return proxy.uri(home.toString() + "/checkContentLength").post();
}

private <T> ResponseEntity<T> first(ResponseEntity<List<T>> response) {
return ResponseEntity.status(response.getStatusCode()).headers(response.getHeaders())
.body(response.getBody().iterator().next());
Expand Down Expand Up @@ -484,6 +505,15 @@ public List<Bar> bars(@RequestBody List<Foo> foos, @RequestHeader HttpHeaders he
return Arrays.asList(new Bar(custom + foos.iterator().next().getName()));
}

@PostMapping("/checkContentLength")
public ResponseEntity<?> checkContentLength(@RequestHeader(name = "Content-Length", required = false) Integer contentLength,
@RequestBody String json) {
if (contentLength != null && contentLength != json.length()) {
return ResponseEntity.badRequest().build();
}
return ResponseEntity.ok().build();
}

@GetMapping("/headers")
public Map<String, List<String>> headers(@RequestHeader HttpHeaders headers) {
return new LinkedMultiValueMap<>(headers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@
*
* <p>
* By default the incoming request body and headers are sent intact to the downstream
* service (with the exception of "sensitive" headers). To manipulate the downstream
* service (with the exception of "excluded" headers). To manipulate the downstream
* request there are "builder" style methods in {@link ProxyExchange}, but only the
* {@link #uri(String)} is mandatory. You can change the sensitive headers by calling the
* {@link #sensitive(String...)} method (Authorization and Cookie are sensitive by
* default).
* {@link #uri(String)} is mandatory. You can change the excluded headers by calling the
* {@link #excluded(String...)} method (the argument resolver will populate these with
* some sensible defaults).
* </p>
* <p>
* The type parameter <code>T</code> in <code>ProxyExchange&lt;T&gt;</code> is the type of
Expand Down Expand Up @@ -111,12 +111,6 @@
*/
public class ProxyExchange<T> {

/**
* Contains headers that are considered case-sensitive by default.
*/
public static Set<String> DEFAULT_SENSITIVE = Collections
.unmodifiableSet(new HashSet<>(Arrays.asList("cookie", "authorization")));

private HttpMethod httpMethod;

private URI uri;
Expand All @@ -131,7 +125,7 @@ public class ProxyExchange<T> {

private BindingContext bindingContext;

private Set<String> sensitive;
private Set<String> excluded;

private HttpHeaders headers = new HttpHeaders();

Expand Down Expand Up @@ -197,19 +191,19 @@ public ProxyExchange<T> headers(HttpHeaders headers) {
}

/**
* Sets the names of sensitive headers that are not passed downstream to the backend
* Sets the names of excluded headers that are not passed downstream to the backend
* service.
* @param names the names of sensitive headers
* @param names the names of excluded headers
* @return this for convenience
*/
public ProxyExchange<T> sensitive(String... names) {
if (this.sensitive == null) {
this.sensitive = new HashSet<>();
public ProxyExchange<T> excluded(String... names) {
if (this.excluded == null) {
this.excluded = new HashSet<>();
}

this.sensitive.clear();
this.excluded.clear();
for (String name : names) {
this.sensitive.add(name.toLowerCase());
this.excluded.add(name.toLowerCase());
}
return this;
}
Expand Down Expand Up @@ -389,8 +383,8 @@ private void addHeaders(HttpHeaders headers, HttpHeaders toAdd) {
}

private Set<String> filterHeaderKeys(HttpHeaders headers) {
final Set<String> sensitiveHeaders = this.sensitive != null ? this.sensitive : DEFAULT_SENSITIVE;
return headers.keySet().stream().filter(header -> !sensitiveHeaders.contains(header.toLowerCase()))
final Set<String> excludedHeaders = this.excluded != null ? this.excluded : Collections.emptySet();
return headers.keySet().stream().filter(header -> !excludedHeaders.contains(header.toLowerCase()))
.collect(Collectors.toSet());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResol

private Set<String> autoForwardedHeaders;

private Set<String> sensitive;
private Set<String> excluded;

public ProxyExchangeArgumentResolver(WebClient builder) {
this.rest = builder;
Expand All @@ -58,8 +58,8 @@ public void setAutoForwardedHeaders(Set<String> autoForwardedHeaders) {
this.autoForwardedHeaders = autoForwardedHeaders;
}

public void setSensitive(Set<String> sensitive) {
this.sensitive = sensitive;
public void setExcluded(Set<String> excluded) {
this.excluded = excluded;
}

@Override
Expand Down Expand Up @@ -87,8 +87,8 @@ public Mono<Object> resolveArgument(MethodParameter parameter, BindingContext bi
if (this.autoForwardedHeaders.size() > 0) {
proxy.headers(extractAutoForwardedHeaders(exchange));
}
if (sensitive != null) {
proxy.sensitive(sensitive.toArray(new String[0]));
if (excluded != null) {
proxy.excluded(excluded.toArray(new String[0]));
}
return Mono.just(proxy);
}
Expand Down
Loading

0 comments on commit b435ce3

Please sign in to comment.