From 166eeaa1f6ecd2b50e7e4827bcaff2b683b3592a Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 5 Oct 2018 18:36:35 +1000 Subject: [PATCH] Jetty 9.3.x #2954 report cause (#2959) Issue #2954 Report badmessage cause Pass BadMessageException from parser to HttpReceiverOVerHTTP This change has already mostly been made in 9.4, so essentially this is a back port. However the primary signature of HttpParser.Handler for badMessage has not been changed and a default method used to handle the cause. This avoids breaking any usages of the interface. Signed-off-by: Greg Wilkins --- .../client/AbstractHttpClientTransport.java | 1 + .../jetty/client/HttpResponseException.java | 7 ++++++- .../client/http/HttpReceiverOverHTTP.java | 8 +++++++- .../client/http/HttpReceiverOverHTTPTest.java | 3 +++ .../org/eclipse/jetty/http/HttpParser.java | 18 ++++++++++++------ .../org/eclipse/jetty/io/ManagedSelector.java | 5 ++++- 6 files changed, 33 insertions(+), 9 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpClientTransport.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpClientTransport.java index 7c091398948..9d61939458b 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpClientTransport.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractHttpClientTransport.java @@ -135,6 +135,7 @@ public abstract class AbstractHttpClientTransport extends ContainerLifeCycle imp catch (IOException xx) { LOG.ignore(xx); + x.addSuppressed(xx); } finally { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponseException.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponseException.java index e494170c34e..b9e568d5821 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponseException.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponseException.java @@ -26,7 +26,12 @@ public class HttpResponseException extends RuntimeException public HttpResponseException(String message, Response response) { - super(message); + this(message, response, null); + } + + public HttpResponseException(String message, Response response, Throwable cause) + { + super(message, cause); this.response = response; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java index 3f839e7c535..8bc0ba2b3f2 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java @@ -302,13 +302,19 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res @Override public void badMessage(int status, String reason) + { + badMessage(status, reason, null); + } + + @Override + public void badMessage(int status, String reason, Throwable cause) { HttpExchange exchange = getHttpExchange(); if (exchange != null) { HttpResponse response = exchange.getResponse(); response.status(status).reason(reason); - failAndClose(new HttpResponseException("HTTP protocol violation: bad response on " + getHttpConnection(), response)); + failAndClose(new HttpResponseException("HTTP protocol violation: bad response on " + getHttpConnection(), response, cause)); } } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java index 7702d27e8e8..cfef6dc577e 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java @@ -32,6 +32,7 @@ import org.eclipse.jetty.client.HttpResponseException; import org.eclipse.jetty.client.Origin; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.util.FutureResponseListener; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpVersion; @@ -204,6 +205,8 @@ public class HttpReceiverOverHTTPTest catch (ExecutionException e) { Assert.assertTrue(e.getCause() instanceof HttpResponseException); + Assert.assertTrue(e.getCause().getCause() instanceof BadMessageException); + Assert.assertTrue(e.getCause().getCause().getCause() instanceof NumberFormatException); } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index 914a4068256..530b8c16644 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -996,7 +996,7 @@ public class HttpParser catch(NumberFormatException e) { LOG.ignore(e); - throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Invalid Content-Length Value"); + throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Invalid Content-Length Value",e); } } @@ -1453,7 +1453,7 @@ public class HttpParser else LOG.warn("bad HTTP parsed: "+e._code+(e.getReason()!=null?" "+e.getReason():"")+" for "+_handler); setState(State.CLOSE); - _handler.badMessage(e.getCode(), e.getReason()); + _handler.badMessage(e.getCode(), e.getReason(), e); } catch(NumberFormatException|IllegalStateException e) { @@ -1461,19 +1461,19 @@ public class HttpParser LOG.warn("parse exception: {} in {} for {}",e.toString(),_state,_handler); if (DEBUG) LOG.debug(e); - badMessage(); + badMessage(e); } catch(Exception|Error e) { BufferUtil.clear(buffer); LOG.warn("parse exception: "+e.toString()+" for "+_handler,e); - badMessage(); + badMessage(e); } return false; } - protected void badMessage() + protected void badMessage(Throwable cause) { if (_headerComplete) { @@ -1482,7 +1482,7 @@ public class HttpParser else if (_state!=State.CLOSED) { setState(State.CLOSE); - _handler.badMessage(400,_requestHandler!=null?"Bad Request":"Bad Response"); + _handler.badMessage(400,_requestHandler!=null?"Bad Request":"Bad Response", cause); } } @@ -1803,6 +1803,12 @@ public class HttpParser */ public void badMessage(int status, String reason); + /* ------------------------------------------------------------ */ + public default void badMessage(int status, String reason, Throwable cause) + { + badMessage(status, reason); + } + /* ------------------------------------------------------------ */ /** @return the size in bytes of the per parser header cache */ diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java index e301c654a27..b3780d26b33 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java @@ -258,11 +258,14 @@ public class ManagedSelector extends AbstractLifeCycle implements Runnable, Dump } catch (Throwable x) { - closeNoExceptions(_selector); if (isRunning()) LOG.warn(x); else + { + LOG.warn(x.toString()); LOG.debug(x); + } + closeNoExceptions(_selector); } return false; }