Issue #1456 - Error dispatch race with async write.

Provisional fix catching IllegalStateExceptions and aborting the
transport; the Servlet Specification should clarify how the race
can be avoided altogether.
This commit is contained in:
Simone Bordet 2017-04-06 10:53:22 +02:00
parent 7e2f08a7ad
commit c26af90978
1 changed files with 25 additions and 37 deletions

View File

@ -18,9 +18,6 @@
package org.eclipse.jetty.server;
import static javax.servlet.RequestDispatcher.ERROR_EXCEPTION;
import static javax.servlet.RequestDispatcher.ERROR_STATUS_CODE;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
@ -30,6 +27,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import javax.servlet.DispatcherType;
import javax.servlet.RequestDispatcher;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpFields;
@ -54,7 +52,6 @@ import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.thread.Scheduler;
/**
* HttpChannel represents a single endpoint for HTTP semantic processing.
* The HttpChannel is both a HttpParser.RequestHandler, where it passively receives events from
@ -345,21 +342,15 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
case ERROR_DISPATCH:
{
if (_response.isCommitted())
{
if (LOG.isDebugEnabled())
LOG.debug("Could not perform Error Dispatch because the response is already committed, aborting");
_transport.abort((Throwable)_request.getAttribute(ERROR_EXCEPTION));
}
else
try
{
_response.reset();
Integer icode = (Integer)_request.getAttribute(ERROR_STATUS_CODE);
Integer icode = (Integer)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
int code = icode != null ? icode : HttpStatus.INTERNAL_SERVER_ERROR_500;
_response.setStatus(code);
_request.setAttribute(ERROR_STATUS_CODE,code);
_request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE,code);
if (icode==null)
_request.setAttribute(ERROR_STATUS_CODE,code);
_request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE,code);
_request.setHandled(false);
_response.getHttpOutput().reopen();
@ -373,6 +364,12 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
_request.setDispatcherType(null);
}
}
catch (Throwable x)
{
if (LOG.isDebugEnabled())
LOG.debug("Could not perform ERROR dispatch, aborting", x);
_transport.abort((Throwable)_request.getAttribute(RequestDispatcher.ERROR_EXCEPTION));
}
break;
}
@ -514,36 +511,27 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
}
try
{
_state.onError(failure);
}
catch (Throwable e)
{
try
{
_state.onError(failure);
failure.addSuppressed(e);
LOG.warn("ERROR dispatch failed", failure);
// Try to send a minimal response.
Integer code=(Integer)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
_response.reset();
_response.setStatus(code == null ? 500 : code);
_response.flushBuffer();
}
catch (Exception e)
catch (Throwable x)
{
LOG.warn(e);
// Error could not be handled, probably due to error thrown from error dispatch
if (_response.isCommitted())
{
LOG.warn("ERROR Dispatch failed: ",failure);
_transport.abort(failure);
}
else
{
// Minimal response
Integer code=(Integer)_request.getAttribute(ERROR_STATUS_CODE);
_response.reset();
_response.setStatus(code == null ? 500 : code);
_response.flushBuffer();
}
failure.addSuppressed(x);
_transport.abort(failure);
}
}
catch(Exception e)
{
failure.addSuppressed(e);
LOG.warn("ERROR Dispatch failed: ",failure);
_transport.abort(failure);
}
}
public boolean isExpecting100Continue()