From 67258446f95e668f471cc6816dd067cb5efe8399 Mon Sep 17 00:00:00 2001 From: Nicolas Byl Date: Thu, 11 Feb 2016 16:34:56 +0100 Subject: [PATCH] Improved query string handling to present empty keys correctly I.e. show empty "foo" as foo (not foo=) Fixes gh-825 --- .../zuul/filters/ProxyRequestHelper.java | 25 ++++++++++------- .../route/SimpleHostRoutingFilter.java | 27 ++++--------------- .../zuul/filters/ProxyRequestHelperTests.java | 25 +++++++++++++++++ 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java index b6379cd2..df12c036 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java @@ -203,18 +203,9 @@ public class ProxyRequestHelper { Map info = new LinkedHashMap(); if (this.traces != null) { RequestContext context = RequestContext.getCurrentContext(); - StringBuilder query = new StringBuilder(); - for (String param : params.keySet()) { - for (String value : params.get(param)) { - query.append(param); - query.append("="); - query.append(value); - query.append("&"); - } - } info.put("method", verb); info.put("path", uri); - info.put("query", query.toString()); + info.put("query", getQueryString(params)); info.put("remote", true); info.put("proxy", context.get("proxy")); Map trace = new LinkedHashMap(); @@ -287,4 +278,18 @@ public class ProxyRequestHelper { } } + public String getQueryString(MultiValueMap params) { + StringBuilder query = new StringBuilder(); + for (String param : params.keySet()) { + for (String value : params.get(param)) { + query.append("&"); + query.append(param); + if(!"".equals(value)) { + query.append("="); + query.append(value); + } + } + } + return (query.length() > 0) ? "?" + query.substring(1) : ""; + } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java index f231d7b9..33efcda6 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java @@ -262,23 +262,23 @@ public class SimpleHostRoutingFilter extends ZuulFilter { ContentType.create(request.getContentType())); switch (verb.toUpperCase()) { case "POST": - HttpPost httpPost = new HttpPost(uri + getQueryString()); + HttpPost httpPost = new HttpPost(uri + this.helper.getQueryString(params)); httpRequest = httpPost; httpPost.setEntity(entity); break; case "PUT": - HttpPut httpPut = new HttpPut(uri + getQueryString()); + HttpPut httpPut = new HttpPut(uri + this.helper.getQueryString(params)); httpRequest = httpPut; httpPut.setEntity(entity); break; case "PATCH": - HttpPatch httpPatch = new HttpPatch(uri + getQueryString()); + HttpPatch httpPatch = new HttpPatch(uri + this.helper.getQueryString(params)); httpRequest = httpPatch; httpPatch.setEntity(entity); break; default: - httpRequest = new BasicHttpRequest(verb, uri + getQueryString()); - log.debug(uri + getQueryString()); + httpRequest = new BasicHttpRequest(verb, uri + this.helper.getQueryString(params)); + log.debug(uri + this.helper.getQueryString(params)); } try { httpRequest.setHeaders(convertHeaders(headers)); @@ -324,23 +324,6 @@ public class SimpleHostRoutingFilter extends ZuulFilter { return httpclient.execute(httpHost, httpRequest); } - private String getQueryString() throws UnsupportedEncodingException { - HttpServletRequest request = RequestContext.getCurrentContext().getRequest(); - MultiValueMap params = this.helper - .buildZuulRequestQueryParams(request); - StringBuilder query = new StringBuilder(); - for (Map.Entry> entry : params.entrySet()) { - String key = URLEncoder.encode(entry.getKey(), "UTF-8"); - for (String value : entry.getValue()) { - query.append("&"); - query.append(key); - query.append("="); - query.append(URLEncoder.encode(value, "UTF-8")); - } - } - return (query.length() > 0) ? "?" + query.substring(1) : ""; - } - private HttpHost getHttpHost(URL host) { HttpHost httpHost = new HttpHost(host.getHost(), host.getPort(), host.getProtocol()); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java index 0bd3d9d8..dcc6da35 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java @@ -17,7 +17,10 @@ package org.springframework.cloud.netflix.zuul.filters; import java.io.IOException; +import java.util.Collection; import java.util.List; +import java.util.Map; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -25,6 +28,7 @@ import org.mockito.Mock; import org.springframework.boot.actuate.trace.InMemoryTraceRepository; import org.springframework.boot.actuate.trace.Trace; import org.springframework.boot.actuate.trace.TraceRepository; +import org.springframework.boot.autoconfigure.web.ResourceProperties; import org.springframework.http.HttpHeaders; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -157,4 +161,25 @@ public class ProxyRequestHelperTests { helper.setResponse(200, request.getInputStream(), headers); assertTrue(context.getResponseGZipped()); } + + @Test + public void getQueryString(){ + MultiValueMap params = new LinkedMultiValueMap<>(); + params.add("a", "1234"); + params.add("b", "5678"); + + String queryString = new ProxyRequestHelper().getQueryString(params); + + assertThat(queryString, is("?a=1234&b=5678")); + } + + @Test + public void getQueryStringWithEmptyParam(){ + MultiValueMap params = new LinkedMultiValueMap<>(); + params.add("wsdl", ""); + + String queryString = new ProxyRequestHelper().getQueryString(params); + + assertThat(queryString, is("?wsdl")); + } }