Skip to content

Commit

Permalink
Issue #2787 BadMessage if bad query detected in dispatcher
Browse files Browse the repository at this point in the history 
Signed-off-by: Greg Wilkins <gregw@webtide.com>
  • Loading branch information
@gregw
gregw committed Aug 18, 2018
1 parent 2c0557b commit 04cbb13
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 6 deletions.
11 changes: 8 additions & 3 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View file
Expand Up @@ -2390,8 +2390,6 @@ public void logout() throws ServletException
/* ------------------------------------------------------------ */
public void mergeQueryParameters(String oldQuery,String newQuery, boolean updateQueryString)
{
// TODO This is seriously ugly

MultiMap<String> newQueryParams = null;
// Have to assume ENCODING because we can't know otherwise.
if (newQuery!=null)
Expand All @@ -2404,7 +2402,14 @@ public void mergeQueryParameters(String oldQuery,String newQuery, boolean update
if (oldQueryParams == null && oldQuery != null)
{
oldQueryParams = new MultiMap<>();
UrlEncoded.decodeTo(oldQuery, oldQueryParams, getQueryEncoding());
try
{
UrlEncoded.decodeTo(oldQuery, oldQueryParams, getQueryEncoding());
}
catch(Throwable th)
{
throw new BadMessageException(400,"Bad query encoding",th);
}
}

MultiMap<String> mergedQueryParams;
Expand Down
8 changes: 8 additions & 0 deletions jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
View file
Expand Up @@ -50,6 +50,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.http.PathMap;
Expand Down Expand Up @@ -647,8 +648,15 @@ else if (th instanceof EofException)
else
response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
}
else if (th instanceof BadMessageException)
{
BadMessageException bme = (BadMessageException)th;
response.sendError(bme.getCode(),bme.getMessage());
}
else
{
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
}
else
{
Expand Down
64 changes: 61 additions & 3 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java
View file
Expand Up @@ -18,6 +18,7 @@

package org.eclipse.jetty.servlet;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -59,6 +60,8 @@
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.UrlEncoded;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.StacklessLogging;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
Expand Down Expand Up @@ -156,6 +159,42 @@ public void testForwardWithParam() throws Exception
assertEquals(expected, responses);
}

@Test
public void testForwardWithBadParams() throws Exception
{
try(StacklessLogging nostack = new StacklessLogging(ServletHandler.class))
{
Log.getLogger(ServletHandler.class).info("Expect Not valid UTF8 warnings...");
_contextHandler.addServlet(AlwaysForwardServlet.class, "/forward/*");
_contextHandler.addServlet(EchoServlet.class, "/echo/*");

String response;

response = _connector.getResponse("GET /context/forward/?echo=allgood HTTP/1.0\n\n");
assertThat(response,containsString(" 200 OK"));
assertThat(response,containsString("allgood"));

response = _connector.getResponse("GET /context/forward/params?echo=allgood HTTP/1.0\n\n");
assertThat(response,containsString(" 200 OK"));
assertThat(response,containsString("allgood"));
assertThat(response,containsString("forward"));

response = _connector.getResponse("GET /context/forward/badparams?echo=badparams HTTP/1.0\n\n");
assertThat(response,containsString(" 500 "));

response = _connector.getResponse("GET /context/forward/?echo=badclient&bad=%88%A4 HTTP/1.0\n\n");
assertThat(response,containsString(" 400 "));

response = _connector.getResponse("GET /context/forward/params?echo=badclient&bad=%88%A4 HTTP/1.0\n\n");
assertThat(response,containsString(" 400 "));

response = _connector.getResponse("GET /context/forward/badparams?echo=badclientandparam&bad=%88%A4 HTTP/1.0\n\n");
assertThat(response,containsString(" 500 "));
}
}



@Test
public void testInclude() throws Exception
{
Expand Down Expand Up @@ -358,6 +397,20 @@ else if(request.getParameter("do").equals("req.echo"))
dispatcher.forward(request, response);
}
}

public static class AlwaysForwardServlet extends HttpServlet implements Servlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
if ("/params".equals(request.getPathInfo()))
getServletContext().getRequestDispatcher("/echo?echo=forward").forward(request, response);
else if ("/badparams".equals(request.getPathInfo()))
getServletContext().getRequestDispatcher("/echo?echo=forward&fbad=%88%A4").forward(request, response);
else
getServletContext().getRequestDispatcher("/echo").forward(request, response);
}
}


public static class ForwardNonUTF8Servlet extends HttpServlet implements Servlet
Expand Down Expand Up @@ -480,15 +533,20 @@ public static class EchoServlet extends GenericServlet
@Override
public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException
{
String echoText = req.getParameter("echo");
String[] echoText = req.getParameterValues("echo");

if ( echoText == null )
if ( echoText == null || echoText.length==0)
{
throw new ServletException("echo is a required parameter");
}
else if (echoText.length==1)
{
res.getWriter().print(echoText[0]);
}
else
{
res.getWriter().print(echoText);
for (String text:echoText)
res.getWriter().print(text);
}
}
}
Expand Down

0 comments on commit 04cbb13

Please sign in to comment.