Issue #4713 Async dispatch with query. (#4721)

* 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>

jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java

@@ -760,15 +760,15 @@ public void setDecodedPath(String path)
         _decodedPath = path;
     }
 
-    public void setPathQuery(String path)
+    public void setPathQuery(String pathQuery)
     {
         _uri = null;
         _path = null;
         _decodedPath = null;
         _param = null;
         _fragment = null;
-        if (path != null)
-            parse(State.PATH, path);
+        if (pathQuery != null)
+            parse(State.PATH, pathQuery);
     }
 
     public void setQuery(String query)

jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContextEvent.java

@@ -25,25 +25,33 @@
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 
+import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.server.handler.ContextHandler.Context;
 import org.eclipse.jetty.util.thread.Scheduler;
 
 public class AsyncContextEvent extends AsyncEvent implements Runnable
 {
     private final Context _context;
     private final AsyncContextState _asyncContext;
+    private final HttpURI _baseURI;
     private volatile HttpChannelState _state;
     private ServletContext _dispatchContext;
     private String _dispatchPath;
     private volatile Scheduler.Task _timeoutTask;
     private Throwable _throwable;
 
     public AsyncContextEvent(Context context, AsyncContextState asyncContext, HttpChannelState state, Request baseRequest, ServletRequest request, ServletResponse response)
+    {
+        this (context, asyncContext, state, baseRequest, request, response, null);
+    }
+
+    public AsyncContextEvent(Context context, AsyncContextState asyncContext, HttpChannelState state, Request baseRequest, ServletRequest request, ServletResponse response, HttpURI baseURI)
     {
         super(null, request, response, null);
         _context = context;
         _asyncContext = asyncContext;
         _state = state;
+        _baseURI = baseURI;
 
         // If we haven't been async dispatched before
         if (baseRequest.getAttribute(AsyncContext.ASYNC_REQUEST_URI) == null)
@@ -74,6 +82,11 @@ public AsyncContextEvent(Context context, AsyncContextState asyncContext, HttpCh
         }
     }
 
+    public HttpURI getBaseURI()
+    {
+        return _baseURI;
+    }
+
     public ServletContext getSuspendedContext()
     {
         return _context;
@@ -94,14 +107,6 @@ public ServletContext getServletContext()
         return _dispatchContext == null ? _context : _dispatchContext;
     }
 
-    /**
-     * @return The path in the context (encoded with possible query string)
-     */
-    public String getPath()
-    {
-        return _dispatchPath;
-    }
-
     public void setTimeoutTask(Scheduler.Task task)
     {
         _timeoutTask = task;
@@ -137,6 +142,14 @@ public void setDispatchContext(ServletContext context)
         _dispatchContext = context;
     }
 
+    /**
+     * @return The path in the context (encoded with possible query string)
+     */
+    public String getDispatchPath()
+    {
+        return _dispatchPath;
+    }
+
     /**
      * @param path encoded URI
      */

jetty-server/src/main/java/org/eclipse/jetty/server/Request.java

@@ -1598,7 +1598,10 @@ public void setHttpURI(HttpURI uri)
     {
         MetaData.Request metadata = _metaData;
         if (metadata != null)
+        {
             metadata.setURI(uri);
+            _queryParameters = null;
+        }
     }
 
     public UserIdentity getUserIdentity()
@@ -2178,17 +2181,8 @@ public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse se
         HttpChannelState state = getHttpChannelState();
         if (_async == null)
             _async = new AsyncContextState(state);
-        AsyncContextEvent event = new AsyncContextEvent(_context, _async, state, this, servletRequest, servletResponse);
+        AsyncContextEvent event = new AsyncContextEvent(_context, _async, state, this, servletRequest, servletResponse, getHttpURI());
         event.setDispatchContext(getServletContext());
-
-        String uri = unwrap(servletRequest).getRequestURI();
-        if (_contextPath != null && uri.startsWith(_contextPath))
-            uri = uri.substring(_contextPath.length());
-        else
-            // TODO probably need to strip encoded context from requestURI, but will do this for now:
-            uri = URIUtil.encodePath(URIUtil.addPaths(getServletPath(), getPathInfo()));
-
-        event.setDispatchPath(uri);
         state.startAsync(event);
         return _async;
     }
@@ -2391,7 +2385,7 @@ else if (oldQueryParams == null || oldQueryParams.size() == 0)
                 setQueryString(oldQuery);
             else if (oldQuery == null)
                 setQueryString(newQuery);
-            else
+            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.
@@ -2413,6 +2407,10 @@ else if (oldQuery == null)
                 }
                 setQueryString(mergedQuery.toString());
             }
+            else
+            {
+                setQueryString(newQuery + '&' + oldQuery);
+            }
         }
     }
 

jetty-server/src/main/java/org/eclipse/jetty/server/Server.java

@@ -47,6 +47,8 @@
 import org.eclipse.jetty.util.Attributes;
 import org.eclipse.jetty.util.Jetty;
 import org.eclipse.jetty.util.MultiException;
+import org.eclipse.jetty.util.MultiMap;
+import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.Uptime;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
@@ -563,22 +565,74 @@ public void handleAsync(HttpChannel channel) throws IOException, ServletExceptio
     {
         final HttpChannelState state = channel.getRequest().getHttpChannelState();
         final AsyncContextEvent event = state.getAsyncContextEvent();
-
         final Request baseRequest = channel.getRequest();
-        final String path = event.getPath();
+        final HttpURI baseUri = event.getBaseURI();
+        String encodedPathQuery = event.getDispatchPath();
 
-        if (path != null)
+        if (encodedPathQuery == null && baseUri == null)
         {
-            // this is a dispatch with a path
-            ServletContext context = event.getServletContext();
-            String query = baseRequest.getQueryString();
-            baseRequest.setURIPathQuery(URIUtil.addEncodedPaths(context == null ? null : URIUtil.encodePath(context.getContextPath()), path));
-            HttpURI uri = baseRequest.getHttpURI();
-            baseRequest.setPathInfo(uri.getDecodedPath());
-            if (uri.getQuery() != null)
-                baseRequest.mergeQueryParameters(query, uri.getQuery(), true); //we have to assume dispatch path and query are UTF8
+            // Simple case, no request modification or merging needed
+            handleAsync(channel, event, baseRequest);
+            return;
         }
 
+        // this is a dispatch with either a provided URI and/or a dispatched path
+        // We will have to modify the request and then revert
+        final ServletContext context = event.getServletContext();
+        final HttpURI oldUri = baseRequest.getHttpURI();
+        final String oldQuery = baseRequest.getQueryString();
+        final MultiMap<String> oldQueryParams = baseRequest.getQueryParameters();
+        try
+        {
+            baseRequest.resetParameters();
+            HttpURI newUri = baseUri == null ? new HttpURI(oldUri) : baseUri;
+            if (encodedPathQuery == null)
+            {
+                baseRequest.setHttpURI(newUri);
+            }
+            else
+            {
+                if (context != null && !StringUtil.isEmpty(context.getContextPath()))
+                    encodedPathQuery = URIUtil.addEncodedPaths(URIUtil.encodePath(context.getContextPath()), encodedPathQuery);
+
+                if (newUri.getQuery() == null)
+                {
+                    // parse new path and query
+                    newUri.setPathQuery(encodedPathQuery);
+                    baseRequest.setHttpURI(newUri);
+                }
+                else
+                {
+                    // do we have a new query in the encodedPathQuery
+                    int q = encodedPathQuery.indexOf('?');
+                    if (q < 0)
+                    {
+                        // No query, so we can just set the encoded path
+                        newUri.setPath(encodedPathQuery);
+                        baseRequest.setHttpURI(newUri);
+                    }
+                    else
+                    {
+                        newUri.setPath(encodedPathQuery.substring(0, q));
+                        baseRequest.setHttpURI(newUri);
+                        baseRequest.mergeQueryParameters(oldQuery, encodedPathQuery.substring(q + 1), true);
+                    }
+                }
+            }
+
+            baseRequest.setPathInfo(newUri.getDecodedPath());
+            handleAsync(channel, event, baseRequest);
+        }
+        finally
+        {
+            baseRequest.setHttpURI(oldUri);
+            baseRequest.setQueryParameters(oldQueryParams);
+            baseRequest.resetParameters();
+        }
+    }
+
+    private void handleAsync(HttpChannel channel, AsyncContextEvent event, Request baseRequest) throws IOException, ServletException
+    {
         final String target = baseRequest.getPathInfo();
         final HttpServletRequest request = Request.unwrap(event.getSuppliedRequest());
         final HttpServletResponse response = Response.unwrap(event.getSuppliedResponse());