diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java index 8a3a11d55aa..dcfea4168c6 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.util.Collections; import java.util.Enumeration; import java.util.Locale; - import javax.servlet.RequestDispatcher; import javax.servlet.ServletException; import javax.servlet.ServletRequest; @@ -360,7 +359,7 @@ public class FormAuthenticator extends LoginAuthenticator { LOG.debug("auth rePOST {}->{}",authentication,j_uri); Request base_request = HttpChannel.getCurrentHttpChannel().getRequest(); - base_request.setParameters(j_post); + base_request.setContentParameters(j_post); } session.removeAttribute(__J_URI); session.removeAttribute(__J_METHOD); @@ -395,8 +394,9 @@ public class FormAuthenticator extends LoginAuthenticator if (MimeTypes.Type.FORM_ENCODED.is(req.getContentType()) && HttpMethod.POST.is(request.getMethod())) { Request base_request = (req instanceof Request)?(Request)req:HttpChannel.getCurrentHttpChannel().getRequest(); - base_request.extractParameters(); - session.setAttribute(__J_POST, new MultiMap(base_request.getParameters())); + MultiMap formParameters = new MultiMap<>(); + base_request.extractFormParameters(formParameters); + session.setAttribute(__J_POST, formParameters); } } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java index a3db34869db..25672897948 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; - import javax.servlet.DispatcherType; import javax.servlet.RequestDispatcher; import javax.servlet.ServletException; @@ -34,13 +33,7 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.MultiMap; -import org.eclipse.jetty.util.UrlEncoded; -/* ------------------------------------------------------------ */ -/** Servlet RequestDispatcher. - * - * - */ public class Dispatcher implements RequestDispatcher { /** Dispatch include attribute names */ @@ -49,71 +42,41 @@ public class Dispatcher implements RequestDispatcher /** Dispatch include attribute names */ public final static String __FORWARD_PREFIX="javax.servlet.forward."; - /** JSP attributes */ - public final static String __JSP_FILE="org.apache.catalina.jsp_file"; - - /* ------------------------------------------------------------ */ private final ContextHandler _contextHandler; private final String _uri; private final String _path; - private final String _dQuery; + private final String _query; private final String _named; - /* ------------------------------------------------------------ */ - /** - * @param contextHandler - * @param uri - * @param pathInContext - * @param query - */ public Dispatcher(ContextHandler contextHandler, String uri, String pathInContext, String query) { _contextHandler=contextHandler; _uri=uri; _path=pathInContext; - _dQuery=query; + _query=query; _named=null; } - - /* ------------------------------------------------------------ */ - /** Constructor. - * @param contextHandler - * @param name - */ - public Dispatcher(ContextHandler contextHandler,String name) - throws IllegalStateException + public Dispatcher(ContextHandler contextHandler, String name) throws IllegalStateException { _contextHandler=contextHandler; _named=name; _uri=null; _path=null; - _dQuery=null; + _query=null; } - /* ------------------------------------------------------------ */ - /* - * @see javax.servlet.RequestDispatcher#forward(javax.servlet.ServletRequest, javax.servlet.ServletResponse) - */ @Override public void forward(ServletRequest request, ServletResponse response) throws ServletException, IOException { forward(request, response, DispatcherType.FORWARD); } - /* ------------------------------------------------------------ */ - /* - * @see javax.servlet.RequestDispatcher#forward(javax.servlet.ServletRequest, javax.servlet.ServletResponse) - */ public void error(ServletRequest request, ServletResponse response) throws ServletException, IOException { forward(request, response, DispatcherType.ERROR); } - /* ------------------------------------------------------------ */ - /* - * @see javax.servlet.RequestDispatcher#include(javax.servlet.ServletRequest, javax.servlet.ServletResponse) - */ @Override public void include(ServletRequest request, ServletResponse response) throws ServletException, IOException { @@ -126,70 +89,48 @@ public class Dispatcher implements RequestDispatcher final DispatcherType old_type = baseRequest.getDispatcherType(); final Attributes old_attr=baseRequest.getAttributes(); - MultiMap old_params=baseRequest.getParameters(); + final MultiMap old_query_params=baseRequest.getQueryParameters(); try { baseRequest.setDispatcherType(DispatcherType.INCLUDE); baseRequest.getResponse().include(); if (_named!=null) + { _contextHandler.handle(_named,baseRequest, (HttpServletRequest)request, (HttpServletResponse)response); + } else { - String query=_dQuery; - - if (query!=null) - { - // force parameter extraction - if (old_params==null) - { - baseRequest.extractParameters(); - old_params=baseRequest.getParameters(); - } - - MultiMap parameters=new MultiMap<>(); - UrlEncoded.decodeTo(query,parameters,baseRequest.getCharacterEncoding(),-1); - - if(old_params != null) { - // Merge parameters. - parameters.addAllValues(old_params); - } - baseRequest.setParameters(parameters); - } - IncludeAttributes attr = new IncludeAttributes(old_attr); attr._requestURI=_uri; attr._contextPath=_contextHandler.getContextPath(); attr._servletPath=null; // set by ServletHandler attr._pathInfo=_path; - attr._query=query; + attr._query=_query; + if (_query!=null) + baseRequest.mergeQueryParameters(_query, false); baseRequest.setAttributes(attr); - _contextHandler.handle(_path,baseRequest, (HttpServletRequest)request, (HttpServletResponse)response); + _contextHandler.handle(_path, baseRequest, (HttpServletRequest)request, (HttpServletResponse)response); } } finally { baseRequest.setAttributes(old_attr); baseRequest.getResponse().included(); - baseRequest.setParameters(old_params); + baseRequest.setQueryParameters(old_query_params); + baseRequest.resetParameters(); baseRequest.setDispatcherType(old_type); } } - - /* ------------------------------------------------------------ */ - /* - * @see javax.servlet.RequestDispatcher#forward(javax.servlet.ServletRequest, javax.servlet.ServletResponse) - */ protected void forward(ServletRequest request, ServletResponse response, DispatcherType dispatch) throws ServletException, IOException { Request baseRequest=(request instanceof Request)?((Request)request):HttpChannel.getCurrentHttpChannel().getRequest(); Response base_response=baseRequest.getResponse(); base_response.resetForForward(); - if (!(request instanceof HttpServletRequest)) request = new ServletRequestHttpWrapper(request); if (!(response instanceof HttpServletResponse)) @@ -201,9 +142,9 @@ public class Dispatcher implements RequestDispatcher final String old_servlet_path=baseRequest.getServletPath(); final String old_path_info=baseRequest.getPathInfo(); final String old_query=baseRequest.getQueryString(); + final MultiMap old_query_params=baseRequest.getQueryParameters(); final Attributes old_attr=baseRequest.getAttributes(); final DispatcherType old_type=baseRequest.getDispatcherType(); - MultiMap old_params=baseRequest.getParameters(); try { @@ -211,24 +152,11 @@ public class Dispatcher implements RequestDispatcher baseRequest.setDispatcherType(dispatch); if (_named!=null) - _contextHandler.handle(_named,baseRequest, (HttpServletRequest)request, (HttpServletResponse)response); + { + _contextHandler.handle(_named, baseRequest, (HttpServletRequest)request, (HttpServletResponse)response); + } else { - - // process any query string from the dispatch URL - String query=_dQuery; - if (query!=null) - { - // force parameter extraction - if (old_params==null) - { - baseRequest.extractParameters(); - old_params=baseRequest.getParameters(); - } - - baseRequest.mergeQueryString(query); - } - ForwardAttributes attr = new ForwardAttributes(old_attr); //If we have already been forwarded previously, then keep using the established @@ -256,9 +184,11 @@ public class Dispatcher implements RequestDispatcher baseRequest.setContextPath(_contextHandler.getContextPath()); baseRequest.setServletPath(null); baseRequest.setPathInfo(_uri); + if (_query!=null) + baseRequest.mergeQueryParameters(_query, true); baseRequest.setAttributes(attr); - _contextHandler.handle(_path,baseRequest, (HttpServletRequest)request, (HttpServletResponse)response); + _contextHandler.handle(_path, baseRequest, (HttpServletRequest)request, (HttpServletResponse)response); if (!baseRequest.getHttpChannelState().isAsync()) commitResponse(response,baseRequest); @@ -271,15 +201,14 @@ public class Dispatcher implements RequestDispatcher baseRequest.setContextPath(old_context_path); baseRequest.setServletPath(old_servlet_path); baseRequest.setPathInfo(old_path_info); - baseRequest.setAttributes(old_attr); - baseRequest.setParameters(old_params); baseRequest.setQueryString(old_query); + baseRequest.setQueryParameters(old_query_params); + baseRequest.resetParameters(); + baseRequest.setAttributes(old_attr); baseRequest.setDispatcherType(old_type); } } - - /* ------------------------------------------------------------ */ private void commitResponse(ServletResponse response, Request baseRequest) throws IOException { if (baseRequest.getResponse().isWriting()) @@ -306,10 +235,6 @@ public class Dispatcher implements RequestDispatcher } } - - /* ------------------------------------------------------------ */ - /* ------------------------------------------------------------ */ - /* ------------------------------------------------------------ */ private class ForwardAttributes implements Attributes { final Attributes _attr; @@ -349,7 +274,6 @@ public class Dispatcher implements RequestDispatcher return _attr.getAttribute(key); } - /* ------------------------------------------------------------ */ @Override public Enumeration getAttributeNames() { @@ -381,7 +305,6 @@ public class Dispatcher implements RequestDispatcher return Collections.enumeration(set); } - /* ------------------------------------------------------------ */ @Override public void setAttribute(String key, Object value) { @@ -409,21 +332,18 @@ public class Dispatcher implements RequestDispatcher _attr.setAttribute(key,value); } - /* ------------------------------------------------------------ */ @Override public String toString() { return "FORWARD+"+_attr.toString(); } - /* ------------------------------------------------------------ */ @Override public void clearAttributes() { throw new IllegalStateException(); } - /* ------------------------------------------------------------ */ @Override public void removeAttribute(String name) { @@ -431,7 +351,6 @@ public class Dispatcher implements RequestDispatcher } } - /* ------------------------------------------------------------ */ private class IncludeAttributes implements Attributes { final Attributes _attr; @@ -447,9 +366,6 @@ public class Dispatcher implements RequestDispatcher _attr=attributes; } - /* ------------------------------------------------------------ */ - /* ------------------------------------------------------------ */ - /* ------------------------------------------------------------ */ @Override public Object getAttribute(String key) { @@ -468,7 +384,6 @@ public class Dispatcher implements RequestDispatcher return _attr.getAttribute(key); } - /* ------------------------------------------------------------ */ @Override public Enumeration getAttributeNames() { @@ -499,7 +414,6 @@ public class Dispatcher implements RequestDispatcher return Collections.enumeration(set); } - /* ------------------------------------------------------------ */ @Override public void setAttribute(String key, Object value) { @@ -521,21 +435,18 @@ public class Dispatcher implements RequestDispatcher _attr.setAttribute(key,value); } - /* ------------------------------------------------------------ */ @Override public String toString() { return "INCLUDE+"+_attr.toString(); } - /* ------------------------------------------------------------ */ @Override public void clearAttributes() { throw new IllegalStateException(); } - /* ------------------------------------------------------------ */ @Override public void removeAttribute(String name) { 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 adf325ef1e6..d1e10f22924 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 @@ -40,7 +40,6 @@ import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; - import javax.servlet.AsyncContext; import javax.servlet.AsyncListener; import javax.servlet.DispatcherType; @@ -177,7 +176,6 @@ public class Request implements HttpServletRequest private boolean _requestedSessionIdFromCookie = false; private volatile Attributes _attributes; private Authentication _authentication; - private MultiMap _baseParameters; private String _characterEncoding; private ContextHandler.Context _context; private String _contextPath; @@ -186,6 +184,8 @@ public class Request implements HttpServletRequest private int _inputState = __NONE; private HttpMethod _httpMethod; private String _httpMethodString; + private MultiMap _queryParameters; + private MultiMap _contentParameters; private MultiMap _parameters; private String _pathInfo; private int _port; @@ -237,148 +237,43 @@ public class Request implements HttpServletRequest throw new IllegalArgumentException(listener.getClass().toString()); } - /* ------------------------------------------------------------ */ - /** - * Extract Parameters from query string and/or form _content. - */ public void extractParameters() { - if (_baseParameters == null) - _baseParameters = new MultiMap<>(); - if (_paramsExtracted) - { - if (_parameters == null) - _parameters = _baseParameters; return; - } _paramsExtracted = true; - try + // Extract query string parameters; these may be replaced by a forward() + // and may have already been extracted by mergeQueryParameters(). + if (_queryParameters == null) + _queryParameters = extractQueryParameters(); + + // Extract content parameters; these cannot be replaced by a forward() + // once extracted and may have already been extracted by getParts() or + // by a processing happening after a form-based authentication. + if (_contentParameters == null) + _contentParameters = extractContentParameters(); + + _parameters = restoreParameters(); + } + + private MultiMap extractQueryParameters() + { + MultiMap result = new MultiMap<>(); + if (_uri != null && _uri.hasQuery()) { - // Handle query string - if (_uri != null && _uri.hasQuery()) + if (_queryEncoding == null) { - if (_queryEncoding == null) - _uri.decodeQueryTo(_baseParameters); - else - { - try - { - _uri.decodeQueryTo(_baseParameters,_queryEncoding); - } - catch (UnsupportedEncodingException e) - { - if (LOG.isDebugEnabled()) - LOG.warn(e); - else - LOG.warn(e.toString()); - } - } + _uri.decodeQueryTo(result); } - - // handle any _content. - String encoding = getCharacterEncoding(); - String content_type = getContentType(); - if (content_type != null && content_type.length() > 0) - { - content_type = HttpFields.valueParameters(content_type,null); - - if (MimeTypes.Type.FORM_ENCODED.is(content_type) && _inputState == __NONE && - (HttpMethod.POST.is(getMethod()) || HttpMethod.PUT.is(getMethod()))) - { - int content_length = getContentLength(); - if (content_length != 0) - { - try - { - int maxFormContentSize = -1; - int maxFormKeys = -1; - - if (_context != null) - { - maxFormContentSize = _context.getContextHandler().getMaxFormContentSize(); - maxFormKeys = _context.getContextHandler().getMaxFormKeys(); - } - - if (maxFormContentSize < 0) - { - Object obj = _channel.getServer().getAttribute("org.eclipse.jetty.server.Request.maxFormContentSize"); - if (obj == null) - maxFormContentSize = 200000; - else if (obj instanceof Number) - { - Number size = (Number)obj; - maxFormContentSize = size.intValue(); - } - else if (obj instanceof String) - { - maxFormContentSize = Integer.valueOf((String)obj); - } - } - - if (maxFormKeys < 0) - { - Object obj = _channel.getServer().getAttribute("org.eclipse.jetty.server.Request.maxFormKeys"); - if (obj == null) - maxFormKeys = 1000; - else if (obj instanceof Number) - { - Number keys = (Number)obj; - maxFormKeys = keys.intValue(); - } - else if (obj instanceof String) - { - maxFormKeys = Integer.valueOf((String)obj); - } - } - - if (content_length > maxFormContentSize && maxFormContentSize > 0) - { - throw new IllegalStateException("Form too large " + content_length + ">" + maxFormContentSize); - } - InputStream in = getInputStream(); - if (_input.isAsync()) - throw new IllegalStateException("Cannot extract parameters with async IO"); - - // Add form params to query params - UrlEncoded.decodeTo(in,_baseParameters,encoding,content_length < 0?maxFormContentSize:-1,maxFormKeys); - } - catch (IOException e) - { - if (LOG.isDebugEnabled()) - LOG.warn(e); - else - LOG.warn(e.toString()); - } - } - } - - } - - if (_parameters == null) - _parameters = _baseParameters; - else if (_parameters != _baseParameters) - { - // Merge parameters (needed if parameters extracted after a forward). - _parameters.addAllValues(_baseParameters); - } - - if (content_type != null && content_type.length()>0 && content_type.startsWith("multipart/form-data") && getAttribute(__MULTIPART_CONFIG_ELEMENT)!=null) + else { try { - getParts(); + _uri.decodeQueryTo(result, _queryEncoding); } - catch (IOException e) - { - if (LOG.isDebugEnabled()) - LOG.warn(e); - else - LOG.warn(e.toString()); - } - catch (ServletException e) + catch (UnsupportedEncodingException e) { if (LOG.isDebugEnabled()) LOG.warn(e); @@ -387,11 +282,114 @@ public class Request implements HttpServletRequest } } } - finally + return result; + } + + private MultiMap extractContentParameters() + { + MultiMap result = new MultiMap<>(); + + String contentType = getContentType(); + if (contentType != null && !contentType.isEmpty()) { - // ensure params always set (even if empty) after extraction - if (_parameters == null) - _parameters = _baseParameters; + contentType = HttpFields.valueParameters(contentType, null); + int contentLength = getContentLength(); + if (contentLength != 0) + { + if (MimeTypes.Type.FORM_ENCODED.is(contentType) && _inputState == __NONE && + (HttpMethod.POST.is(getMethod()) || HttpMethod.PUT.is(getMethod()))) + { + extractFormParameters(result); + } + else if (contentType.startsWith("multipart/form-data") && + getAttribute(__MULTIPART_CONFIG_ELEMENT) != null && + _multiPartInputStream == null) + { + extractMultipartParameters(result); + } + } + } + + return result; + } + + public void extractFormParameters(MultiMap params) + { + try + { + int maxFormContentSize = -1; + int maxFormKeys = -1; + + if (_context != null) + { + maxFormContentSize = _context.getContextHandler().getMaxFormContentSize(); + maxFormKeys = _context.getContextHandler().getMaxFormKeys(); + } + + if (maxFormContentSize < 0) + { + Object obj = _channel.getServer().getAttribute("org.eclipse.jetty.server.Request.maxFormContentSize"); + if (obj == null) + maxFormContentSize = 200000; + else if (obj instanceof Number) + { + Number size = (Number)obj; + maxFormContentSize = size.intValue(); + } + else if (obj instanceof String) + { + maxFormContentSize = Integer.valueOf((String)obj); + } + } + + if (maxFormKeys < 0) + { + Object obj = _channel.getServer().getAttribute("org.eclipse.jetty.server.Request.maxFormKeys"); + if (obj == null) + maxFormKeys = 1000; + else if (obj instanceof Number) + { + Number keys = (Number)obj; + maxFormKeys = keys.intValue(); + } + else if (obj instanceof String) + { + maxFormKeys = Integer.valueOf((String)obj); + } + } + + int contentLength = getContentLength(); + if (contentLength > maxFormContentSize && maxFormContentSize > 0) + { + throw new IllegalStateException("Form too large: " + contentLength + " > " + maxFormContentSize); + } + InputStream in = getInputStream(); + if (_input.isAsync()) + throw new IllegalStateException("Cannot extract parameters with async IO"); + + UrlEncoded.decodeTo(in,params,getCharacterEncoding(),contentLength<0?maxFormContentSize:-1,maxFormKeys); + } + catch (IOException e) + { + if (LOG.isDebugEnabled()) + LOG.warn(e); + else + LOG.warn(e.toString()); + } + } + + private void extractMultipartParameters(MultiMap result) + { + try + { + getParts(result); + } + catch (IOException | ServletException e) + { + if (LOG.isDebugEnabled()) + LOG.warn(e); + else + LOG.warn(e.toString()); } } @@ -827,6 +825,8 @@ public class Request implements HttpServletRequest { if (!_paramsExtracted) extractParameters(); + if (_parameters == null) + _parameters = restoreParameters(); return _parameters.getValue(name,0); } @@ -839,7 +839,8 @@ public class Request implements HttpServletRequest { if (!_paramsExtracted) extractParameters(); - + if (_parameters == null) + _parameters = restoreParameters(); return Collections.unmodifiableMap(_parameters.toStringArrayMap()); } @@ -852,18 +853,11 @@ public class Request implements HttpServletRequest { if (!_paramsExtracted) extractParameters(); + if (_parameters == null) + _parameters = restoreParameters(); return Collections.enumeration(_parameters.keySet()); } - /* ------------------------------------------------------------ */ - /** - * @return Returns the parameters. - */ - public MultiMap getParameters() - { - return _parameters; - } - /* ------------------------------------------------------------ */ /* * @see javax.servlet.ServletRequest#getParameterValues(java.lang.String) @@ -873,12 +867,44 @@ public class Request implements HttpServletRequest { if (!_paramsExtracted) extractParameters(); + if (_parameters == null) + _parameters = restoreParameters(); List vals = _parameters.getValues(name); if (vals == null) return null; return vals.toArray(new String[vals.size()]); } + private MultiMap restoreParameters() + { + MultiMap result = new MultiMap<>(); + if (_queryParameters == null) + _queryParameters = extractQueryParameters(); + result.addAllValues(_queryParameters); + result.addAllValues(_contentParameters); + return result; + } + + public MultiMap getQueryParameters() + { + return _queryParameters; + } + + public void setQueryParameters(MultiMap queryParameters) + { + _queryParameters = queryParameters; + } + + public void setContentParameters(MultiMap contentParameters) + { + _contentParameters = contentParameters; + } + + public void resetParameters() + { + _parameters = null; + } + /* ------------------------------------------------------------ */ /* * @see javax.servlet.http.HttpServletRequest#getPathInfo() @@ -1620,8 +1646,8 @@ public class Request implements HttpServletRequest _servletPath = null; _timeStamp = 0; _uri = null; - if (_baseParameters != null) - _baseParameters.clear(); + _queryParameters = null; + _contentParameters = null; _parameters = null; _paramsExtracted = false; _inputState = __NONE; @@ -1858,18 +1884,6 @@ public class Request implements HttpServletRequest return HttpMethod.HEAD==_httpMethod; } - /* ------------------------------------------------------------ */ - /** - * @param parameters - * The parameters to set. - */ - public void setParameters(MultiMap parameters) - { - _parameters = (parameters == null)?_baseParameters:parameters; - if (_paramsExtracted && _parameters == null) - throw new IllegalStateException(); - } - /* ------------------------------------------------------------ */ /** * @param pathInfo @@ -2103,10 +2117,14 @@ public class Request implements HttpServletRequest { if (getContentType() == null || !getContentType().startsWith("multipart/form-data")) throw new ServletException("Content-Type != multipart/form-data"); + return getParts(null); + } + private Collection getParts(MultiMap params) throws IOException, ServletException + { if (_multiPartInputStream == null) _multiPartInputStream = (MultiPartInputStreamParser)getAttribute(__MULTIPART_INPUT_STREAM); - + if (_multiPartInputStream == null) { MultipartConfigElement config = (MultipartConfigElement)getAttribute(__MULTIPART_CONFIG_ELEMENT); @@ -2127,20 +2145,20 @@ public class Request implements HttpServletRequest MultiPartInputStreamParser.MultiPart mp = (MultiPartInputStreamParser.MultiPart)p; if (mp.getContentDispositionFilename() == null) { - //Servlet Spec 3.0 pg 23, parts without filenames must be put into init params + // Servlet Spec 3.0 pg 23, parts without filename must be put into params. String charset = null; if (mp.getContentType() != null) charset = MimeTypes.getCharsetFromContentType(mp.getContentType()); - //get the bytes regardless of being in memory or in temp file try (InputStream is = mp.getInputStream()) { if (os == null) os = new ByteArrayOutputStream(); IO.copy(is, os); String content=new String(os.toByteArray(),charset==null?StandardCharsets.UTF_8:Charset.forName(charset)); - getParameter(""); //cause params to be evaluated - getParameters().add(mp.getName(), content); + if (_contentParameters == null) + _contentParameters = params == null ? new MultiMap() : params; + _contentParameters.add(mp.getName(), content); } os.reset(); } @@ -2175,72 +2193,57 @@ public class Request implements HttpServletRequest _authentication=Authentication.UNAUTHENTICATED; } - /* ------------------------------------------------------------ */ - /** - * Merge in a new query string. The query string is merged with the existing parameters and {@link #setParameters(MultiMap)} and - * {@link #setQueryString(String)} are called with the result. The merge is according to the rules of the servlet dispatch forward method. - * - * @param query - * The query string to merge into the request. - */ - public void mergeQueryString(String query) + public void mergeQueryParameters(String newQuery, boolean updateQueryString) { - // extract parameters from dispatch query - MultiMap parameters = new MultiMap<>(); - UrlEncoded.decodeTo(query,parameters, UrlEncoded.ENCODING,-1); //have to assume ENCODING because we can't know otherwise + MultiMap newQueryParams = new MultiMap<>(); + // Have to assume ENCODING because we can't know otherwise. + UrlEncoded.decodeTo(newQuery, newQueryParams, UrlEncoded.ENCODING, -1); - boolean merge_old_query = false; - - // Have we evaluated parameters - if (!_paramsExtracted) - extractParameters(); - - // Are there any existing parameters? - if (_parameters != null && _parameters.size() > 0) + MultiMap oldQueryParams = _queryParameters; + if (oldQueryParams == null && _queryString != null) { - // Merge parameters; new parameters of the same name take precedence. - merge_old_query = parameters.addAllValues(_parameters); + oldQueryParams = new MultiMap<>(); + UrlEncoded.decodeTo(_queryString, oldQueryParams, getQueryEncoding(), -1); } - if (_queryString != null && _queryString.length() > 0) + MultiMap mergedQueryParams = new MultiMap<>(newQueryParams); + boolean hasParamsInCommon = false; + if (oldQueryParams != null) { - if (merge_old_query) + // Parameters in the newQuery replace parameters of the oldQuery. + MultiMap copy = new MultiMap<>(oldQueryParams); + hasParamsInCommon = copy.keySet().removeAll(newQueryParams.keySet()); + mergedQueryParams.addAllValues(copy); + } + + setQueryParameters(mergedQueryParams); + resetParameters(); + + if (updateQueryString) + { + // Build the new merged query string. + StringBuilder mergedQuery = new StringBuilder(newQuery); + if (hasParamsInCommon) { - StringBuilder overridden_query_string = new StringBuilder(); - MultiMap overridden_old_query = new MultiMap<>(); - UrlEncoded.decodeTo(_queryString,overridden_old_query,getQueryEncoding(),-1);//decode using any queryencoding set for the request - - - MultiMap overridden_new_query = new MultiMap<>(); - UrlEncoded.decodeTo(query,overridden_new_query,UrlEncoded.ENCODING,-1); //have to assume ENCODING as we cannot know otherwise - - for(String name: overridden_old_query.keySet()) + for (Map.Entry> entry : mergedQueryParams.entrySet()) { - if (!overridden_new_query.containsKey(name)) - { - List values = overridden_old_query.get(name); - for(String v: values) - { - overridden_query_string.append("&").append(name).append("=").append(v); - } - } + if (newQueryParams.containsKey(entry.getKey())) + continue; + for (String value : entry.getValue()) + mergedQuery.append("&").append(entry.getKey()).append("=").append(value); } - - query = query + overridden_query_string; } else { - query = query + "&" + _queryString; + if (_queryString != null) + mergedQuery.append("&").append(_queryString); } - } - setParameters(parameters); - setQueryString(query); + setQueryString(mergedQuery.toString()); + } } - - - /** + /** * @see javax.servlet.http.HttpServletRequest#upgrade(java.lang.Class) */ @Override diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 09747622e05..b1437d6d98d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -32,7 +32,6 @@ import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; - import javax.servlet.ServletContext; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -502,7 +501,7 @@ public class Server extends HandlerWrapper implements Attributes baseRequest.setRequestURI(null); baseRequest.setPathInfo(baseRequest.getRequestURI()); if (uri.getQuery()!=null) - baseRequest.mergeQueryString(uri.getQuery()); //we have to assume dispatch path and query are UTF8 + baseRequest.mergeQueryParameters(uri.getQuery(), true); //we have to assume dispatch path and query are UTF8 } final String target=baseRequest.getPathInfo(); 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 new file mode 100644 index 00000000000..01330f50dce --- /dev/null +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherForwardTest.java @@ -0,0 +1,518 @@ +// +// ======================================================================== +// Copyright (c) 1995-2014 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.servlet; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Server; +import org.hamcrest.Matcher; +import org.hamcrest.Matchers; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; + +public class DispatcherForwardTest +{ + private Server server; + private LocalConnector connector; + private HttpServlet servlet1; + private HttpServlet servlet2; + private List failures = new ArrayList<>(); + + public void prepare() throws Exception + { + server = new Server(); + connector = new LocalConnector(server); + server.addConnector(connector); + + ServletContextHandler context = new ServletContextHandler(server, "/"); + context.addServlet(new ServletHolder(servlet1), "/one"); + context.addServlet(new ServletHolder(servlet2), "/two"); + + server.start(); + } + + @After + public void dispose() throws Exception + { + for (Exception failure : failures) + throw failure; + server.stop(); + } + + // Replacement for Assert that allows to check failures after the response has been sent. + private void checkThat(S item, Matcher matcher) + { + if (!matcher.matches(item)) + failures.add(new Exception()); + } + + @Test + public void testQueryRetainedByForwardWithoutQuery() throws Exception + { + // 1. request /one?a=1 + // 1. forward /two + // 2. assert query => a=1 + // 1. assert query => a=1 + + final String query1 = "a=1"; + servlet1 = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + + req.getRequestDispatcher("/two").forward(req, resp); + + checkThat(query1, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + } + }; + servlet2 = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + } + }; + + prepare(); + + String request = "" + + "GET /one?" + query1 + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Connection: close\r\n" + + "\r\n"; + String response = connector.getResponses(request); + Assert.assertTrue(response, response.startsWith("HTTP/1.1 200")); + } + + @Test + public void testQueryReplacedByForwardWithQuery() throws Exception + { + // 1. request /one?a=1 + // 1. forward /two?a=2 + // 2. assert query => a=2 + // 1. assert query => a=1 + + final String query1 = "a=1&b=2"; + final String query2 = "a=3"; + final String query3 = "a=3&b=2"; + servlet1 = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + + req.getRequestDispatcher("/two?" + query2).forward(req, resp); + + checkThat(query1, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + checkThat("2", Matchers.equalTo(req.getParameter("b"))); + } + }; + servlet2 = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query3, Matchers.equalTo(req.getQueryString())); + checkThat("3", Matchers.equalTo(req.getParameter("a"))); + checkThat("2", Matchers.equalTo(req.getParameter("b"))); + } + }; + + prepare(); + + String request = "" + + "GET /one?" + query1 + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Connection: close\r\n" + + "\r\n"; + String response = connector.getResponses(request); + Assert.assertTrue(response, response.startsWith("HTTP/1.1 200")); + } + + @Test + public void testQueryMergedByForwardWithQuery() throws Exception + { + // 1. request /one?a=1 + // 1. forward /two?b=2 + // 2. assert query => a=1&b=2 + // 1. assert query => a=1 + + final String query1 = "a=1"; + final String query2 = "b=2"; + final String query3 = "b=2&a=1"; + servlet1 = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + + req.getRequestDispatcher("/two?" + query2).forward(req, resp); + + checkThat(query1, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + } + }; + servlet2 = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query3, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + checkThat("2", Matchers.equalTo(req.getParameter("b"))); + } + }; + + prepare(); + + String request = "" + + "GET /one?" + query1 + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Connection: close\r\n" + + "\r\n"; + String response = connector.getResponses(request); + Assert.assertTrue(response, response.startsWith("HTTP/1.1 200")); + } + + @Test + public void testQueryAggregatesWithFormByForwardWithoutQuery() throws Exception + { + // 1. request /one?a=1 + content a=2 + // 1. forward /two + // 2. assert query => a=1 + params => a=1,2 + // 1. assert query => a=1 + params => a=1,2 + + final String query1 = "a=1"; + final String form = "a=2"; + servlet1 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + + req.getRequestDispatcher("/two").forward(req, resp); + + checkThat(query1, Matchers.equalTo(req.getQueryString())); + String[] values = req.getParameterValues("a"); + checkThat(values, Matchers.notNullValue()); + checkThat(2, Matchers.equalTo(values.length)); + checkThat(values, Matchers.arrayContainingInAnyOrder("1", "2")); + } + }; + servlet2 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + String[] values = req.getParameterValues("a"); + checkThat(values, Matchers.notNullValue()); + checkThat(2, Matchers.equalTo(values.length)); + checkThat(values, Matchers.arrayContainingInAnyOrder("1", "2")); + } + }; + + prepare(); + + String request = "" + + "POST /one?" + query1 + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Content-Length: " + form.length() + "\r\n" + + "Connection: close\r\n" + + "\r\n" + + form; + String response = connector.getResponses(request); + Assert.assertTrue(response, response.startsWith("HTTP/1.1 200")); + } + + @Test + public void testQueryAggregatesWithFormReplacedByForwardWithQuery() throws Exception + { + // 1. request /one?a=1 + content a=2 + // 1. forward /two?a=3 + // 2. assert query => a=3 + params => a=3,2 + // 1. assert query => a=1 + params => a=1,2 + + final String query1 = "a=1"; + final String query2 = "a=3"; + final String form = "a=2"; + servlet1 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + + req.getRequestDispatcher("/two?" + query2).forward(req, resp); + + checkThat(query1, Matchers.equalTo(req.getQueryString())); + String[] values = req.getParameterValues("a"); + checkThat(values, Matchers.notNullValue()); + checkThat(2, Matchers.equalTo(values.length)); + checkThat(values, Matchers.arrayContainingInAnyOrder("1", "2")); + } + }; + servlet2 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query2, Matchers.equalTo(req.getQueryString())); + String[] values = req.getParameterValues("a"); + checkThat(values, Matchers.notNullValue()); + checkThat(2, Matchers.equalTo(values.length)); + checkThat(values, Matchers.arrayContainingInAnyOrder("3", "2")); + } + }; + + prepare(); + + String request = "" + + "POST /one?" + query1 + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Content-Length: " + form.length() + "\r\n" + + "Connection: close\r\n" + + "\r\n" + + form; + String response = connector.getResponses(request); + Assert.assertTrue(response, response.startsWith("HTTP/1.1 200")); + } + + @Test + public void testQueryAggregatesWithFormMergedByForwardWithQuery() throws Exception + { + // 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 + // 1. assert query => a=1 + params => a=1&b=2 + + final String query1 = "a=1"; + final String query2 = "c=3"; + final String query3 = "c=3&a=1"; + final String form = "b=2"; + servlet1 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + + req.getRequestDispatcher("/two?" + query2).forward(req, resp); + + checkThat(query1, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + checkThat("2", Matchers.equalTo(req.getParameter("b"))); + checkThat(req.getParameter("c"), Matchers.nullValue()); + } + }; + servlet2 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query3, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + checkThat("2", Matchers.equalTo(req.getParameter("b"))); + checkThat("3", Matchers.equalTo(req.getParameter("c"))); + } + }; + + prepare(); + + String request = "" + + "POST /one?" + query1 + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Content-Length: " + form.length() + "\r\n" + + "Connection: close\r\n" + + "\r\n" + + form; + String response = connector.getResponses(request); + Assert.assertTrue(response, response.startsWith("HTTP/1.1 200")); + } + + @Test + public void testQueryAggregatesWithFormBeforeAndAfterForward() throws Exception + { + // 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 + // 1. assert query => a=1 + params => a=1&b=2 + + final String query1 = "a=1"; + final String query2 = "c=3"; + final String query3 = "c=3&a=1"; + final String form = "b=2"; + servlet1 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + checkThat("2", Matchers.equalTo(req.getParameter("b"))); + + req.getRequestDispatcher("/two?" + query2).forward(req, resp); + + checkThat(query1, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + checkThat("2", Matchers.equalTo(req.getParameter("b"))); + checkThat(req.getParameter("c"), Matchers.nullValue()); + } + }; + servlet2 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query3, Matchers.equalTo(req.getQueryString())); + checkThat("1", Matchers.equalTo(req.getParameter("a"))); + checkThat("2", Matchers.equalTo(req.getParameter("b"))); + checkThat("3", Matchers.equalTo(req.getParameter("c"))); + } + }; + + prepare(); + + String request = "" + + "POST /one?" + query1 + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Content-Length: " + form.length() + "\r\n" + + "Connection: close\r\n" + + "\r\n" + + form; + String response = connector.getResponses(request); + Assert.assertTrue(response, response.startsWith("HTTP/1.1 200")); + } + + @Test + public void testContentCanBeReadViaInputStreamAfterForwardWithoutQuery() throws Exception + { + final String query1 = "a=1"; + final String form = "c=3"; + servlet1 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + + req.getRequestDispatcher("/two").forward(req, resp); + + checkThat(query1, Matchers.equalTo(req.getQueryString())); + checkThat(req.getParameter("c"), Matchers.nullValue()); + } + }; + servlet2 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + ServletInputStream input = req.getInputStream(); + for (int i = 0; i < form.length(); ++i) + checkThat(form.charAt(i) & 0xFFFF, Matchers.equalTo(input.read())); + } + }; + + prepare(); + + String request = "" + + "POST /one?" + query1 + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Content-Length: " + form.length() + "\r\n" + + "Connection: close\r\n" + + "\r\n" + + form; + String response = connector.getResponses(request); + Assert.assertTrue(response, response.startsWith("HTTP/1.1 200")); + } + + @Test + public void testContentCanBeReadViaInputStreamAfterForwardWithQuery() throws Exception + { + final String query1 = "a=1"; + final String query2 = "b=2"; + final String query3 = "b=2&a=1"; + final String form = "c=3"; + servlet1 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query1, Matchers.equalTo(req.getQueryString())); + + req.getRequestDispatcher("/two?" + query2).forward(req, resp); + + checkThat(query1, Matchers.equalTo(req.getQueryString())); + checkThat(req.getParameter("c"), Matchers.nullValue()); + } + }; + servlet2 = new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + checkThat(query3, Matchers.equalTo(req.getQueryString())); + ServletInputStream input = req.getInputStream(); + for (int i = 0; i < form.length(); ++i) + checkThat(form.charAt(i) & 0xFFFF, Matchers.equalTo(input.read())); + checkThat(-1, Matchers.equalTo(input.read())); + } + }; + + prepare(); + + String request = "" + + "POST /one?" + query1 + " HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Content-Length: " + form.length() + "\r\n" + + "Connection: close\r\n" + + "\r\n" + + form; + String response = connector.getResponses(request); + Assert.assertTrue(response, response.startsWith("HTTP/1.1 200")); + } + + // TODO: add multipart tests +} 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 8ed551d5d47..d5ecb5b3425 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 @@ -18,18 +18,11 @@ package org.eclipse.jetty.servlet; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.startsWith; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; import java.util.List; - import javax.servlet.DispatcherType; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -64,6 +57,12 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + public class DispatcherTest { private Server _server; @@ -349,7 +348,6 @@ public class DispatcherTest dispatcher = getServletContext().getRequestDispatcher("/IncludeServlet/includepath?do=assertforwardinclude"); else if(request.getParameter("do").equals("assertincludeforward")) dispatcher = getServletContext().getRequestDispatcher("/AssertIncludeForwardServlet/assertpath?do=end"); - else if(request.getParameter("do").equals("assertforward")) dispatcher = getServletContext().getRequestDispatcher("/AssertForwardServlet?do=end&do=the"); else if(request.getParameter("do").equals("ctx.echo"))