From 1d5635c76c7de641441d180d7c74e8392b05e22f Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 8 Nov 2013 14:32:50 +1100 Subject: [PATCH] 420776 complete error pages after startAsync handle complete and dispatch calls before the thrown exception Conflicts: jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContinuation.java jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java --- .../jetty/server/AsyncContextEvent.java | 17 ++-- .../org/eclipse/jetty/server/HttpChannel.java | 9 +- .../jetty/server/HttpChannelState.java | 35 +++++++- .../org/eclipse/jetty/server/Request.java | 3 +- .../eclipse/jetty/servlet/ServletHandler.java | 4 +- .../jetty/servlet/AsyncContextTest.java | 83 ++++++++++++++++++- 6 files changed, 132 insertions(+), 19 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContextEvent.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContextEvent.java index 11d334d3b94..8c59ef03aed 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContextEvent.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContextEvent.java @@ -35,7 +35,7 @@ public class AsyncContextEvent extends AsyncEvent final private AsyncContextState _asyncContext; volatile HttpChannelState _state; private ServletContext _dispatchContext; - private String _pathInContext; + private String _dispatchPath; private Scheduler.Task _timeoutTask; private Throwable _throwable; @@ -98,7 +98,7 @@ public class AsyncContextEvent extends AsyncEvent */ public String getPath() { - return _pathInContext; + return _dispatchPath; } public void setTimeoutTask(Scheduler.Task task) @@ -131,14 +131,15 @@ public class AsyncContextEvent extends AsyncEvent _throwable=throwable; } - public void setDispatchTarget(ServletContext context, String path) + public void setDispatchContext(ServletContext context) { - if (context!=null) - _dispatchContext=context; - if (path!=null) - _pathInContext=path; + _dispatchContext=context; + } + + public void setDispatchPath(String path) + { + _dispatchPath=path; } - public void completed() { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index d44710fadf3..4a0732ef094 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -256,6 +256,7 @@ public class HttpChannel implements HttpParser.RequestHandler, Runnable HttpChannelState.Next next = _state.handling(); while (next==Next.CONTINUE && getServer().isRunning()) { + boolean error=false; try { _request.setHandled(false); @@ -294,7 +295,7 @@ public class HttpChannel implements HttpParser.RequestHandler, Runnable { String error_page=((ErrorHandler.ErrorPageMapper)eh).getErrorPage((HttpServletRequest)_state.getAsyncContextEvent().getSuppliedRequest()); if (error_page!=null) - _state.getAsyncContextEvent().setDispatchTarget(_state.getContextHandler().getServletContext(),error_page); + _state.getAsyncContextEvent().setDispatchPath(error_page); } } else @@ -307,10 +308,14 @@ public class HttpChannel implements HttpParser.RequestHandler, Runnable if ("ContinuationThrowable".equals(e.getClass().getSimpleName())) LOG.ignore(e); else + { + error=true; throw e; + } } catch (Exception e) { + error=true; if (e instanceof EofException) LOG.debug(e); else @@ -321,6 +326,8 @@ public class HttpChannel implements HttpParser.RequestHandler, Runnable } finally { + if (error && _state.isAsyncStarted()) + _state.errorComplete(); next = _state.unhandle(); } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java index 99c17172bb5..29804a5e265 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java @@ -206,7 +206,6 @@ public class HttpChannelState _responseWrapped=false; return Next.CONTINUE; - } } @@ -314,20 +313,24 @@ public class HttpChannelState { case ASYNCSTARTED: _state=State.REDISPATCHING; - _event.setDispatchTarget(context,path); _dispatched=true; - return; + dispatch=false; + break; case ASYNCWAIT: dispatch=!_expired; _state=State.REDISPATCH; - _event.setDispatchTarget(context,path); _dispatched=true; break; default: throw new IllegalStateException(this.getStatusString()); } + + if (context!=null) + _event.setDispatchContext(context); + if (path!=null) + _event.setDispatchPath(path); } if (dispatch) @@ -437,6 +440,30 @@ public class HttpChannelState } } + public void errorComplete() + { + synchronized (this) + { + switch(_state) + { + case ASYNCSTARTED: + case REDISPATCHING: + _state=State.COMPLETECALLED; + break; + + case COMPLETECALLED: + break; + + default: + throw new IllegalStateException(this.getStatusString()); + } + + _dispatched=false; + _event.setDispatchContext(null); + _event.setDispatchPath(null); + } + } + protected void completed() { final List aListeners; 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 5283fbf80c1..a963069b6f6 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 @@ -2023,7 +2023,8 @@ public class Request implements HttpServletRequest if (_async==null) _async=new AsyncContextState(state); AsyncContextEvent event = new AsyncContextEvent(_context,_async,state,this,servletRequest,servletResponse); - event.setDispatchTarget(getServletContext(),URIUtil.addPaths(getServletPath(),getPathInfo())); + event.setDispatchContext(getServletContext()); + event.setDispatchPath(URIUtil.addPaths(getServletPath(),getPathInfo())); state.startAsync(event); return _async; } diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index 4d210c2470d..4904ffc8a91 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -21,7 +21,6 @@ package org.eclipse.jetty.servlet; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; @@ -56,7 +55,6 @@ import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.security.IdentityService; import org.eclipse.jetty.security.SecurityHandler; -import org.eclipse.jetty.server.Dispatcher; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.QuietServletException; import org.eclipse.jetty.server.Request; @@ -615,7 +613,7 @@ public class ServletHandler extends ScopedHandler { // Complete async errored requests if (th!=null && request.isAsyncStarted()) - request.getAsyncContext().complete(); + baseRequest.getHttpChannelState().errorComplete(); if (servlet_holder!=null) baseRequest.setHandled(true); diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java index e707884f9f5..bcc6ae89a48 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java @@ -122,8 +122,11 @@ public class AsyncContextTest @Test public void testStartThrow() throws Exception { - String request = "GET /ctx/startthrow HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Type: application/x-www-form-urlencoded\r\n" - + "Connection: close\r\n" + "\r\n"; + String request = + "GET /ctx/startthrow HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Connection: close\r\n" + + "\r\n"; String responseString = _connector.getResponses(request); BufferedReader br = new BufferedReader(new StringReader(responseString)); @@ -137,6 +140,68 @@ public class AsyncContextTest Assert.assertEquals("error servlet","PathInfo= /IOE",br.readLine()); Assert.assertEquals("error servlet","EXCEPTION: java.io.IOException: Test",br.readLine()); } + + @Test + public void testStartDispatchThrow() throws Exception + { + String request = "GET /ctx/startthrow?dispatch=true HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Connection: close\r\n" + + "\r\n"; + String responseString = _connector.getResponses(request); + + BufferedReader br = new BufferedReader(new StringReader(responseString)); + + assertEquals("HTTP/1.1 500 Server Error",br.readLine()); + br.readLine();// connection close + br.readLine();// server + br.readLine();// empty + Assert.assertEquals("error servlet","ERROR: /error",br.readLine()); + Assert.assertEquals("error servlet","PathInfo= /IOE",br.readLine()); + Assert.assertEquals("error servlet","EXCEPTION: java.io.IOException: Test",br.readLine()); + } + + @Test + public void testStartCompleteThrow() throws Exception + { + String request = "GET /ctx/startthrow?complete=true HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Connection: close\r\n" + + "\r\n"; + String responseString = _connector.getResponses(request); + + BufferedReader br = new BufferedReader(new StringReader(responseString)); + + assertEquals("HTTP/1.1 500 Server Error",br.readLine()); + br.readLine();// connection close + br.readLine();// server + br.readLine();// empty + Assert.assertEquals("error servlet","ERROR: /error",br.readLine()); + Assert.assertEquals("error servlet","PathInfo= /IOE",br.readLine()); + Assert.assertEquals("error servlet","EXCEPTION: java.io.IOException: Test",br.readLine()); + } + + @Test + public void testStartFlushCompleteThrow() throws Exception + { + String request = "GET /ctx/startthrow?flush=true&complete=true HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Connection: close\r\n" + + "\r\n"; + String responseString = _connector.getResponses(request); + + BufferedReader br = new BufferedReader(new StringReader(responseString)); + + assertEquals("HTTP/1.1 200 OK",br.readLine()); + br.readLine();// connection close + br.readLine();// server + br.readLine();// empty + + Assert.assertEquals("error servlet","completeBeforeThrow",br.readLine()); + } @Test public void testDispatchAsyncContext() throws Exception @@ -497,6 +562,20 @@ public class AsyncContextTest if (request.getDispatcherType()==DispatcherType.REQUEST) { request.startAsync(request, response); + + if (Boolean.valueOf(request.getParameter("dispatch"))) + { + request.getAsyncContext().dispatch(); + } + + if (Boolean.valueOf(request.getParameter("complete"))) + { + response.getOutputStream().write("completeBeforeThrow".getBytes()); + if (Boolean.valueOf(request.getParameter("flush"))) + response.flushBuffer(); + request.getAsyncContext().complete(); + } + throw new QuietServletException(new IOException("Test")); } }