Use the original query string when forwarding the request

to ensure compatibility with legacy apps Fixes gh-989
spring-cloud · Apr 26, 2016 · 8952cff · 8952cff
1 parent 1c3de94
commit 8952cff
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 12 deletions.
diff --git a/...core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java b/...core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java
@@ -230,14 +230,20 @@ public boolean isIncludedHeader(String headerName) {
 	}
 
 	public Map<String, Object> debug(String verb, String uri,
-			MultiValueMap<String, String> headers, MultiValueMap<String, String> params,
+									 MultiValueMap<String, String> headers, MultiValueMap<String, String> params,
+									 InputStream requestEntity) throws IOException {
+		return debug(verb, uri, headers, getQueryString(params), requestEntity);
+	}
+
+	public Map<String, Object> debug(String verb, String uri,
+			MultiValueMap<String, String> headers, String queryString,
 			InputStream requestEntity) throws IOException {
 		Map<String, Object> info = new LinkedHashMap<>();
 		if (this.traces != null) {
 			RequestContext context = RequestContext.getCurrentContext();
 			info.put("method", verb);
 			info.put("path", uri);
-			info.put("query", getQueryString(params));
+			info.put("query", queryString);
 			info.put("remote", true);
 			info.put("proxy", context.get("proxy"));
 			Map<String, Object> trace = new LinkedHashMap<>();
@@ -334,4 +340,8 @@ public String getQueryString(MultiValueMap<String, String> params) {
 		UriTemplate template = new UriTemplate("?" + query.toString().substring(1));
 		return template.expand(singles).toString();
 	}
+
+	public String formatQueryString(String queryString) {
+		return (queryString == null) ? "": "?" + queryString;
+	}
 }
diff --git a/...in/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/...in/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java
@@ -158,8 +158,6 @@ public Object run() {
 		HttpServletRequest request = context.getRequest();
 		MultiValueMap<String, String> headers = this.helper
 				.buildZuulRequestHeaders(request);
-		MultiValueMap<String, String> params = this.helper
-				.buildZuulRequestQueryParams(request);
 		String verb = getVerb(request);
 		InputStream requestEntity = getRequestBody(request);
 		if (request.getContentLength() < 0) {
@@ -171,7 +169,7 @@ public Object run() {
 
 		try {
 			HttpResponse response = forward(this.httpClient, verb, uri, request, headers,
-					params, requestEntity);
+					request.getQueryString(), requestEntity);
 			setResponse(response);
 		}
 		catch (Exception ex) {
@@ -248,9 +246,9 @@ public HttpUriRequest getRedirect(HttpRequest request,
 
 	private HttpResponse forward(HttpClient httpclient, String verb, String uri,
 			HttpServletRequest request, MultiValueMap<String, String> headers,
-			MultiValueMap<String, String> params, InputStream requestEntity)
+			String queryString, InputStream requestEntity)
 					throws Exception {
-		Map<String, Object> info = this.helper.debug(verb, uri, headers, params,
+		Map<String, Object> info = this.helper.debug(verb, uri, headers, queryString,
 				requestEntity);
 		URL host = RequestContext.getCurrentContext().getRouteHost();
 		HttpHost httpHost = getHttpHost(host);
@@ -261,24 +259,23 @@ private HttpResponse forward(HttpClient httpclient, String verb, String uri,
 				ContentType.create(request.getContentType()));
 		switch (verb.toUpperCase()) {
 		case "POST":
-			HttpPost httpPost = new HttpPost(uri + this.helper.getQueryString(params));
+			HttpPost httpPost = new HttpPost(uri + this.helper.formatQueryString(queryString));
 			httpRequest = httpPost;
 			httpPost.setEntity(entity);
 			break;
 		case "PUT":
-			HttpPut httpPut = new HttpPut(uri + this.helper.getQueryString(params));
+			HttpPut httpPut = new HttpPut(uri + this.helper.formatQueryString(queryString));
 			httpRequest = httpPut;
 			httpPut.setEntity(entity);
 			break;
 		case "PATCH":
-			HttpPatch httpPatch = new HttpPatch(uri + this.helper.getQueryString(params));
+			HttpPatch httpPatch = new HttpPatch(uri + this.helper.formatQueryString(queryString));
 			httpRequest = httpPatch;
 			httpPatch.setEntity(entity);
 			break;
 		default:
 			httpRequest = new BasicHttpRequest(verb,
-					uri + this.helper.getQueryString(params));
+					uri + this.helper.formatQueryString(queryString));
-			log.debug(uri + this.helper.getQueryString(params));
 		}
 		try {
 			httpRequest.setHeaders(convertHeaders(headers));

diff --git a/...src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java b/...src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java
@@ -259,4 +259,15 @@ public void getQueryStringWithEmptyParam() {
 		assertThat(queryString, is("?wsdl"));
 	}
 
+	@Test
+	public void formatQueryStringShouldPrependQuestionMark() {
+		String queryString = new ProxyRequestHelper().formatQueryString("a=1234&b=5678");
+
+		assertThat(queryString, is("?a=1234&b=5678"));
+	}
+
+	@Test
+	public void formatQueryStringShouldReturnEmptyStringForNullValue() {
+		assertThat(new ProxyRequestHelper().formatQueryString(null), is(""));
+	}
 }