Commit bd6d732f by Jacques-Etienne Beaudet Committed by Spencer Gibb

Fix potential decoding and reencoding issues in query params when using…

Fix potential decoding and reencoding issues in query params when using SimpleHostRoutingFilter (#1464) Some webserver are picky with the encoding (uWSGI for example) and the current code behavior of decoding and reencoding the query string can lead to valid requests rejected by the backend after the modifications in Zuul. This commit adds an optional parameter to force the original encoding of the query string in SimpleHostRoutingFilter. Fixes gh-971
parent 4eac8a5a
......@@ -1638,6 +1638,26 @@ $ curl -v -H "Transfer-Encoding: chunked" \
-F "file=@mylarge.iso" localhost:9999/zuul/simple/file
----
=== Query String Encoding
When processing the incoming request, query params are decoded so they can be available for possible modifications in
Zuul filters. They are then re-encoded when building the backend request in the route filters. The result
can be different than the original input if it was encoded using Javascript's `encodeURIComponent()` method for example.
While this causes no issues in most cases, some web servers can be picky with the encoding of complex query string.
To force the original encoding of the query string, it is possible to pass a special flag to `ZuulProperties` so
that the query string is taken as is with the `HttpServletRequest::getQueryString` method :
.application.yml
[source,yaml]
----
zuul:
forceOriginalQueryStringEncoding: true
----
*Note:* This special flag only works with `SimpleHostRoutingFilter` and you loose the ability to easily override
query parameters with `RequestContext.getCurrentContext().setRequestQueryParams(someOverriddenParameters)` since
the query string is now fetched directly on the original `HttpServletRequest`.
=== Plain Embedded Zuul
You can also run a Zuul server without the proxying, or switch on parts of the proxying platform selectively, if you
......
......@@ -109,6 +109,15 @@ public class ZuulProperties {
* see https://docs.spring.io/spring-security/site/docs/current/reference/html/headers.html#default-security-headers
*/
private boolean ignoreSecurityHeaders = true;
/**
* Flag to force the original query string encoding when building the backend URI in
* SimpleHostRoutingFilter. When activated, query string will be built using
* HttpServletRequest getQueryString() method instead of UriTemplate. Note that this
* flag is not used in RibbonRoutingFilter with services found via DiscoveryClient
* (like Eureka).
*/
private boolean forceOriginalQueryStringEncoding = false;
/**
* Path to install Zuul as a servlet (not part of Spring MVC). The servlet is more
......
......@@ -96,6 +96,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
"SimpleHostRoutingFilter.connectionManagerTimer", true);
private boolean sslHostnameValidationEnabled;
private boolean forceOriginalQueryStringEncoding;
private ProxyRequestHelper helper;
private Host hostProperties;
......@@ -119,6 +120,8 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
this.helper = helper;
this.hostProperties = properties.getHost();
this.sslHostnameValidationEnabled = properties.isSslHostnameValidationEnabled();
this.forceOriginalQueryStringEncoding = properties
.isForceOriginalQueryStringEncoding();
}
@PostConstruct
......@@ -275,7 +278,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
request.getContentType() != null
? ContentType.create(request.getContentType()) : null);
HttpRequest httpRequest = buildHttpRequest(verb, uri, entity, headers, params);
HttpRequest httpRequest = buildHttpRequest(verb, uri, entity, headers, params, request);
try {
log.debug(httpHost.getHostName() + " " + httpHost.getPort() + " "
+ httpHost.getSchemeName());
......@@ -292,43 +295,49 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
}
}
protected HttpRequest buildHttpRequest (String verb, String uri, InputStreamEntity entity,
MultiValueMap<String, String> headers, MultiValueMap<String, String> params)
{
protected HttpRequest buildHttpRequest(String verb, String uri,
InputStreamEntity entity, MultiValueMap<String, String> headers,
MultiValueMap<String, String> params, HttpServletRequest request) {
HttpRequest httpRequest;
String uriWithQueryString = uri + (this.forceOriginalQueryStringEncoding
? getEncodedQueryString(request) : this.helper.getQueryString(params));
switch (verb.toUpperCase()) {
case "POST":
HttpPost httpPost = new HttpPost(uri + this.helper.getQueryString(params));
HttpPost httpPost = new HttpPost(uriWithQueryString);
httpRequest = httpPost;
httpPost.setEntity(entity);
break;
case "PUT":
HttpPut httpPut = new HttpPut(uri + this.helper.getQueryString(params));
HttpPut httpPut = new HttpPut(uriWithQueryString);
httpRequest = httpPut;
httpPut.setEntity(entity);
break;
case "PATCH":
HttpPatch httpPatch = new HttpPatch(uri + this.helper.getQueryString(params));
HttpPatch httpPatch = new HttpPatch(uriWithQueryString);
httpRequest = httpPatch;
httpPatch.setEntity(entity);
break;
case "DELETE":
BasicHttpEntityEnclosingRequest entityRequest = new BasicHttpEntityEnclosingRequest(verb,
uri + this.helper.getQueryString(params));
BasicHttpEntityEnclosingRequest entityRequest = new BasicHttpEntityEnclosingRequest(
verb, uriWithQueryString);
httpRequest = entityRequest;
entityRequest.setEntity(entity);
break;
default:
httpRequest = new BasicHttpRequest(verb,
uri + this.helper.getQueryString(params));
log.debug(uri + this.helper.getQueryString(params));
httpRequest = new BasicHttpRequest(verb, uriWithQueryString);
log.debug(uriWithQueryString);
}
httpRequest.setHeaders(convertHeaders(headers));
return httpRequest;
}
private String getEncodedQueryString(HttpServletRequest request) {
String query = request.getQueryString();
return (query != null) ? "?" + query : "";
}
private MultiValueMap<String, String> revertHeaders(Header[] headers) {
MultiValueMap<String, String> map = new LinkedMultiValueMap<String, String>();
for (Header header : headers) {
......@@ -399,4 +408,4 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
boolean isSslHostnameValidationEnabled() {
return this.sslHostnameValidationEnabled;
}
}
}
\ No newline at end of file
/*
* Copyright 2013-2015 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.cloud.netflix.zuul;
import static org.junit.Assert.*;
import java.net.URI;
import java.net.URISyntaxException;
import javax.servlet.http.HttpServletRequest;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
import org.springframework.boot.test.web.client.TestRestTemplate;
import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientRouteLocator;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import com.netflix.zuul.context.RequestContext;
@RunWith(SpringJUnit4ClassRunner.class)
@SpringBootTest(classes = SimpleZuulProxyApplicationTests.SimpleZuulProxyApplication.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = {
"server.port: 0", "zuul.forceOriginalQueryStringEncoding: true" })
@DirtiesContext
public class SimpleZuulProxyApplicationTests {
@Value("${local.server.port}")
private int port;
@Autowired
private DiscoveryClientRouteLocator routes;
@Autowired
private RoutesEndpoint endpoint;
@Before
public void setTestRequestcontext() {
RequestContext context = new RequestContext();
RequestContext.testSetCurrentContext(context);
this.routes.addRoute("/foo/**", "http://localhost:" + this.port + "/bar");
this.endpoint.reset();
}
@Test
public void getOnSelfViaSimpleHostRoutingFilter() {
ResponseEntity<String> result = executeSimpleRequest(HttpMethod.GET);
assertResponseCodeAndBody(result, "get bar");
}
@Test
public void postOnSelfViaSimpleHostRoutingFilter() {
ResponseEntity<String> result = executeSimpleRequest(HttpMethod.POST);
assertResponseCodeAndBody(result, "post bar");
}
@Test
public void putOnSelfViaSimpleHostRoutingFilter() {
ResponseEntity<String> result = executeSimpleRequest(HttpMethod.PUT);
assertResponseCodeAndBody(result, "put bar");
}
@Test
public void patchOnSelfViaSimpleHostRoutingFilter() {
ResponseEntity<String> result = executeSimpleRequest(HttpMethod.PATCH);
assertResponseCodeAndBody(result, "patch bar");
}
@Test
public void deleteOnSelfViaSimpleHostRoutingFilter() {
ResponseEntity<String> result = executeSimpleRequest(HttpMethod.DELETE);
assertResponseCodeAndBody(result, "delete bar");
}
@Test
public void getOnSelfWithComplexQueryParam() throws URISyntaxException {
String encodedQueryString = "foo=%7B%22project%22%3A%22stream%22%2C%22logger%22%3A%22javascript%22%2C%22platform%22%3A%22javascript%22%2C%22request%22%3A%7B%22url%22%3A%22https%3A%2F%2Ffoo%2Fadmin";
ResponseEntity<String> result = new TestRestTemplate().exchange(
new URI("http://localhost:" + this.port + "/foo?" + encodedQueryString),
HttpMethod.GET, new HttpEntity<>((Void) null), String.class);
assertEquals(HttpStatus.OK, result.getStatusCode());
assertEquals(encodedQueryString, result.getBody());
}
private void assertResponseCodeAndBody(ResponseEntity<String> result,
String expectedBody) {
assertEquals(HttpStatus.OK, result.getStatusCode());
assertEquals(expectedBody, result.getBody());
}
private ResponseEntity<String> executeSimpleRequest(HttpMethod httpMethod) {
ResponseEntity<String> result = new TestRestTemplate().exchange(
"http://localhost:" + this.port + "/foo?id=bar", httpMethod,
new HttpEntity<>((Void) null), String.class);
return result;
}
// Don't use @SpringBootApplication because we don't want to component scan
@Configuration
@EnableAutoConfiguration
@RestController
@EnableZuulProxy
static class SimpleZuulProxyApplication {
@RequestMapping(value = "/bar", method = RequestMethod.GET)
public String get(@RequestParam String id) {
return "get " + id;
}
@RequestMapping(value = "/bar", method = RequestMethod.GET, params = { "foo" })
public String complexGet(@RequestParam String foo, HttpServletRequest request) {
return request.getQueryString();
}
@RequestMapping(value = "/bar", method = RequestMethod.POST)
public String post(@RequestParam String id) {
return "post " + id;
}
@RequestMapping(value = "/bar", method = RequestMethod.PUT)
public String put(@RequestParam String id) {
return "put " + id;
}
@RequestMapping(value = "/bar", method = RequestMethod.DELETE)
public String delete(@RequestParam String id) {
return "delete " + id;
}
@RequestMapping(value = "/bar", method = RequestMethod.PATCH)
public String patch(@RequestParam String id) {
return "patch " + id;
}
}
}
\ No newline at end of file
......@@ -31,6 +31,7 @@ import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.util.LinkedMultiValueMap;
import static org.junit.Assert.assertEquals;
......@@ -91,7 +92,7 @@ public class SimpleHostRoutingFilterTests {
setupContext();
InputStreamEntity inputStreamEntity = new InputStreamEntity(new ByteArrayInputStream(new byte[]{1}));
HttpRequest httpRequest = getFilter().buildHttpRequest("DELETE", "uri", inputStreamEntity,
new LinkedMultiValueMap<String, String>(), new LinkedMultiValueMap<String, String>());
new LinkedMultiValueMap<String, String>(), new LinkedMultiValueMap<String, String>(), new MockHttpServletRequest());
assertTrue(httpRequest instanceof HttpEntityEnclosingRequest);
HttpEntityEnclosingRequest httpEntityEnclosingRequest = (HttpEntityEnclosingRequest) httpRequest;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment