Skip to content

Commit

Permalink
spring-cloud#3154: prevent Content-Length and Host headers from being…
Browse files Browse the repository at this point in the history 
… copied

Also remove the bogus spring.cloud.gateway.proxy.auto-forward setting from the tests
  • Loading branch information
@jkuipers
jkuipers committed Mar 21, 2024
1 parent cc62626 commit d96ce91
Show file tree
Hide file tree
Showing 12 changed files with 234 additions and 147 deletions.
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/spring-cloud-gateway-proxy-exchange.adoc
View file
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.

75 changes: 34 additions & 41 deletions ...-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/ProxyExchange.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,6 @@

package org.springframework.cloud.gateway.mvc;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.lang.reflect.WildcardType;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.Vector;
import java.util.function.Function;
import java.util.stream.Collectors;

import jakarta.servlet.ReadListener;
import jakarta.servlet.ServletInputStream;
import jakarta.servlet.ServletOutputStream;
Expand All @@ -44,7 +24,6 @@
import jakarta.servlet.http.HttpServletRequestWrapper;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpServletResponseWrapper;

import org.springframework.core.Conventions;
import org.springframework.core.MethodParameter;
import org.springframework.core.ParameterizedTypeReference;
Expand All @@ -69,6 +48,26 @@
import org.springframework.web.servlet.HandlerMapping;
import org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.lang.reflect.WildcardType;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.Vector;
import java.util.function.Function;
import java.util.stream.Collectors;

/**
* A <code>@RequestMapping</code> argument type that can proxy the request to a backend.
* Spring will inject one of these into your MVC handler method, and you get return a
Expand All @@ -85,11 +84,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 +136,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 +150,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 +203,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 +362,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
27 changes: 13 additions & 14 deletions ...main/java/org/springframework/cloud/gateway/mvc/config/ProxyExchangeArgumentResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,7 @@

package org.springframework.cloud.gateway.mvc.config;

import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.Collections;
import java.util.Enumeration;
import java.util.Set;

import jakarta.servlet.http.HttpServletRequest;

import org.springframework.cloud.gateway.mvc.ProxyExchange;
import org.springframework.core.MethodParameter;
import org.springframework.http.HttpHeaders;
Expand All @@ -33,6 +26,12 @@
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.method.support.ModelAndViewContainer;

import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.Collections;
import java.util.Enumeration;
import java.util.Set;

import static java.util.stream.Collectors.toSet;

/**
Expand All @@ -47,7 +46,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 +61,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 +76,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 +114,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
26 changes: 20 additions & 6 deletions ...teway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyProperties.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,22 @@

package org.springframework.cloud.gateway.mvc.config;

import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.cloud.gateway.mvc.ProxyExchange;
import org.springframework.http.HttpHeaders;

import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;

import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.cloud.gateway.mvc.ProxyExchange;
import org.springframework.http.HttpHeaders;

/**
* Configuration properties for the {@link ProxyExchange} argument handler in
* <code>@RequestMapping</code> methods.
*
* @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
19 changes: 14 additions & 5 deletions ...ain/java/org/springframework/cloud/gateway/mvc/config/ProxyResponseAutoConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@

package org.springframework.cloud.gateway.mvc.config;

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

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
Expand All @@ -38,6 +34,12 @@
import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

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

/**
* Autoconfiguration for the {@link ProxyExchange} argument handler in Spring MVC
* <code>@RequestMapping</code> methods.
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
11 changes: 5 additions & 6 deletions ...eway-mvc/src/test/java/org/springframework/cloud/gateway/mvc/GetWithBodyRequestTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,9 @@

package org.springframework.cloud.gateway.mvc;

import java.io.IOException;
import java.net.URI;
import java.util.Collections;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.test.context.SpringBootTest;
Expand Down Expand Up @@ -52,6 +47,10 @@
import org.springframework.web.client.DefaultResponseErrorHandler;
import org.springframework.web.client.RestTemplate;

import java.io.IOException;
import java.net.URI;
import java.util.Collections;

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

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
Expand Down Expand Up @@ -111,7 +110,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
Loading

0 comments on commit d96ce91

Please sign in to comment.