* Issus #2787 unwrap ServletException Signed-off-by: Greg Wilkins <gregw@webtide.com> * Do not unwrap unavailable exception Signed-off-by: Greg Wilkins <gregw@webtide.com> * unwrap to specific targets Signed-off-by: Greg Wilkins <gregw@webtide.com> * fixes from review Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
parent
33228edc7d
commit
52de1965b6
|
@ -18,7 +18,6 @@
|
|||
|
||||
package org.eclipse.jetty.embedded;
|
||||
|
||||
import org.eclipse.jetty.server.HttpConnectionFactory;
|
||||
import org.eclipse.jetty.server.Server;
|
||||
|
||||
public class OneHandler
|
||||
|
|
|
@ -520,9 +520,9 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
* spawned thread writes the response content; in such case, we attempt to commit the error directly
|
||||
* bypassing the {@link ErrorHandler} mechanisms and the response OutputStream.</p>
|
||||
*
|
||||
* @param x the Throwable that caused the problem
|
||||
* @param failure the Throwable that caused the problem
|
||||
*/
|
||||
protected void handleException(Throwable x)
|
||||
protected void handleException(Throwable failure)
|
||||
{
|
||||
if (_state.isAsyncStarted())
|
||||
{
|
||||
|
@ -530,15 +530,15 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
Throwable root = _state.getAsyncContextEvent().getThrowable();
|
||||
if (root==null)
|
||||
{
|
||||
_state.error(x);
|
||||
_state.error(failure);
|
||||
}
|
||||
else
|
||||
{
|
||||
// TODO Can this happen? Should this just be ISE???
|
||||
// We've already processed an error before!
|
||||
root.addSuppressed(x);
|
||||
root.addSuppressed(failure);
|
||||
LOG.warn("Error while handling async error: ", root);
|
||||
abort(x);
|
||||
abort(failure);
|
||||
_state.errorComplete();
|
||||
}
|
||||
}
|
||||
|
@ -548,27 +548,28 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
{
|
||||
// Handle error normally
|
||||
_request.setHandled(true);
|
||||
_request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, x);
|
||||
_request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE, x.getClass());
|
||||
_request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, failure);
|
||||
_request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE, failure.getClass());
|
||||
|
||||
if (isCommitted())
|
||||
{
|
||||
abort(x);
|
||||
abort(failure);
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Could not send response error 500, already committed", x);
|
||||
LOG.debug("Could not send response error 500, already committed", failure);
|
||||
}
|
||||
else
|
||||
{
|
||||
_response.setHeader(HttpHeader.CONNECTION.asString(), HttpHeaderValue.CLOSE.asString());
|
||||
|
||||
if (x instanceof BadMessageException)
|
||||
Throwable cause = unwrap(failure,BadMessageException.class,UnavailableException.class);
|
||||
if (cause instanceof BadMessageException)
|
||||
{
|
||||
BadMessageException bme = (BadMessageException)x;
|
||||
BadMessageException bme = (BadMessageException)cause;
|
||||
_response.sendError(bme.getCode(), bme.getReason());
|
||||
}
|
||||
else if (x instanceof UnavailableException)
|
||||
else if (cause instanceof UnavailableException)
|
||||
{
|
||||
if (((UnavailableException)x).isPermanent())
|
||||
if (((UnavailableException)cause).isPermanent())
|
||||
_response.sendError(HttpStatus.NOT_FOUND_404);
|
||||
else
|
||||
_response.sendError(HttpStatus.SERVICE_UNAVAILABLE_503);
|
||||
|
@ -579,13 +580,34 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
}
|
||||
catch (Throwable e)
|
||||
{
|
||||
abort(e);
|
||||
if (failure==null)
|
||||
failure = e;
|
||||
else
|
||||
failure.addSuppressed(e);
|
||||
abort(failure);
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Could not commit response error 500", e);
|
||||
LOG.debug("Could not commit response error 500", failure);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Unwrap failure causes to find target class
|
||||
* @param failure The throwable to have its causes unwrapped
|
||||
* @param targets Exception classes that we should not unwrap
|
||||
* @return A target throwable or null
|
||||
*/
|
||||
protected Throwable unwrap(Throwable failure, Class<?> ... targets)
|
||||
{
|
||||
while (failure!=null)
|
||||
{
|
||||
for (Class<?> x : targets)
|
||||
if (x.isInstance(failure))
|
||||
return failure;
|
||||
failure = failure.getCause();
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
public boolean isExpecting100Continue()
|
||||
{
|
||||
return false;
|
||||
|
|
|
@ -19,15 +19,22 @@
|
|||
package org.eclipse.jetty.server;
|
||||
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.hamcrest.Matchers.is;
|
||||
import static org.hamcrest.Matchers.not;
|
||||
import static org.hamcrest.Matchers.startsWith;
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
import java.io.IOException;
|
||||
|
||||
import javax.servlet.DispatcherType;
|
||||
import javax.servlet.RequestDispatcher;
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
|
||||
import org.eclipse.jetty.http.BadMessageException;
|
||||
import org.eclipse.jetty.http.HttpTester;
|
||||
import org.eclipse.jetty.server.handler.AbstractHandler;
|
||||
import org.eclipse.jetty.server.handler.ErrorHandler;
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
|
@ -72,8 +79,34 @@ public class ErrorHandlerTest
|
|||
default:
|
||||
super.generateAcceptableResponse(baseRequest,request,response,code,message,mimeType);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
server.setHandler(new AbstractHandler()
|
||||
{
|
||||
@Override
|
||||
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
|
||||
{
|
||||
if (baseRequest.getDispatcherType()==DispatcherType.ERROR)
|
||||
{
|
||||
baseRequest.setHandled(true);
|
||||
response.sendError(((Integer)request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE)).intValue());
|
||||
return;
|
||||
}
|
||||
|
||||
if(target.startsWith("/charencoding/"))
|
||||
{
|
||||
baseRequest.setHandled(true);
|
||||
response.setCharacterEncoding("utf-8");
|
||||
response.sendError(404);
|
||||
return;
|
||||
}
|
||||
|
||||
if(target.startsWith("/badmessage/"))
|
||||
{
|
||||
throw new ServletException(new BadMessageException(Integer.valueOf(target.substring(12))));
|
||||
}
|
||||
}
|
||||
|
||||
});
|
||||
server.start();
|
||||
}
|
||||
|
@ -257,4 +290,15 @@ public class ErrorHandlerTest
|
|||
assertThat(response,containsString("Content-Type: text/json"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testBadMessage() throws Exception
|
||||
{
|
||||
String rawResponse = connector.getResponse(
|
||||
"GET /badmessage/444 HTTP/1.1\r\n"+
|
||||
"Host: Localhost\r\n"+
|
||||
"\r\n");
|
||||
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
|
||||
|
||||
assertThat("Response status code", response.getStatus(), is(444));
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue