Issue #2074 no merge query string (#4730)

* Issue #4713 Async dispatch with query.

+ Preserve the entire URI with query when startAsync(req,res) is used.
+ merge any query string from dispatch path with either original query or preserved query from forward

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4713 asyncDispatch with query parameters

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #2074 Do not merge query strings

Do not merge the query string itself, only the parameter map.
The query string is either the original or replaced by the one from the dispatch.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2020-03-31 14:26:25 +02:00 committed by GitHub
parent 3f7d04ff96
commit ef9e155118
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 22 additions and 58 deletions

View File

@ -2368,7 +2368,7 @@ public class Request implements HttpServletRequest
if (newQueryParams == null || newQueryParams.size() == 0)
mergedQueryParams = oldQueryParams == null ? NO_PARAMS : oldQueryParams;
else if (oldQueryParams == null || oldQueryParams.size() == 0)
mergedQueryParams = newQueryParams == null ? NO_PARAMS : newQueryParams;
mergedQueryParams = newQueryParams;
else
{
// Parameters values are accumulated.
@ -2380,38 +2380,7 @@ public class Request implements HttpServletRequest
resetParameters();
if (updateQueryString)
{
if (newQuery == null)
setQueryString(oldQuery);
else if (oldQuery == null)
setQueryString(newQuery);
else if (oldQueryParams.keySet().stream().anyMatch(newQueryParams.keySet()::contains))
{
// Build the new merged query string, parameters in the
// new query string hide parameters in the old query string.
StringBuilder mergedQuery = new StringBuilder();
if (newQuery != null)
mergedQuery.append(newQuery);
for (Map.Entry<String, List<String>> entry : mergedQueryParams.entrySet())
{
if (newQueryParams != null && newQueryParams.containsKey(entry.getKey()))
continue;
for (String value : entry.getValue())
{
if (mergedQuery.length() > 0)
mergedQuery.append("&");
URIUtil.encodePath(mergedQuery, entry.getKey());
mergedQuery.append('=');
URIUtil.encodePath(mergedQuery, value);
}
}
setQueryString(mergedQuery.toString());
}
else
{
setQueryString(newQuery + '&' + oldQuery);
}
}
setQueryString(newQuery == null ? oldQuery : newQuery);
}
@Override

View File

@ -105,14 +105,14 @@ public class AsyncContextDispatchWithQueryStrings
{
AsyncContext async = request.startAsync();
async.dispatch("/secondDispatchNewValueForExistingQueryString?newQueryString=newValue");
assertEquals("newQueryString=initialValue&initialParam=right", queryString);
assertEquals("newQueryString=initialValue", queryString);
}
else
{
response.setContentType("text/html");
response.setStatus(HttpServletResponse.SC_OK);
response.getWriter().println("<h1>woohhooooo</h1>");
assertEquals("newQueryString=newValue&initialParam=right", queryString);
assertEquals("newQueryString=newValue", queryString);
}
}
}

View File

@ -533,12 +533,12 @@ public class AsyncServletTest
assertThat(response, startsWith("HTTP/1.1 200 OK"));
assertThat(__history, contains(
"FWD REQUEST /ctx/fwd/info?start=200&dispatch=20",
"FORWARD /ctx/path1?forward=true&start=200&dispatch=20",
"FORWARD /ctx/path1?forward=true",
"initial",
"start",
"dispatch",
"FWD ASYNC /ctx/fwd/info?start=200&dispatch=20",
"FORWARD /ctx/path1?forward=true&start=200&dispatch=20",
"FORWARD /ctx/path1?forward=true",
"!initial",
"onComplete"));
assertContains("DISPATCHED", response);
@ -551,7 +551,7 @@ public class AsyncServletTest
assertThat(response, startsWith("HTTP/1.1 200 OK"));
assertThat(__history, contains(
"FWD REQUEST /ctx/fwd/info?start=200&dispatch=20&path=/path2",
"FORWARD /ctx/path1?forward=true&start=200&dispatch=20&path=/path2",
"FORWARD /ctx/path1?forward=true",
"initial",
"start",
"dispatch",
@ -568,11 +568,11 @@ public class AsyncServletTest
assertThat(response, startsWith("HTTP/1.1 200 OK"));
assertThat(__history, contains(
"FWD REQUEST /ctx/fwd/info?wrap=true&start=200&dispatch=20",
"FORWARD /ctx/path1?forward=true&wrap=true&start=200&dispatch=20",
"FORWARD /ctx/path1?forward=true",
"initial",
"start",
"dispatch",
"ASYNC /ctx/path1?forward=true&wrap=true&start=200&dispatch=20",
"ASYNC /ctx/path1?forward=true",
"wrapped REQ RSP",
"!initial",
"onComplete"));
@ -586,11 +586,11 @@ public class AsyncServletTest
assertThat(response, startsWith("HTTP/1.1 200 OK"));
assertThat(__history, contains(
"FWD REQUEST /ctx/fwd/info?wrap=true&start=200&dispatch=20&path=/path2",
"FORWARD /ctx/path1?forward=true&wrap=true&start=200&dispatch=20&path=/path2",
"FORWARD /ctx/path1?forward=true",
"initial",
"start",
"dispatch",
"ASYNC /ctx/path2?forward=true&wrap=true&start=200&dispatch=20&path=/path2",
"ASYNC /ctx/path2?forward=true",
"wrapped REQ RSP",
"!initial",
"onComplete"));

View File

@ -142,7 +142,6 @@ public class DispatcherForwardTest
CountDownLatch latch = new CountDownLatch(1);
final String query1 = "a=1%20one&b=2%20two";
final String query2 = "a=3%20three";
final String query3 = "a=3%20three&b=2%20two";
servlet1 = new HttpServlet()
{
@Override
@ -163,7 +162,7 @@ public class DispatcherForwardTest
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
checkThat(req.getQueryString(), Matchers.equalTo(query3));
checkThat(req.getQueryString(), Matchers.equalTo(query2));
checkThat(req.getParameter("a"), Matchers.equalTo("3 three"));
checkThat(req.getParameter("b"), Matchers.equalTo("2 two"));
}
@ -186,13 +185,12 @@ public class DispatcherForwardTest
{
// 1. request /one?a=1
// 1. forward /two?b=2
// 2. assert query => a=1&b=2
// 2. assert query => b=2
// 1. assert query => a=1
CountDownLatch latch = new CountDownLatch(1);
final String query1 = "a=1%20one";
final String query2 = "b=2%20two";
final String query3 = "b=2%20two&a=1%20one";
servlet1 = new HttpServlet()
{
@Override
@ -212,7 +210,7 @@ public class DispatcherForwardTest
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
checkThat(req.getQueryString(), Matchers.equalTo(query3));
checkThat(req.getQueryString(), Matchers.equalTo(query2));
checkThat(req.getParameter("a"), Matchers.equalTo("1 one"));
checkThat(req.getParameter("b"), Matchers.equalTo("2 two"));
}
@ -348,13 +346,12 @@ public class DispatcherForwardTest
{
// 1. request /one?a=1 + content b=2
// 1. forward /two?c=3
// 2. assert query => a=1&c=3 + params => a=1&b=2&c=3
// 2. assert query => c=3 + params => a=1&b=2&c=3
// 1. assert query => a=1 + params => a=1&b=2
CountDownLatch latch = new CountDownLatch(1);
final String query1 = "a=1%20one";
final String query2 = "c=3%20three";
final String query3 = "c=3%20three&a=1%20one";
final String form = "b=2%20two";
servlet1 = new HttpServlet()
{
@ -377,7 +374,7 @@ public class DispatcherForwardTest
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
checkThat(req.getQueryString(), Matchers.equalTo(query3));
checkThat(req.getQueryString(), Matchers.equalTo(query2));
checkThat(req.getParameter("a"), Matchers.equalTo("1 one"));
checkThat(req.getParameter("b"), Matchers.equalTo("2 two"));
checkThat(req.getParameter("c"), Matchers.equalTo("3 three"));
@ -405,13 +402,12 @@ public class DispatcherForwardTest
// 1. request /one?a=1 + content b=2
// 1. assert params => a=1&b=2
// 1. forward /two?c=3
// 2. assert query => a=1&c=3 + params => a=1&b=2&c=3
// 2. assert query => c=3 + params => a=1&b=2&c=3
// 1. assert query => a=1 + params => a=1&b=2
CountDownLatch latch = new CountDownLatch(1);
final String query1 = "a=1%20one";
final String query2 = "c=3%20three";
final String query3 = "c=3%20three&a=1%20one";
final String form = "b=2%20two";
servlet1 = new HttpServlet()
{
@ -436,7 +432,7 @@ public class DispatcherForwardTest
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
checkThat(req.getQueryString(), Matchers.equalTo(query3));
checkThat(req.getQueryString(), Matchers.equalTo(query2));
checkThat(req.getParameter("a"), Matchers.equalTo("1 one"));
checkThat(req.getParameter("b"), Matchers.equalTo("2 two"));
checkThat(req.getParameter("c"), Matchers.equalTo("3 three"));
@ -513,7 +509,6 @@ public class DispatcherForwardTest
CountDownLatch latch = new CountDownLatch(1);
final String query1 = "a=1%20one";
final String query2 = "b=2%20two";
final String query3 = "b=2%20two&a=1%20one";
final String form = "c=3%20three";
servlet1 = new HttpServlet()
{
@ -534,7 +529,7 @@ public class DispatcherForwardTest
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
checkThat(req.getQueryString(), Matchers.equalTo(query3));
checkThat(req.getQueryString(), Matchers.equalTo(query2));
ServletInputStream input = req.getInputStream();
for (int i = 0; i < form.length(); ++i)
{

View File

@ -743,7 +743,7 @@ public class DispatcherTest
assertEquals(null, request.getPathInfo());
assertEquals(null, request.getPathTranslated());
assertEquals("do=end&do=the&test=1", request.getQueryString());
assertEquals("do=end&do=the", request.getQueryString());
assertEquals("/context/AssertForwardServlet", request.getRequestURI());
assertEquals("/context", request.getContextPath());
assertEquals("/AssertForwardServlet", request.getServletPath());
@ -788,8 +788,8 @@ public class DispatcherTest
q2.decode(query.getString("else"));
String russian = q2.encode();
assertThat(russian, is("%D0%B2%D1%8B%D0%B1%D1%80%D0%B0%D0%BD%D0%BE=%D0%A2%D0%B5%D0%BC%D0%BF%D0%B5%D1%80%D0%B0%D1%82%D1%83%D1%80%D0%B0"));
assertThat(query.getString("test"), is("1"));
assertThat(query.containsKey("foreign"), is(true));
assertThat(query.containsKey("test"), is(false));
assertThat(query.containsKey("foreign"), is(false));
String[] vals = request.getParameterValues("foreign");
assertTrue(vals != null);