Issue #1854 consistent exception handling for Request parameters

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

@@ -23,6 +23,7 @@ import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
 import java.util.List;
 import java.util.concurrent.Executor;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Supplier;
@@ -522,7 +523,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
             if (LOG.isDebugEnabled())
                 LOG.debug(_request.getRequestURI(), failure);
         }
-        else if (failure instanceof BadMessageException)
+        else if (failure instanceof BadMessageException | failure instanceof IOException | failure instanceof TimeoutException)
         {
             if (LOG.isDebugEnabled())
                 LOG.debug(_request.getRequestURI(), failure);

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

@@ -75,6 +75,7 @@ import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http.MetaData;
 import org.eclipse.jetty.http.MimeTypes;
+import org.eclipse.jetty.io.RuntimeIOException;
 import org.eclipse.jetty.server.handler.ContextHandler;
 import org.eclipse.jetty.server.handler.ContextHandler.Context;
 import org.eclipse.jetty.server.session.Session;
@@ -126,6 +127,9 @@ import org.eclipse.jetty.util.log.Logger;
  * {@link ContextHandler#getMaxFormContentSize()} or if there is no context then the "org.eclipse.jetty.server.Request.maxFormContentSize" {@link Server}
  * attribute. The number of parameters keys is limited by {@link ContextHandler#getMaxFormKeys()} or if there is no context then the
  * "org.eclipse.jetty.server.Request.maxFormKeys" {@link Server} attribute.
+ * </p>
+ * <p>If IOExceptions or timeouts occur while reading form parameters, these are thrown as unchecked Exceptions: ether {@link RuntimeIOException},
+ * {@link BadMessageException} or {@link RuntimeException} as appropriate.</p>
  */
 public class Request implements HttpServletRequest
 {
@@ -526,10 +530,8 @@ public class Request implements HttpServletRequest
         }
         catch (IOException e)
         {
-            if (LOG.isDebugEnabled())
+            LOG.debug(e);
-                LOG.warn(e);
+            throw new RuntimeIOException(e);
-            else
-                LOG.warn(e.toString());
         }
     }
 
@@ -542,8 +544,8 @@ public class Request implements HttpServletRequest
         }
         catch (IOException | ServletException e)
         {
-            LOG.warn(e);
+            LOG.debug(e);
-            throw new RuntimeException(e);
+            throw new RuntimeIOException(e);
         }
     }
 

jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java

@@ -68,6 +68,7 @@ import org.eclipse.jetty.server.handler.ErrorHandler;
 import org.eclipse.jetty.toolchain.test.AdvancedRunner;
 import org.eclipse.jetty.toolchain.test.FS;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.MultiPartInputStreamParser;
 import org.eclipse.jetty.util.log.Log;
@@ -100,6 +101,7 @@ public class RequestTest
         http.getHttpConfiguration().addCustomizer(new ForwardedRequestCustomizer());
         _connector = new LocalConnector(_server,http);
         _server.addConnector(_connector);
+        _connector.setIdleTimeout(500);
         _handler = new RequestHandler();
         _server.setHandler(_handler);
         
@@ -177,6 +179,39 @@ public class RequestTest
         assertThat("Responses", responses, startsWith("HTTP/1.1 400"));
     }
 
+
+    @Test
+    public void testParamExtraction_Timeout() throws Exception
+    {
+        _handler._checker = new RequestTester()
+        {
+            @Override
+            public boolean check(HttpServletRequest request,HttpServletResponse response)
+            {
+                Map<String, String[]> map = request.getParameterMap();
+                // should have thrown a BadMessageException
+                return false;
+            }
+        };
+
+        //Send a request with query string with illegal hex code to cause
+        //an exception parsing the params
+        String request="POST / HTTP/1.1\r\n"+
+                       "Host: whatever\r\n"+
+                       "Content-Type: "+MimeTypes.Type.FORM_ENCODED.asString()+"\n"+
+                       "Connection: close\n"+
+                       "Content-Length: 100\n"+
+                       "\n"+
+                       "name=value";
+
+        LocalEndPoint endp = _connector.connect();
+        endp.addInput(request);
+
+        String response = BufferUtil.toString(endp.waitForResponse(false, 1, TimeUnit.SECONDS));
+        assertThat("Responses", response, startsWith("HTTP/1.1 500"));
+    }
+
+
     @Test
     public void testEmptyHeaders() throws Exception
     {