Skip to content

S
spring-cloud-netflix
Commit d21a7ef8 by Jacques-Etienne Beaudet
Options

Fix an issue where the FormBodyWrapperFilter would encode the form parameters…

Fix an issue where the FormBodyWrapperFilter would encode the form parameters differently than on the original request. The FormBodyWrapperFilter handles the application/x-www-form-urlencoded. In the case of requests received by curl or javascript, the FormHttpMessageConverter will reencode the parameters differently (for example, '(' will be encoded while it's not with the javascript encodeURIComponent method). While this doesn't create any problem, the content length was not properly set in AbstractRibbonCommand. This causes the form params being stripped or the backend server would wait a long time for additional bytes depending on if the content length header was bigger/smaller than the actual data.
parent 8b911e60
Showing with 153 additions and 4 deletions
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/FormBodyWrapperFilter.java
......@@ -26,6 +26,8 @@ import java.util.Map.Entry;
import java.util.Set;
import javax.servlet.ServletInputStream;
import javax.servlet.ServletRequest;
import javax.servlet.ServletRequestWrapper;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.Part;
......@@ -57,13 +59,19 @@ import com.netflix.zuul.http.ServletInputStreamWrapper;
public class FormBodyWrapperFilter extends ZuulFilter {
private Field requestField;
private Field servletRequestField;
public FormBodyWrapperFilter() {
this.requestField = ReflectionUtils.findField(HttpServletRequestWrapper.class,
"req", HttpServletRequest.class);
this.servletRequestField = ReflectionUtils.findField(ServletRequestWrapper.class,
"request", ServletRequest.class);
Assert.notNull(this.requestField,
"HttpServletRequestWrapper.req field not found");
Assert.notNull(this.servletRequestField,
"ServletRequestWrapper.request field not found");
this.requestField.setAccessible(true);
this.servletRequestField.setAccessible(true);
}
@Override
......@@ -113,6 +121,9 @@ public class FormBodyWrapperFilter extends ZuulFilter {
.getField(this.requestField, request);
wrapper = new FormBodyRequestWrapper(wrapped);
ReflectionUtils.setField(this.requestField, request, wrapper);
if(request instanceof ServletRequestWrapper) {
ReflectionUtils.setField(this.servletRequestField, request, wrapper);
}
}
else {
wrapper = new FormBodyRequestWrapper(request);
......@@ -161,6 +172,11 @@ public class FormBodyWrapperFilter extends ZuulFilter {
}
@Override
public long getContentLengthLong() {
return getContentLength();
}
@Override
public ServletInputStream getInputStream() throws IOException {
if (this.contentData == null) {
buildContentData();
......
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java
......@@ -74,9 +74,9 @@ public abstract class AbstractRibbonCommand<LBC extends AbstractLoadBalancerAwar
@Override
protected ClientHttpResponse run() throws Exception {
final RequestContext context = RequestContext.getCurrentContext();
String contentLengthHeader = context.getRequest().getHeader("Content-Length");
if (StringUtils.hasText(contentLengthHeader)) {
this.context.setContentLength(new Long(contentLengthHeader));
long contentLength = context.getRequest().getContentLengthLong();
if (contentLength != -1) {
this.context.setContentLength(contentLength);
}
RQ request = createRequest();
......
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java
......@@ -24,6 +24,8 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.UUID;
import javax.servlet.http.HttpServletRequest;
......@@ -47,6 +49,7 @@ import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonComm
import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommandFactory;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory;
import org.springframework.cloud.netflix.zuul.filters.route.support.NoEncodingFormHttpMessageConverter;
import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase;
import org.springframework.cloud.netflix.zuul.filters.RouteLocator;
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
......@@ -59,10 +62,14 @@ import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.http.client.ClientHttpResponse;
import org.springframework.http.converter.HttpMessageConverter;
import org.springframework.http.converter.StringHttpMessageConverter;
import org.springframework.mock.http.client.MockClientHttpResponse;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.context.web.WebAppConfiguration;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.bind.annotation.MatrixVariable;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
......@@ -230,6 +237,24 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase {
this.ribbonCommandFactory instanceof TestConfig.MyRibbonCommandFactory);
}
@Override
@SuppressWarnings("deprecation")
@Test
public void javascriptEncodedFormParams() {
TestRestTemplate testRestTemplate = new TestRestTemplate();
ArrayList<HttpMessageConverter<?>> converters = new ArrayList<>();
converters.addAll(Arrays.asList(new StringHttpMessageConverter(),
new NoEncodingFormHttpMessageConverter()));
testRestTemplate.setMessageConverters(converters);
MultiValueMap<String, String> map = new LinkedMultiValueMap<>();
map.add("foo", "(bar)");
ResponseEntity<String> result = testRestTemplate.postForEntity(
"http://localhost:" + this.port + "/simple/local", map, String.class);
assertEquals(HttpStatus.OK, result.getStatusCode());
assertEquals("Posted [(bar)] and Content-Length was: -1!", result.getBody());
}
@Test
public void routeLocatorOverridden() {
assertTrue("routeLocator not a MyRouteLocator",
......
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/NoEncodingFormHttpMessageConverter.java 0 → 100644
/*
* Copyright 2013-2016 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.filters.route.support;
import java.io.IOException;
import java.util.Iterator;
import org.springframework.http.HttpOutputMessage;
import org.springframework.http.MediaType;
import org.springframework.http.converter.FormHttpMessageConverter;
import org.springframework.http.converter.HttpMessageNotWritableException;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StreamUtils;
/**
* @author Jacques-Etienne Beaudet
*/
public class NoEncodingFormHttpMessageConverter extends FormHttpMessageConverter {
@SuppressWarnings("unchecked")
@Override
public void write(MultiValueMap<String, ?> map, MediaType contentType, HttpOutputMessage outputMessage)
throws IOException, HttpMessageNotWritableException {
MultiValueMap<String, String> form = (MultiValueMap<String, String>) map;
StringBuilder builder = new StringBuilder();
for (Iterator<String> nameIterator = form.keySet().iterator(); nameIterator.hasNext();) {
String name = nameIterator.next();
for (Iterator<String> valueIterator = form.get(name).iterator(); valueIterator.hasNext();) {
String value = valueIterator.next();
builder.append(name);
if (value != null) {
builder.append('=');
builder.append(value);
if (valueIterator.hasNext()) {
builder.append('&');
}
}
}
if (nameIterator.hasNext()) {
builder.append('&');
}
}
final byte[] bytes = builder.toString().getBytes(FormHttpMessageConverter.DEFAULT_CHARSET);
outputMessage.getHeaders().setContentLength(bytes.length);
outputMessage.getHeaders().setContentType(MediaType.APPLICATION_FORM_URLENCODED);
StreamUtils.copy(bytes, outputMessage.getBody());
}
}
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java
......@@ -17,6 +17,8 @@
package org.springframework.cloud.netflix.zuul.filters.route.support;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
......@@ -25,7 +27,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
import javax.servlet.http.HttpServletRequest;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Test;
import org.springframework.beans.factory.annotation.Autowired;
......@@ -45,7 +46,12 @@ 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.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.FormHttpMessageConverter;
import org.springframework.http.converter.HttpMessageConverter;
import org.springframework.http.converter.StringHttpMessageConverter;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestBody;
......@@ -53,6 +59,7 @@ 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.servlet.config.annotation.DelegatingWebMvcConfiguration;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;
import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping;
import com.netflix.loadbalancer.Server;
......@@ -248,6 +255,23 @@ public abstract class ZuulProxyTestBase {
assertEquals("Patched 1!", result.getBody());
}
@SuppressWarnings("deprecation")
@Test
public void javascriptEncodedFormParams() {
TestRestTemplate testRestTemplate = new TestRestTemplate();
ArrayList<HttpMessageConverter<?>> converters = new ArrayList<>();
converters.addAll(Arrays.asList(new StringHttpMessageConverter(),
new NoEncodingFormHttpMessageConverter()));
testRestTemplate.setMessageConverters(converters);
MultiValueMap<String, String> map = new LinkedMultiValueMap<>();
map.add("foo", "(bar)");
ResponseEntity<String> result = testRestTemplate.postForEntity(
"http://localhost:" + this.port + "/simple/local", map, String.class);
assertEquals(HttpStatus.OK, result.getStatusCode());
assertEquals("Posted [(bar)] and Content-Length was: 13!", result.getBody());
}
protected abstract boolean supportsPatch();
public static abstract class AbstractZuulProxyApplication
......@@ -269,6 +293,12 @@ public abstract class ZuulProxyTestBase {
return "Hello local";
}
@RequestMapping(value = "/local", method = RequestMethod.POST)
public String postWithFormParam(HttpServletRequest request,
@RequestBody MultiValueMap<String, String> body) {
return "Posted " + body.get("foo") + " and Content-Length was: " + request.getContentLength() + "!";
}
@RequestMapping(value = "/local/{id}", method = RequestMethod.DELETE)
public String delete(@PathVariable String id) {
return "Deleted " + id + "!";
......@@ -342,6 +372,19 @@ public abstract class ZuulProxyTestBase {
}
}
@Configuration
public class FormEncodedMessageConverterConfiguration extends WebMvcConfigurerAdapter {
@Override
public void configureMessageConverters(List<HttpMessageConverter<?>> converters) {
FormHttpMessageConverter converter = new FormHttpMessageConverter();
MediaType mediaType = new MediaType("application", "x-www-form-urlencoded", Charset.forName("UTF-8"));
converter.setSupportedMediaTypes(Arrays.asList(mediaType));
converters.add(converter);
super.configureMessageConverters(converters);
}
}
// Load balancer with fixed server list for "simple" pointing to localhost
@Configuration
public static class SimpleRibbonClientConfiguration {
......
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