Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Unwrap ServletException #2787 (#2790) 

* Issue #2787 Unwrap ServletException
* Do not unwrap UnavailableException
* unwrap to specific targets
* fixes from review
* fixes after merge

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2018-08-15 12:51:27 +10:00 committed by GitHub
parent e5f0531db0
commit a315d5464f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 81 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
View File

@ -34,6 +34,8 @@ import java.util.function.Supplier;
import javax.servlet.DispatcherType;
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletException;
import javax.servlet.UnavailableException;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpFields;
@ -552,19 +554,19 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
*/
protected void handleException(Throwable failure)
{
// Unwrap wrapping Jetty exceptions.
if (failure instanceof RuntimeIOException)
failure = failure.getCause();
// Unwrap wrapping Jetty and Servlet exceptions.
Throwable quiet = unwrap(failure, QuietException.class);
Throwable no_stack = unwrap(failure, BadMessageException.class, IOException.class, TimeoutException.class);
if (failure instanceof QuietException || !getServer().isRunning())
if (quiet!=null || !getServer().isRunning())
{
if (LOG.isDebugEnabled())
LOG.debug(_request.getRequestURI(), failure);
}
else if (failure instanceof BadMessageException | failure instanceof IOException | failure instanceof TimeoutException)
else if (no_stack!=null)
{
// No stack trace unless there is debug turned on
LOG.warn("{} {}",_request.getRequestURI(), failure.toString());
LOG.warn("{} {}",_request.getRequestURI(), no_stack.toString());
if (LOG.isDebugEnabled())
LOG.debug(_request.getRequestURI(), failure);
}
@ -587,23 +589,6 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
}
}
private void minimalErrorResponse(Throwable failure)
{
try
{
Integer code=(Integer)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
_response.reset(true);
_response.setStatus(code == null ? 500 : code);
_response.flushBuffer();
}
catch (Throwable x)
{
if (x != failure)
failure.addSuppressed(x);
abort(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
@ -620,6 +605,33 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
}
return null;
}
private void minimalErrorResponse(Throwable failure)
{
try
{
int code = 500;
Integer status=(Integer)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
if (status!=null)
code = status.intValue();
else
{
Throwable cause = unwrap(failure,BadMessageException.class);
if (cause instanceof BadMessageException)
code = ((BadMessageException)cause).getCode();
}
_response.reset(true);
_response.setStatus(code);
_response.flushBuffer();
}
catch (Throwable x)
{
if (x != failure)
failure.addSuppressed(x);
abort(failure);
}
}
public boolean isExpecting100Continue()
{

32
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
View File

@ -18,6 +18,11 @@
package org.eclipse.jetty.server;
import static javax.servlet.RequestDispatcher.ERROR_EXCEPTION;
import static javax.servlet.RequestDispatcher.ERROR_EXCEPTION_TYPE;
import static javax.servlet.RequestDispatcher.ERROR_MESSAGE;
import static javax.servlet.RequestDispatcher.ERROR_STATUS_CODE;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
@ -38,10 +43,6 @@ import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.thread.Locker;
import org.eclipse.jetty.util.thread.Scheduler;
import static javax.servlet.RequestDispatcher.ERROR_EXCEPTION;
import static javax.servlet.RequestDispatcher.ERROR_MESSAGE;
import static javax.servlet.RequestDispatcher.ERROR_STATUS_CODE;
/**
* Implementation of AsyncContext interface that holds the state of request-response cycle.
*/
@ -713,7 +714,7 @@ public class HttpChannelState
cancelTimeout();
}
protected void onError(Throwable failure)
protected void onError(Throwable th)
{
final List<AsyncListener> listeners;
final AsyncContextEvent event;
@ -721,15 +722,16 @@ public class HttpChannelState
int code=HttpStatus.INTERNAL_SERVER_ERROR_500;
String reason=null;
if (failure instanceof BadMessageException)
Throwable cause = _channel.unwrap(th,BadMessageException.class,UnavailableException.class);
if (cause instanceof BadMessageException)
{
BadMessageException bme = (BadMessageException)failure;
BadMessageException bme = (BadMessageException)cause;
code = bme.getCode();
reason = bme.getReason();
}
else if (failure instanceof UnavailableException)
else if (cause instanceof UnavailableException)
{
if (((UnavailableException)failure).isPermanent())
if (((UnavailableException)cause).isPermanent())
code = HttpStatus.NOT_FOUND_404;
else
code = HttpStatus.SERVICE_UNAVAILABLE_503;
@ -738,15 +740,15 @@ public class HttpChannelState
try(Locker.Lock lock= _locker.lock())
{
if (LOG.isDebugEnabled())
LOG.debug("onError {} {}",toStringLocked(),failure);
LOG.debug("onError {} {}",toStringLocked(),th);
// Set error on request.
if(_event!=null)
{
_event.addThrowable(failure);
_event.addThrowable(th);
_event.getSuppliedRequest().setAttribute(ERROR_STATUS_CODE,code);
_event.getSuppliedRequest().setAttribute(ERROR_EXCEPTION,failure);
_event.getSuppliedRequest().setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE,failure==null?null:failure.getClass());
_event.getSuppliedRequest().setAttribute(ERROR_EXCEPTION,th);
_event.getSuppliedRequest().setAttribute(ERROR_EXCEPTION_TYPE,th==null?null:th.getClass());
_event.getSuppliedRequest().setAttribute(ERROR_MESSAGE,reason);
}
else
@ -755,8 +757,8 @@ public class HttpChannelState
if (error!=null)
throw new IllegalStateException("Error already set",error);
baseRequest.setAttribute(ERROR_STATUS_CODE,code);
baseRequest.setAttribute(ERROR_EXCEPTION,failure);
baseRequest.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE,failure==null?null:failure.getClass());
baseRequest.setAttribute(ERROR_EXCEPTION,th);
baseRequest.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE,th==null?null:th.getClass());
baseRequest.setAttribute(ERROR_MESSAGE,reason);
}

29
jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java
View File

@ -27,6 +27,8 @@ import static org.junit.Assert.assertThat;
import java.io.IOException;
import javax.servlet.DispatcherType;
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletException;
import javax.servlet.DispatcherType;
import javax.servlet.RequestDispatcher;
@ -124,10 +126,25 @@ public class ErrorHandlerTest
@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))));
}
}
});
@ -328,4 +345,16 @@ public class ErrorHandlerTest
assertThat("Response Content-Type", contentType, is(notNullValue()));
assertThat("Response Content-Type value", contentType.getValue(), not(containsString("null")));
}
@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));
}
}