From ef9e15511834ed8d85b3d1a07145e2b122e76d53 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 31 Mar 2020 14:26:25 +0200 Subject: [PATCH] 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 * Issue #4713 asyncDispatch with query parameters Signed-off-by: Greg Wilkins * 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 --- .../org/eclipse/jetty/server/Request.java | 35 ++----------------- .../AsyncContextDispatchWithQueryStrings.java | 4 +-- .../jetty/servlet/AsyncServletTest.java | 14 ++++---- .../jetty/servlet/DispatcherForwardTest.java | 21 +++++------ .../eclipse/jetty/servlet/DispatcherTest.java | 6 ++-- 5 files changed, 22 insertions(+), 58 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 07f2d318be4..0572a6abd16 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -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> 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 diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextDispatchWithQueryStrings.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextDispatchWithQueryStrings.java index 00fb163cd7d..b174c80a9e5 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextDispatchWithQueryStrings.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextDispatchWithQueryStrings.java @@ -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("

woohhooooo

"); - assertEquals("newQueryString=newValue&initialParam=right", queryString); + assertEquals("newQueryString=newValue", queryString); } } } diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletTest.java index f04cffee271..5db4e65f897 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletTest.java @@ -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")); diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherForwardTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherForwardTest.java index b73f73320e7..4e933e75ba9 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherForwardTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherForwardTest.java @@ -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) { diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java index 9cacec898b7..8c886a54149 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java @@ -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);