462162 - StackOverflowException when response commit fails.
Fixed HttpChannel to avoid to stack overflow in case of unusual exceptions thrown while committing. Now it directly calls the transport to try to send a 500.
This commit is contained in:
parent
93a6811580
commit
4f0c63734c
|
@ -98,10 +98,15 @@ public class AbstractServerTest
|
|||
}
|
||||
|
||||
protected boolean parseResponse(Socket client, Parser parser) throws IOException
|
||||
{
|
||||
return parseResponse(client, parser, 1000);
|
||||
}
|
||||
|
||||
protected boolean parseResponse(Socket client, Parser parser, long timeout) throws IOException
|
||||
{
|
||||
byte[] buffer = new byte[2048];
|
||||
InputStream input = client.getInputStream();
|
||||
client.setSoTimeout(1000);
|
||||
client.setSoTimeout((int)timeout);
|
||||
while (true)
|
||||
{
|
||||
try
|
||||
|
|
|
@ -19,14 +19,17 @@
|
|||
package org.eclipse.jetty.http2.server;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.InterruptedIOException;
|
||||
import java.io.OutputStream;
|
||||
import java.net.Socket;
|
||||
import java.nio.ByteBuffer;
|
||||
import java.nio.channels.SelectionKey;
|
||||
import java.nio.channels.SocketChannel;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.http.HttpServlet;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
|
@ -43,7 +46,12 @@ import org.eclipse.jetty.http2.frames.PrefaceFrame;
|
|||
import org.eclipse.jetty.http2.frames.SettingsFrame;
|
||||
import org.eclipse.jetty.http2.parser.Parser;
|
||||
import org.eclipse.jetty.io.ByteBufferPool;
|
||||
import org.eclipse.jetty.io.ManagedSelector;
|
||||
import org.eclipse.jetty.io.SelectChannelEndPoint;
|
||||
import org.eclipse.jetty.server.HttpConfiguration;
|
||||
import org.eclipse.jetty.server.ServerConnector;
|
||||
import org.eclipse.jetty.util.BufferUtil;
|
||||
import org.eclipse.jetty.util.Callback;
|
||||
import org.junit.Assert;
|
||||
import org.junit.Test;
|
||||
|
||||
|
@ -276,4 +284,67 @@ public class HTTP2ServerTest extends AbstractServerTest
|
|||
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCommitFailure() throws Exception
|
||||
{
|
||||
final long delay = 1000;
|
||||
final AtomicBoolean broken = new AtomicBoolean();
|
||||
startServer(new HttpServlet()
|
||||
{
|
||||
@Override
|
||||
protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
|
||||
{
|
||||
try
|
||||
{
|
||||
// Wait for the SETTINGS frames to be exchanged.
|
||||
Thread.sleep(delay);
|
||||
broken.set(true);
|
||||
}
|
||||
catch (InterruptedException x)
|
||||
{
|
||||
throw new InterruptedIOException();
|
||||
}
|
||||
}
|
||||
});
|
||||
server.stop();
|
||||
|
||||
ServerConnector connector2 = new ServerConnector(server, new HTTP2ServerConnectionFactory(new HttpConfiguration()))
|
||||
{
|
||||
@Override
|
||||
protected SelectChannelEndPoint newEndPoint(SocketChannel channel, ManagedSelector selectSet, SelectionKey key) throws IOException
|
||||
{
|
||||
return new SelectChannelEndPoint(channel, selectSet, key, getScheduler(), getIdleTimeout())
|
||||
{
|
||||
@Override
|
||||
public void write(Callback callback, ByteBuffer... buffers) throws IllegalStateException
|
||||
{
|
||||
if (broken.get())
|
||||
callback.failed(new IOException("explicitly_thrown_by_test"));
|
||||
else
|
||||
super.write(callback, buffers);
|
||||
}
|
||||
};
|
||||
}
|
||||
};
|
||||
server.addConnector(connector2);
|
||||
server.start();
|
||||
|
||||
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
|
||||
generator.control(lease, new PrefaceFrame());
|
||||
MetaData.Request metaData = newRequest("GET", new HttpFields());
|
||||
generator.control(lease, new HeadersFrame(1, metaData, null, true));
|
||||
try (Socket client = new Socket("localhost", connector2.getLocalPort()))
|
||||
{
|
||||
OutputStream output = client.getOutputStream();
|
||||
for (ByteBuffer buffer : lease.getByteBuffers())
|
||||
output.write(BufferUtil.toArray(buffer));
|
||||
|
||||
// The server will close the connection abruptly since it
|
||||
// cannot write and therefore cannot even send the GO_AWAY.
|
||||
Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter(), 4096, 8192);
|
||||
boolean closed = parseResponse(client, parser, 2 * delay);
|
||||
Assert.assertTrue(closed);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -25,7 +25,6 @@ import java.nio.channels.ClosedChannelException;
|
|||
import java.util.List;
|
||||
import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
|
||||
import javax.servlet.DispatcherType;
|
||||
import javax.servlet.RequestDispatcher;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
|
@ -76,11 +75,10 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
private final Response _response;
|
||||
private MetaData.Response _committedMetaData;
|
||||
private RequestLog _requestLog;
|
||||
|
||||
|
||||
/** Bytes written after interception (eg after compression) */
|
||||
private long _written;
|
||||
|
||||
|
||||
public HttpChannel(Connector connector, HttpConfiguration configuration, EndPoint endPoint, HttpTransport transport)
|
||||
{
|
||||
_connector = connector;
|
||||
|
@ -105,8 +103,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
{
|
||||
return new HttpOutput(this);
|
||||
}
|
||||
|
||||
|
||||
|
||||
public HttpChannelState getState()
|
||||
{
|
||||
return _state;
|
||||
|
@ -164,7 +161,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
{
|
||||
_endPoint.setIdleTimeout(timeoutMs);
|
||||
}
|
||||
|
||||
|
||||
public ByteBufferPool getByteBufferPool()
|
||||
{
|
||||
return _connector.getByteBufferPool();
|
||||
|
@ -234,9 +231,9 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
}
|
||||
|
||||
public void asyncReadFillInterested()
|
||||
{
|
||||
{
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public void run()
|
||||
{
|
||||
|
@ -521,7 +518,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
public void onCompleted()
|
||||
{
|
||||
if (_requestLog!=null )
|
||||
_requestLog.log(_request,_committedMetaData==null?-1:_committedMetaData.getStatus(), _written);
|
||||
_requestLog.log(_request, _committedMetaData == null ? -1 : _committedMetaData.getStatus(), _written);
|
||||
_transport.onCompleted();
|
||||
}
|
||||
|
||||
|
@ -689,17 +686,17 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
@Override
|
||||
public void failed(final Throwable x)
|
||||
{
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Commit failed", x);
|
||||
|
||||
if (x instanceof EofException || x instanceof ClosedChannelException)
|
||||
{
|
||||
LOG.debug(x);
|
||||
_callback.failed(x);
|
||||
_response.getHttpOutput().closed();
|
||||
}
|
||||
else
|
||||
{
|
||||
_committed.set(false);
|
||||
LOG.warn("Commit failed",x);
|
||||
sendResponse(HttpGenerator.RESPONSE_500_INFO,null,true,new Callback()
|
||||
_transport.send(HttpGenerator.RESPONSE_500_INFO, false, null, true, new Callback()
|
||||
{
|
||||
@Override
|
||||
public void succeeded()
|
||||
|
@ -711,7 +708,6 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
|
|||
@Override
|
||||
public void failed(Throwable th)
|
||||
{
|
||||
LOG.ignore(th);
|
||||
_callback.failed(x);
|
||||
_response.getHttpOutput().closed();
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue