440020 - ProxyServlet does not handle correctly failure after committed response to client.
Fixed by introducing a request attribute "org.eclipse.jetty.server .Response.failure" used by HttpChannel to immediately close the connection when it sees it.
This commit is contained in:
parent
816b85ea4d
commit
cae4204150
|
@ -34,6 +34,7 @@ import java.util.concurrent.TimeUnit;
|
|||
import java.util.concurrent.TimeoutException;
|
||||
import javax.servlet.AsyncContext;
|
||||
import javax.servlet.ServletConfig;
|
||||
import javax.servlet.ServletContext;
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.UnavailableException;
|
||||
import javax.servlet.http.HttpServlet;
|
||||
|
@ -559,8 +560,15 @@ public class ProxyServlet extends HttpServlet
|
|||
protected void onResponseFailure(HttpServletRequest request, HttpServletResponse response, Response proxyResponse, Throwable failure)
|
||||
{
|
||||
_log.debug(getRequestId(request) + " proxying failed", failure);
|
||||
if (!response.isCommitted())
|
||||
if (response.isCommitted())
|
||||
{
|
||||
request.setAttribute("org.eclipse.jetty.server.Response.failure", failure);
|
||||
AsyncContext asyncContext = request.getAsyncContext();
|
||||
asyncContext.complete();
|
||||
}
|
||||
else
|
||||
{
|
||||
response.resetBuffer();
|
||||
if (failure instanceof TimeoutException)
|
||||
response.setStatus(HttpServletResponse.SC_GATEWAY_TIMEOUT);
|
||||
else
|
||||
|
|
|
@ -19,8 +19,10 @@
|
|||
package org.eclipse.jetty.proxy;
|
||||
|
||||
import java.io.ByteArrayOutputStream;
|
||||
import java.io.EOFException;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.io.InterruptedIOException;
|
||||
import java.io.OutputStream;
|
||||
import java.io.PrintWriter;
|
||||
import java.net.ConnectException;
|
||||
|
@ -43,6 +45,7 @@ import javax.servlet.AsyncContext;
|
|||
import javax.servlet.AsyncEvent;
|
||||
import javax.servlet.AsyncListener;
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.ServletOutputStream;
|
||||
import javax.servlet.http.Cookie;
|
||||
import javax.servlet.http.HttpServlet;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
|
@ -55,9 +58,13 @@ import org.eclipse.jetty.client.api.ContentResponse;
|
|||
import org.eclipse.jetty.client.api.Request;
|
||||
import org.eclipse.jetty.client.api.Response;
|
||||
import org.eclipse.jetty.client.api.Result;
|
||||
import org.eclipse.jetty.client.http.HttpDestinationOverHTTP;
|
||||
import org.eclipse.jetty.client.util.BufferingResponseListener;
|
||||
import org.eclipse.jetty.client.util.BytesContentProvider;
|
||||
import org.eclipse.jetty.client.util.InputStreamResponseListener;
|
||||
import org.eclipse.jetty.http.HttpMethod;
|
||||
import org.eclipse.jetty.server.HttpConfiguration;
|
||||
import org.eclipse.jetty.server.HttpConnectionFactory;
|
||||
import org.eclipse.jetty.server.Server;
|
||||
import org.eclipse.jetty.server.ServerConnector;
|
||||
import org.eclipse.jetty.servlet.ServletContextHandler;
|
||||
|
@ -112,7 +119,12 @@ public class ProxyServletTest
|
|||
private void prepareProxy(Map<String, String> initParams) throws Exception
|
||||
{
|
||||
proxy = new Server();
|
||||
proxyConnector = new ServerConnector(proxy);
|
||||
|
||||
HttpConfiguration configuration = new HttpConfiguration();
|
||||
String value = initParams.get("outputBufferSize");
|
||||
if (value != null)
|
||||
configuration.setOutputBufferSize(Integer.valueOf(value));
|
||||
proxyConnector = new ServerConnector(proxy, new HttpConnectionFactory(configuration));
|
||||
proxy.addConnector(proxyConnector);
|
||||
|
||||
ServletContextHandler proxyCtx = new ServletContextHandler(proxy, "/", true, false);
|
||||
|
@ -899,5 +911,148 @@ public class ProxyServletTest
|
|||
Assert.assertTrue(response3.getHeaders().containsKey(PROXIED_HEADER));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testProxyRequestFailureInTheMiddleOfProxyingSmallContent() throws Exception
|
||||
{
|
||||
final long proxyTimeout = 1000;
|
||||
Map<String, String> proxyParams = new HashMap<>();
|
||||
proxyParams.put("timeout", String.valueOf(proxyTimeout));
|
||||
prepareProxy(proxyParams);
|
||||
|
||||
final CountDownLatch chunk1Latch = new CountDownLatch(1);
|
||||
final int chunk1 = 'q';
|
||||
final int chunk2 = 'w';
|
||||
prepareServer(new HttpServlet()
|
||||
{
|
||||
@Override
|
||||
protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
|
||||
{
|
||||
ServletOutputStream output = response.getOutputStream();
|
||||
output.write(chunk1);
|
||||
response.flushBuffer();
|
||||
|
||||
// Wait for the client to receive this chunk.
|
||||
await(chunk1Latch, 5000);
|
||||
|
||||
// Send second chunk, must not be received by proxy.
|
||||
output.write(chunk2);
|
||||
}
|
||||
|
||||
private boolean await(CountDownLatch latch, long ms) throws IOException
|
||||
{
|
||||
try
|
||||
{
|
||||
return latch.await(ms, TimeUnit.MILLISECONDS);
|
||||
}
|
||||
catch (InterruptedException x)
|
||||
{
|
||||
throw new InterruptedIOException();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
HttpClient client = prepareClient();
|
||||
InputStreamResponseListener listener = new InputStreamResponseListener();
|
||||
int port = serverConnector.getLocalPort();
|
||||
client.newRequest("localhost", port).send(listener);
|
||||
|
||||
// Make the proxy request fail; given the small content, the
|
||||
// proxy-to-client response is not committed yet so it will be reset.
|
||||
TimeUnit.MILLISECONDS.sleep(2 * proxyTimeout);
|
||||
|
||||
Response response = listener.get(5, TimeUnit.SECONDS);
|
||||
Assert.assertEquals(504, response.getStatus());
|
||||
|
||||
// Make sure there is no content, as the proxy-to-client response has been reset.
|
||||
InputStream input = listener.getInputStream();
|
||||
Assert.assertEquals(-1, input.read());
|
||||
|
||||
chunk1Latch.countDown();
|
||||
|
||||
// Result succeeds because a 504 is a valid HTTP response.
|
||||
Result result = listener.await(5, TimeUnit.SECONDS);
|
||||
Assert.assertTrue(result.isSucceeded());
|
||||
|
||||
// Make sure the proxy does not receive chunk2.
|
||||
Assert.assertEquals(-1, input.read());
|
||||
|
||||
HttpDestinationOverHTTP destination = (HttpDestinationOverHTTP)client.getDestination("http", "localhost", port);
|
||||
Assert.assertEquals(0, destination.getConnectionPool().getIdleConnections().size());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testProxyRequestFailureInTheMiddleOfProxyingBigContent() throws Exception
|
||||
{
|
||||
final long proxyTimeout = 1000;
|
||||
int outputBufferSize = 1024;
|
||||
Map<String, String> proxyParams = new HashMap<>();
|
||||
proxyParams.put("timeout", String.valueOf(proxyTimeout));
|
||||
proxyParams.put("outputBufferSize", String.valueOf(outputBufferSize));
|
||||
prepareProxy(proxyParams);
|
||||
|
||||
final CountDownLatch chunk1Latch = new CountDownLatch(1);
|
||||
final byte[] chunk1 = new byte[outputBufferSize];
|
||||
new Random().nextBytes(chunk1);
|
||||
final int chunk2 = 'w';
|
||||
prepareServer(new HttpServlet()
|
||||
{
|
||||
@Override
|
||||
protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
|
||||
{
|
||||
ServletOutputStream output = response.getOutputStream();
|
||||
output.write(chunk1);
|
||||
response.flushBuffer();
|
||||
|
||||
// Wait for the client to receive this chunk.
|
||||
await(chunk1Latch, 5000);
|
||||
|
||||
// Send second chunk, must not be received by proxy.
|
||||
output.write(chunk2);
|
||||
}
|
||||
|
||||
private boolean await(CountDownLatch latch, long ms) throws IOException
|
||||
{
|
||||
try
|
||||
{
|
||||
return latch.await(ms, TimeUnit.MILLISECONDS);
|
||||
}
|
||||
catch (InterruptedException x)
|
||||
{
|
||||
throw new InterruptedIOException();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
HttpClient client = prepareClient();
|
||||
InputStreamResponseListener listener = new InputStreamResponseListener();
|
||||
int port = serverConnector.getLocalPort();
|
||||
client.newRequest("localhost", port).send(listener);
|
||||
|
||||
Response response = listener.get(5, TimeUnit.SECONDS);
|
||||
Assert.assertEquals(200, response.getStatus());
|
||||
|
||||
InputStream input = listener.getInputStream();
|
||||
for (int i = 0; i < chunk1.length; ++i)
|
||||
Assert.assertEquals(chunk1[i] & 0xFF, input.read());
|
||||
|
||||
TimeUnit.MILLISECONDS.sleep(2 * proxyTimeout);
|
||||
|
||||
chunk1Latch.countDown();
|
||||
|
||||
try
|
||||
{
|
||||
// Make sure the proxy does not receive chunk2.
|
||||
input.read();
|
||||
Assert.fail();
|
||||
}
|
||||
catch (EOFException x)
|
||||
{
|
||||
// Expected
|
||||
}
|
||||
|
||||
HttpDestinationOverHTTP destination = (HttpDestinationOverHTTP)client.getDestination("http", "localhost", port);
|
||||
Assert.assertEquals(0, destination.getConnectionPool().getIdleConnections().size());
|
||||
}
|
||||
|
||||
// TODO: test proxy authentication
|
||||
}
|
||||
|
|
|
@ -397,10 +397,27 @@ public class HttpChannel<T> implements HttpParser.RequestHandler<T>, Runnable, H
|
|||
_state.completed();
|
||||
|
||||
if (!_response.isCommitted() && !_request.isHandled())
|
||||
{
|
||||
_response.sendError(404);
|
||||
}
|
||||
else
|
||||
// Complete generating the response
|
||||
_response.closeOutput();
|
||||
{
|
||||
// There is no way in the Servlet API to directly close a connection,
|
||||
// so we rely on applications to pass this attribute to signal they
|
||||
// want to hard close the connection, without even closing the output.
|
||||
Object failure = _request.getAttribute("org.eclipse.jetty.server.Response.failure");
|
||||
if (failure != null)
|
||||
{
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("Explicit response failure", failure);
|
||||
failed();
|
||||
}
|
||||
else
|
||||
{
|
||||
// Complete generating the response
|
||||
_response.closeOutput();
|
||||
}
|
||||
}
|
||||
}
|
||||
catch(EofException|ClosedChannelException e)
|
||||
{
|
||||
|
|
|
@ -539,9 +539,9 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
|
|||
@Override
|
||||
public void failed()
|
||||
{
|
||||
_generator.setPersistent(false);
|
||||
getEndPoint().shutdownOutput();
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public boolean messageComplete()
|
||||
|
|
Loading…
Reference in New Issue