Fixes #5685 - AsyncProxyServlet calls onProxyResponseSuccess() when internally it throws "Response header too large" exception. (#12351)

* Introduced "maxResponseHeadersSize" parameter to AbstractProxyServlet.
* Introduced HttpGenerator.maxResponseHeadersSize and added checks to not exceed it.
* Fixed HttpParser to generate a 400 in case HttpParser.maxHeaderBytes are exceeded for a response.
* HTTP2Flusher now resets streams in case of failures.
* Removed cases in HTTP2Session where a GOAWAY frame was generated with lastStreamId=0.
  GOAWAY.lastStreamId=0 means that not a single request was processed by the server, which was obviously incorrect.
* Written tests for both ProxyHandler and ProxyServlet about max response headers size exceeded.
* Simplified server-side response header allocation for HTTP/1.1.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2024-10-30 19:17:24 +02:00 committed by GitHub
parent e2277f087b
commit 85ce1521c1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 550 additions and 132 deletions

View File

@ -49,6 +49,6 @@ public class BadMessageException extends HttpException.RuntimeException
public BadMessageException(int code, String reason, Throwable cause)
{
super(code, reason, cause);
assert code >= 400 && code < 500;
assert HttpStatus.isClientError(code);
}
}

View File

@ -41,9 +41,14 @@ public class HttpGenerator
private static final Logger LOG = LoggerFactory.getLogger(HttpGenerator.class);
public static final boolean __STRICT = Boolean.getBoolean("org.eclipse.jetty.http.HttpGenerator.STRICT");
private static final byte[] __colon_space = new byte[]{':', ' '};
public static final MetaData.Response CONTINUE_100_INFO = new MetaData.Response(100, null, HttpVersion.HTTP_1_1, HttpFields.EMPTY);
private static final Index<Boolean> ASSUMED_CONTENT_METHODS = new Index.Builder<Boolean>()
.caseSensitive(false)
.with(HttpMethod.POST.asString(), Boolean.TRUE)
.with(HttpMethod.PUT.asString(), Boolean.TRUE)
.build();
public static final int CHUNK_SIZE = 12;
// states
public enum State
@ -68,25 +73,14 @@ public class HttpGenerator
DONE // The current phase of generation is complete
}
// other statics
public static final int CHUNK_SIZE = 12;
private State _state = State.START;
private EndOfContent _endOfContent = EndOfContent.UNKNOWN_CONTENT;
private MetaData _info;
private long _contentPrepared = 0;
private boolean _noContentResponse = false;
private Boolean _persistent = null;
private static final Index<Boolean> ASSUMED_CONTENT_METHODS = new Index.Builder<Boolean>()
.caseSensitive(false)
.with(HttpMethod.POST.asString(), Boolean.TRUE)
.with(HttpMethod.PUT.asString(), Boolean.TRUE)
.build();
// data
private boolean _needCRLF = false;
private int _maxHeaderBytes;
public HttpGenerator()
{
@ -103,6 +97,16 @@ public class HttpGenerator
_needCRLF = false;
}
public int getMaxHeaderBytes()
{
return _maxHeaderBytes;
}
public void setMaxHeaderBytes(int maxHeaderBytes)
{
_maxHeaderBytes = maxHeaderBytes;
}
public State getState()
{
return _state;
@ -594,28 +598,28 @@ public class HttpGenerator
HttpField field = fields.getField(f);
HttpHeader h = field.getHeader();
if (h == null)
{
putTo(field, header);
}
else
{
switch (h)
{
case CONTENT_LENGTH:
case CONTENT_LENGTH ->
{
if (contentLength < 0)
contentLength = field.getLongValue();
else if (contentLength != field.getLongValue())
throw new HttpException.RuntimeException(INTERNAL_SERVER_ERROR_500, String.format("Incorrect Content-Length %d!=%d", contentLength, field.getLongValue()));
contentLengthField = true;
break;
case CONTENT_TYPE:
}
case CONTENT_TYPE ->
{
// write the field to the header
contentType = true;
putTo(field, header);
break;
}
case TRANSFER_ENCODING:
case TRANSFER_ENCODING ->
{
if (http11)
{
@ -627,10 +631,8 @@ public class HttpGenerator
transferEncoding = transferEncoding.withValues(field.getValues());
chunkedHint |= field.contains(HttpHeaderValue.CHUNKED.asString());
}
break;
}
case CONNECTION:
case CONNECTION ->
{
// Save to connection field for processing when all other fields are known
if (connection == null)
@ -641,13 +643,11 @@ public class HttpGenerator
connectionClose |= field.contains(HttpHeaderValue.CLOSE.asString());
connectionKeepAlive |= field.contains(HttpHeaderValue.KEEP_ALIVE.asString());
connectionUpgrade |= field.contains(HttpHeaderValue.UPGRADE.asString());
break;
}
default:
putTo(field, header);
default -> putTo(field, header);
}
}
checkMaxHeaderBytes(header);
}
}
@ -887,12 +887,23 @@ public class HttpGenerator
// end the header.
header.put(HttpTokens.CRLF);
checkMaxHeaderBytes(header);
}
private void checkMaxHeaderBytes(ByteBuffer header)
{
int maxHeaderBytes = getMaxHeaderBytes();
if (maxHeaderBytes > 0 && header.position() > maxHeaderBytes)
throw new BufferOverflowException();
}
private static void putContentLength(ByteBuffer header, long contentLength)
{
if (contentLength == 0)
{
header.put(CONTENT_LENGTH_0);
}
else
{
header.put(HttpHeader.CONTENT_LENGTH.getBytesColonSpace());

View File

@ -676,7 +676,10 @@ public class HttpParser
if (_maxHeaderBytes > 0 && ++_headerBytes > _maxHeaderBytes)
{
LOG.warn("padding is too large >{}", _maxHeaderBytes);
if (_requestParser)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400);
else
throw new HttpException.RuntimeException(_responseStatus, "Bad Response");
}
}
}
@ -740,11 +743,16 @@ public class HttpParser
else
{
if (_requestParser)
{
LOG.warn("request is too large >{}", _maxHeaderBytes);
else
LOG.warn("response is too large >{}", _maxHeaderBytes);
throw new BadMessageException(HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431);
}
else
{
LOG.warn("response is too large >{}", _maxHeaderBytes);
throw new HttpException.RuntimeException(_responseStatus, "Response Header Bytes Too Large");
}
}
}
switch (_state)
@ -865,7 +873,10 @@ public class HttpParser
break;
default:
throw new BadMessageException(_requestParser ? "No URI" : "No Status");
if (_requestParser)
throw new BadMessageException("No URI");
else
throw new HttpException.RuntimeException(_responseStatus, "No Status");
}
break;
@ -1261,10 +1272,12 @@ public class HttpParser
if (_maxHeaderBytes > 0 && ++_headerBytes > _maxHeaderBytes)
{
boolean header = _state == State.HEADER;
LOG.warn("{} is too large {}>{}", header ? "Header" : "Trailer", _headerBytes, _maxHeaderBytes);
throw new BadMessageException(header
? HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431
: HttpStatus.PAYLOAD_TOO_LARGE_413);
if (debugEnabled)
LOG.debug("{} is too large {}>{}", header ? "Header" : "Trailer", _headerBytes, _maxHeaderBytes);
if (_requestParser)
throw new BadMessageException(header ? HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431 : HttpStatus.PAYLOAD_TOO_LARGE_413);
// There is no equivalent of 431 for response headers.
throw new HttpException.RuntimeException(_responseStatus, "Response Header Fields Too Large");
}
switch (_fieldState)
@ -1744,7 +1757,10 @@ public class HttpParser
if (debugEnabled)
LOG.debug("{} EOF in {}", this, _state);
setState(State.CLOSED);
_handler.badMessage(new BadMessageException(HttpStatus.BAD_REQUEST_400));
if (_requestParser)
_handler.badMessage(new BadMessageException(HttpStatus.BAD_REQUEST_400, "Early EOF"));
else
_handler.badMessage(new HttpException.RuntimeException(_responseStatus, "Early EOF"));
break;
}
}
@ -1752,9 +1768,18 @@ public class HttpParser
catch (Throwable x)
{
BufferUtil.clear(buffer);
HttpException bad = x instanceof HttpException http
? http
: new BadMessageException(_requestParser ? "Bad Request" : "Bad Response", x);
HttpException bad;
if (x instanceof HttpException http)
{
bad = http;
}
else
{
if (_requestParser)
bad = new BadMessageException("Bad Request", x);
else
bad = new HttpException.RuntimeException(_responseStatus, "Bad Response", x);
}
badMessage(bad);
}
return false;

View File

@ -740,11 +740,6 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements Session
}
private GoAwayFrame newGoAwayFrame(int error, String reason)
{
return newGoAwayFrame(getLastRemoteStreamId(), error, reason);
}
private GoAwayFrame newGoAwayFrame(int lastRemoteStreamId, int error, String reason)
{
byte[] payload = null;
if (reason != null)
@ -753,7 +748,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements Session
reason = reason.substring(0, Math.min(reason.length(), 32));
payload = reason.getBytes(StandardCharsets.UTF_8);
}
return new GoAwayFrame(lastRemoteStreamId, error, payload);
return new GoAwayFrame(getLastRemoteStreamId(), error, payload);
}
@Override
@ -1267,15 +1262,20 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements Session
return false;
}
@Override
public void failed(Throwable x)
public void closeAndFail(Throwable failure)
{
if (stream != null)
{
stream.close();
stream.getSession().removeStream(stream);
}
super.failed(x);
failed(failure);
}
public void resetAndFail(Throwable x)
{
if (stream != null)
stream.reset(new ResetFrame(stream.getId(), ErrorCode.CANCEL_STREAM_ERROR.code), Callback.from(() -> failed(x)));
}
/**
@ -1808,10 +1808,8 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements Session
if (failStreams)
{
// Must compare the lastStreamId only with local streams.
// For example, a client that sent request with streamId=137 may send a GOAWAY(4),
// where streamId=4 is the last stream pushed by the server to the client.
// The server must not compare its local streamId=4 with remote streamId=137.
// The lastStreamId carried by the GOAWAY is that of a local stream,
// so the lastStreamId must be compared only to local streams ids.
Predicate<Stream> failIf = stream -> stream.isLocal() && stream.getId() > frame.getLastStreamId();
failStreams(failIf, "closing", false);
}
@ -1839,7 +1837,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements Session
case REMOTELY_CLOSED ->
{
closed = CloseState.CLOSING;
GoAwayFrame goAwayFrame = newGoAwayFrame(0, ErrorCode.NO_ERROR.code, reason);
GoAwayFrame goAwayFrame = newGoAwayFrame(ErrorCode.NO_ERROR.code, reason);
zeroStreamsAction = () -> terminate(goAwayFrame);
failure = new ClosedChannelException();
failStreams = true;
@ -1869,7 +1867,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements Session
}
else
{
GoAwayFrame goAwayFrame = newGoAwayFrame(0, ErrorCode.NO_ERROR.code, reason);
GoAwayFrame goAwayFrame = newGoAwayFrame(ErrorCode.NO_ERROR.code, reason);
abort(reason, cause, Callback.from(() -> terminate(goAwayFrame)));
}
}
@ -1998,7 +1996,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements Session
closed = CloseState.CLOSING;
zeroStreamsAction = () ->
{
GoAwayFrame goAwayFrame = newGoAwayFrame(0, ErrorCode.NO_ERROR.code, reason);
GoAwayFrame goAwayFrame = newGoAwayFrame(ErrorCode.NO_ERROR.code, reason);
terminate(goAwayFrame);
};
failure = x;

View File

@ -179,10 +179,16 @@ public class HTTP2Stream implements Stream, Attachable, Closeable, Callback, Dum
}
session.dataConsumed(this, flowControlLength);
if (resetFailure != null)
{
close();
session.removeStream(this);
callback.failed(resetFailure);
}
else
{
session.reset(this, frame, callback);
}
}
private boolean startWrite(Callback callback)
{

View File

@ -98,7 +98,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
return true;
}
}
closed(entry, closed);
entry.closeAndFail(closed);
return false;
}
@ -108,9 +108,13 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
try (AutoLock ignored = lock.lock())
{
closed = terminated;
// If it was not possible to HPACK encode, then allow to send a GOAWAY.
if (closed instanceof HpackException.SessionException && entry.frame().getType() == FrameType.GO_AWAY)
// If it was not possible to HPACK encode, then allow to send RST_STREAM and GOAWAY.
if (closed instanceof HpackException.SessionException)
{
FrameType frameType = entry.frame().getType();
if (frameType == FrameType.RST_STREAM || frameType == FrameType.GO_AWAY)
closed = null;
}
if (closed == null)
{
entries.offer(entry);
@ -119,7 +123,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
return true;
}
}
closed(entry, closed);
entry.closeAndFail(closed);
return false;
}
@ -137,7 +141,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
return true;
}
}
list.forEach(entry -> closed(entry, closed));
list.forEach(entry -> entry.closeAndFail(closed));
return false;
}
@ -171,11 +175,18 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
if (terminated instanceof HpackException.SessionException)
{
HTTP2Session.Entry entry = entries.peek();
if (entry != null && entry.frame().getType() == FrameType.GO_AWAY)
if (entry != null)
{
FrameType frameType = entry.frame().getType();
if (frameType == FrameType.RST_STREAM || frameType == FrameType.GO_AWAY)
{
rethrow = false;
if (frameType == FrameType.GO_AWAY)
{
// Allow a SessionException to be processed once to send a GOAWAY.
terminated = new ClosedChannelException().initCause(terminated);
rethrow = false;
}
}
}
}
if (rethrow)
@ -222,7 +233,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
{
if (LOG.isDebugEnabled())
LOG.debug("Dropped {}", entry);
entry.failed(new EofException("dropped"));
entry.closeAndFail(new EofException("dropped"));
pending.remove();
continue;
}
@ -262,7 +273,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
{
if (LOG.isDebugEnabled())
LOG.debug("Failure generating {}", entry, failure);
entry.failed(failure);
entry.resetAndFail(failure);
pending.remove();
}
catch (HpackException.SessionException failure)
@ -365,23 +376,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
protected void onCompleteFailure(Throwable x)
{
accumulator.release();
Throwable closed = fail(x);
// If the failure came from within the
// flusher, we need to close the connection.
if (closed == null)
session.onWriteFailure(x);
}
private void onSessionFailure(Throwable x)
{
accumulator.release();
Throwable closed = fail(x);
if (closed == null)
session.close(ErrorCode.COMPRESSION_ERROR.code, null, NOOP);
}
private Throwable fail(Throwable x)
{
Throwable closed;
Set<HTTP2Session.Entry> allEntries;
try (AutoLock ignored = lock.lock())
@ -397,13 +392,47 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
allEntries = new HashSet<>(entries);
entries.clear();
}
allEntries.addAll(processedEntries);
processedEntries.clear();
allEntries.addAll(pendingEntries);
pendingEntries.clear();
allEntries.forEach(entry -> entry.failed(x));
return closed;
// If the failure comes from within the flusher,
// fail the current streams and close the connection.
if (closed == null)
session.onWriteFailure(x);
allEntries.forEach(entry -> entry.closeAndFail(x));
}
private void onSessionFailure(Throwable x)
{
accumulator.release();
Throwable closed;
Set<HTTP2Session.Entry> allEntries;
try (AutoLock ignored = lock.lock())
{
closed = terminated;
terminated = x;
if (LOG.isDebugEnabled())
LOG.debug(String.format("%s, entries processed/pending/queued=%d/%d/%d",
closed != null ? "Closing" : "Failing",
processedEntries.size(),
pendingEntries.size(),
entries.size()), x);
allEntries = new HashSet<>(entries);
entries.clear();
}
allEntries.addAll(processedEntries);
processedEntries.clear();
allEntries.addAll(pendingEntries);
pendingEntries.clear();
allEntries.forEach(entry -> entry.resetAndFail(x));
if (closed == null)
session.close(ErrorCode.COMPRESSION_ERROR.code, null, NOOP);
}
public void terminate(Throwable cause)
@ -420,11 +449,6 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
iterate();
}
private void closed(HTTP2Session.Entry entry, Throwable failure)
{
entry.failed(failure);
}
@Override
public String dump()
{

View File

@ -303,6 +303,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
ServerSessionListener listener = newSessionListener(connector, endPoint);
Generator generator = new Generator(connector.getByteBufferPool(), isUseOutputDirectByteBuffers(), getMaxHeaderBlockFragment());
generator.getHpackEncoder().setMaxHeaderListSize(getHttpConfiguration().getResponseHeaderSize());
FlowControlStrategy flowControl = getFlowControlStrategyFactory().newFlowControlStrategy();
ServerParser parser = newServerParser(connector, getRateControlFactory().newRateControl(endPoint));

View File

@ -14,6 +14,7 @@
package org.eclipse.jetty.proxy;
import java.util.List;
import java.util.function.Consumer;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.transport.HttpClientConnectionFactory;
@ -39,6 +40,8 @@ public class AbstractProxyTest
return List.of(HttpVersion.HTTP_1_1, HttpVersion.HTTP_2);
}
protected HttpConfiguration serverHttpConfig = new HttpConfiguration();
protected HttpConfiguration proxyHttpConfig = new HttpConfiguration();
protected Server server;
protected ServerConnector serverConnector;
protected Server proxy;
@ -46,6 +49,11 @@ public class AbstractProxyTest
protected HttpClient client;
protected void startClient() throws Exception
{
startClient(client -> {});
}
protected void startClient(Consumer<HttpClient> configurator) throws Exception
{
ClientConnector clientConnector = new ClientConnector();
QueuedThreadPool clientThreads = new QueuedThreadPool();
@ -53,6 +61,7 @@ public class AbstractProxyTest
clientConnector.setExecutor(clientThreads);
HTTP2Client http2Client = new HTTP2Client(clientConnector);
client = new HttpClient(new HttpClientTransportDynamic(clientConnector, HttpClientConnectionFactory.HTTP11, new ClientConnectionFactoryOverHTTP2.HTTP2(http2Client)));
configurator.accept(client);
client.start();
}
@ -61,10 +70,9 @@ public class AbstractProxyTest
QueuedThreadPool proxyPool = new QueuedThreadPool();
proxyPool.setName("proxy");
proxy = new Server(proxyPool);
HttpConfiguration configuration = new HttpConfiguration();
configuration.setSendDateHeader(false);
configuration.setSendServerVersion(false);
proxyConnector = new ServerConnector(proxy, 1, 1, new HttpConnectionFactory(configuration), new HTTP2CServerConnectionFactory(configuration));
proxyHttpConfig.setSendDateHeader(false);
proxyHttpConfig.setSendServerVersion(false);
proxyConnector = new ServerConnector(proxy, 1, 1, new HttpConnectionFactory(proxyHttpConfig), new HTTP2CServerConnectionFactory(proxyHttpConfig));
proxy.addConnector(proxyConnector);
proxy.setHandler(handler);
proxy.start();
@ -75,8 +83,7 @@ public class AbstractProxyTest
QueuedThreadPool serverPool = new QueuedThreadPool();
serverPool.setName("server");
server = new Server(serverPool);
HttpConfiguration httpConfig = new HttpConfiguration();
serverConnector = new ServerConnector(server, 1, 1, new HttpConnectionFactory(httpConfig), new HTTP2CServerConnectionFactory(httpConfig));
serverConnector = new ServerConnector(server, 1, 1, new HttpConnectionFactory(serverHttpConfig), new HTTP2CServerConnectionFactory(serverHttpConfig));
server.addConnector(serverConnector);
server.setHandler(handler);
server.start();

View File

@ -13,13 +13,18 @@
package org.eclipse.jetty.proxy;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.client.CompletableResponseListener;
import org.eclipse.jetty.client.ContentResponse;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.StringRequestContent;
import org.eclipse.jetty.client.transport.HttpClientConnectionFactory;
import org.eclipse.jetty.client.transport.HttpClientTransportDynamic;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http2.client.HTTP2Client;
@ -35,6 +40,10 @@ import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class ReverseProxyTest extends AbstractProxyTest
{
@ -62,12 +71,7 @@ public class ReverseProxyTest extends AbstractProxyTest
@Override
protected HttpClient newHttpClient()
{
ClientConnector proxyClientConnector = new ClientConnector();
QueuedThreadPool proxyClientThreads = new QueuedThreadPool();
proxyClientThreads.setName("proxy-client");
proxyClientConnector.setExecutor(proxyClientThreads);
HTTP2Client proxyHTTP2Client = new HTTP2Client(proxyClientConnector);
return new HttpClient(new HttpClientTransportDynamic(proxyClientConnector, HttpClientConnectionFactory.HTTP11, new ClientConnectionFactoryOverHTTP2.HTTP2(proxyHTTP2Client)));
return newProxyHttpClient();
}
@Override
@ -111,12 +115,7 @@ public class ReverseProxyTest extends AbstractProxyTest
@Override
protected HttpClient newHttpClient()
{
ClientConnector proxyClientConnector = new ClientConnector();
QueuedThreadPool proxyClientThreads = new QueuedThreadPool();
proxyClientThreads.setName("proxy-client");
proxyClientConnector.setExecutor(proxyClientThreads);
HTTP2Client proxyHTTP2Client = new HTTP2Client(proxyClientConnector);
return new HttpClient(new HttpClientTransportDynamic(proxyClientConnector, HttpClientConnectionFactory.HTTP11, new ClientConnectionFactoryOverHTTP2.HTTP2(proxyHTTP2Client)));
return newProxyHttpClient();
}
@Override
@ -138,4 +137,290 @@ public class ReverseProxyTest extends AbstractProxyTest
assertEquals(200, response.getStatus());
assertEquals("", response.getHeaders().get(emptyHeaderName));
}
@ParameterizedTest
@MethodSource("httpVersions")
public void testServerResponseHeadersTooLargeForServerConfiguration(HttpVersion httpVersion) throws Exception
{
// Server is not able to write response and aborts.
// Proxy sees the abort and sends 502 to client.
int maxResponseHeadersSize = 256;
serverHttpConfig.setResponseHeaderSize(maxResponseHeadersSize);
startServer(new Handler.Abstract()
{
@Override
public boolean handle(Request request, Response response, Callback callback)
{
response.getHeaders().put("X-Large", "A".repeat(maxResponseHeadersSize));
// With HTTP/1.1, calling response.write() would fail the Handler callback
// which would trigger ErrorHandler and result in a 500 to the proxy.
// response.write(true, null, callback);
// With HTTP/1.1, succeeding the callback before the actual last write
// results in skipping the ErrorHandler and aborting the response and
// the connection, which the proxy interprets as a 502.
// HTTP/2 always behaves by aborting the connection.
callback.succeeded();
return true;
}
});
CountDownLatch serverToProxyFailureLatch = new CountDownLatch(1);
startProxy(new ProxyHandler.Reverse(clientToProxyRequest ->
HttpURI.build(clientToProxyRequest.getHttpURI()).port(serverConnector.getLocalPort()))
{
@Override
protected HttpClient newHttpClient()
{
return newProxyHttpClient();
}
@Override
protected org.eclipse.jetty.client.Request newProxyToServerRequest(Request clientToProxyRequest, HttpURI newHttpURI)
{
// Use the client to proxy protocol also from the proxy to server.
return super.newProxyToServerRequest(clientToProxyRequest, newHttpURI)
.version(httpVersion);
}
@Override
protected void onServerToProxyResponseFailure(Request clientToProxyRequest, org.eclipse.jetty.client.Request proxyToServerRequest, org.eclipse.jetty.client.Response serverToProxyResponse, Response proxyToClientResponse, Callback proxyToClientCallback, Throwable failure)
{
serverToProxyFailureLatch.countDown();
super.onServerToProxyResponseFailure(clientToProxyRequest, proxyToServerRequest, serverToProxyResponse, proxyToClientResponse, proxyToClientCallback, failure);
}
});
startClient();
ContentResponse response = client.newRequest("localhost", proxyConnector.getLocalPort())
.version(httpVersion)
.timeout(5, TimeUnit.SECONDS)
.send();
assertTrue(serverToProxyFailureLatch.await(5, TimeUnit.SECONDS));
assertEquals(HttpStatus.BAD_GATEWAY_502, response.getStatus());
}
@ParameterizedTest
@MethodSource("httpVersions")
public void testServerResponseHeadersTooLargeForProxyConfiguration(HttpVersion httpVersion) throws Exception
{
// Server is able to write the response.
// Proxy cannot parse the response from server, fails and sends 502 to client.
int maxResponseHeadersSize = 256;
startServer(new Handler.Abstract()
{
@Override
public boolean handle(Request request, Response response, Callback callback)
{
response.getHeaders().put("X-Large", "A".repeat(maxResponseHeadersSize));
callback.succeeded();
return true;
}
});
CountDownLatch serverToProxyFailureLatch = new CountDownLatch(1);
startProxy(new ProxyHandler.Reverse(clientToProxyRequest ->
HttpURI.build(clientToProxyRequest.getHttpURI()).port(serverConnector.getLocalPort()))
{
@Override
protected HttpClient newHttpClient()
{
HttpClient httpClient = newProxyHttpClient();
httpClient.setMaxResponseHeadersSize(maxResponseHeadersSize);
return httpClient;
}
@Override
protected org.eclipse.jetty.client.Request newProxyToServerRequest(Request clientToProxyRequest, HttpURI newHttpURI)
{
// Use the client to proxy protocol also from the proxy to server.
return super.newProxyToServerRequest(clientToProxyRequest, newHttpURI)
.version(httpVersion);
}
@Override
protected void onServerToProxyResponseFailure(Request clientToProxyRequest, org.eclipse.jetty.client.Request proxyToServerRequest, org.eclipse.jetty.client.Response serverToProxyResponse, Response proxyToClientResponse, Callback proxyToClientCallback, Throwable failure)
{
serverToProxyFailureLatch.countDown();
super.onServerToProxyResponseFailure(clientToProxyRequest, proxyToServerRequest, serverToProxyResponse, proxyToClientResponse, proxyToClientCallback, failure);
}
});
startClient();
ContentResponse response = client.newRequest("localhost", proxyConnector.getLocalPort())
.version(httpVersion)
.timeout(5, TimeUnit.SECONDS)
.send();
assertTrue(serverToProxyFailureLatch.await(5, TimeUnit.SECONDS));
assertEquals(HttpStatus.BAD_GATEWAY_502, response.getStatus());
}
@ParameterizedTest
@MethodSource("httpVersions")
public void testProxyResponseHeadersTooLargeForProxyConfiguration(HttpVersion httpVersion) throws Exception
{
// Proxy client receives response from server.
// Proxy server is not able to write the response to client.
int maxResponseHeadersSize = 256;
startServer(new Handler.Abstract()
{
@Override
public boolean handle(Request request, Response response, Callback callback)
{
callback.succeeded();
return true;
}
});
CountDownLatch proxyToClientFailureLatch = new CountDownLatch(1);
proxyHttpConfig.setResponseHeaderSize(maxResponseHeadersSize);
startProxy(new ProxyHandler.Reverse(clientToProxyRequest ->
HttpURI.build(clientToProxyRequest.getHttpURI()).port(serverConnector.getLocalPort()))
{
@Override
protected HttpClient newHttpClient()
{
return newProxyHttpClient();
}
@Override
protected org.eclipse.jetty.client.Request newProxyToServerRequest(Request clientToProxyRequest, HttpURI newHttpURI)
{
// Use the client to proxy protocol also from the proxy to server.
return super.newProxyToServerRequest(clientToProxyRequest, newHttpURI)
.version(httpVersion);
}
@Override
protected org.eclipse.jetty.client.Response.CompleteListener newServerToProxyResponseListener(Request clientToProxyRequest, org.eclipse.jetty.client.Request proxyToServerRequest, Response proxyToClientResponse, Callback proxyToClientCallback)
{
return new ProxyResponseListener(clientToProxyRequest, proxyToServerRequest, proxyToClientResponse, proxyToClientCallback)
{
@Override
public void onHeaders(org.eclipse.jetty.client.Response serverToProxyResponse)
{
proxyToClientResponse.getHeaders().put("X-Large", "A".repeat(maxResponseHeadersSize));
super.onHeaders(serverToProxyResponse);
}
};
}
@Override
protected void onProxyToClientResponseFailure(Request clientToProxyRequest, org.eclipse.jetty.client.Request proxyToServerRequest, org.eclipse.jetty.client.Response serverToProxyResponse, Response proxyToClientResponse, Callback proxyToClientCallback, Throwable failure)
{
proxyToClientFailureLatch.countDown();
super.onProxyToClientResponseFailure(clientToProxyRequest, proxyToServerRequest, serverToProxyResponse, proxyToClientResponse, proxyToClientCallback, failure);
}
});
startClient();
var request = client.newRequest("localhost", proxyConnector.getLocalPort())
.version(httpVersion)
.timeout(5, TimeUnit.SECONDS);
CompletableFuture<ContentResponse> completable = new CompletableResponseListener(request).send();
assertTrue(proxyToClientFailureLatch.await(5, TimeUnit.SECONDS));
completable.handle((response, failure) ->
{
switch (httpVersion)
{
case HTTP_1_1 ->
{
// HTTP/1.1 fails to generate the response, but does not commit,
// so it is able to write an error response to the client.
assertNotNull(response);
assertNull(failure);
assertEquals(HttpStatus.INTERNAL_SERVER_ERROR_500, response.getStatus());
}
case HTTP_2 ->
{
// HTTP/2 fails to generate the response, sends a GOAWAY,
// and the client aborts the response.
assertNull(response);
assertNotNull(failure);
}
}
return null;
}).get(5, TimeUnit.SECONDS);
}
@ParameterizedTest
@MethodSource("httpVersions")
public void testProxyResponseHeadersTooLargeForClientConfiguration(HttpVersion httpVersion) throws Exception
{
int maxResponseHeadersSize = 256;
startServer(new Handler.Abstract()
{
@Override
public boolean handle(Request request, Response response, Callback callback)
{
callback.succeeded();
return true;
}
});
startProxy(new ProxyHandler.Reverse(clientToProxyRequest ->
HttpURI.build(clientToProxyRequest.getHttpURI()).port(serverConnector.getLocalPort()))
{
@Override
protected HttpClient newHttpClient()
{
return newProxyHttpClient();
}
@Override
protected org.eclipse.jetty.client.Request newProxyToServerRequest(Request clientToProxyRequest, HttpURI newHttpURI)
{
// Use the client to proxy protocol also from the proxy to server.
return super.newProxyToServerRequest(clientToProxyRequest, newHttpURI)
.version(httpVersion);
}
@Override
protected org.eclipse.jetty.client.Response.CompleteListener newServerToProxyResponseListener(Request clientToProxyRequest, org.eclipse.jetty.client.Request proxyToServerRequest, Response proxyToClientResponse, Callback proxyToClientCallback)
{
return new ProxyResponseListener(clientToProxyRequest, proxyToServerRequest, proxyToClientResponse, proxyToClientCallback)
{
@Override
public void onHeaders(org.eclipse.jetty.client.Response serverToProxyResponse)
{
proxyToClientResponse.getHeaders().put("X-Large", "A".repeat(maxResponseHeadersSize));
super.onHeaders(serverToProxyResponse);
}
};
}
});
startClient(client -> client.setMaxResponseHeadersSize(maxResponseHeadersSize));
CountDownLatch responseFailureLatch = new CountDownLatch(1);
assertThrows(ExecutionException.class, () -> client.newRequest("localhost", proxyConnector.getLocalPort())
.version(httpVersion)
.onResponseFailure((r, x) -> responseFailureLatch.countDown())
.timeout(5, TimeUnit.SECONDS)
.send());
assertTrue(responseFailureLatch.await(5, TimeUnit.SECONDS));
}
private static HttpClient newProxyHttpClient()
{
ClientConnector proxyClientConnector = new ClientConnector();
QueuedThreadPool proxyClientThreads = new QueuedThreadPool();
proxyClientThreads.setName("proxy-client");
proxyClientConnector.setExecutor(proxyClientThreads);
HTTP2Client proxyHTTP2Client = new HTTP2Client(proxyClientConnector);
return new HttpClient(new HttpClientTransportDynamic(proxyClientConnector, HttpClientConnectionFactory.HTTP11, new ClientConnectionFactoryOverHTTP2.HTTP2(proxyHTTP2Client)));
}
}

View File

@ -172,7 +172,9 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab
protected HttpGenerator newHttpGenerator()
{
return new HttpGenerator();
HttpGenerator generator = new HttpGenerator();
generator.setMaxHeaderBytes(getHttpConfiguration().getResponseHeaderSize());
return generator;
}
protected HttpParser newHttpParser(HttpCompliance compliance)
@ -768,16 +770,12 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab
case NEED_HEADER:
{
_header = _bufferPool.acquire(Math.min(getHttpConfiguration().getResponseHeaderSize(), getHttpConfiguration().getOutputBufferSize()), useDirectByteBuffers);
_header = _bufferPool.acquire(getHttpConfiguration().getResponseHeaderSize(), useDirectByteBuffers);
continue;
}
case HEADER_OVERFLOW:
{
if (_header.capacity() >= getHttpConfiguration().getResponseHeaderSize())
throw new HttpException.RuntimeException(INTERNAL_SERVER_ERROR_500, "Response header too large");
releaseHeader();
_header = _bufferPool.acquire(getHttpConfiguration().getResponseHeaderSize(), useDirectByteBuffers);
continue;
throw new HttpException.RuntimeException(INTERNAL_SERVER_ERROR_500, "Response Header Fields Too Large");
}
case NEED_CHUNK:
{
@ -913,7 +911,7 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab
}
@Override
public void onCompleteFailure(final Throwable x)
public void onCompleteFailure(Throwable x)
{
failedCallback(release(), x);
}

View File

@ -53,7 +53,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
public class LargeHeaderTest
{
private static final Logger LOG = LoggerFactory.getLogger(LargeHeaderTest.class);
private static final String EXPECTED_ERROR_TEXT = "<h2>HTTP ERROR 500 Response header too large</h2>";
private static final String EXPECTED_ERROR_TEXT = "<h2>HTTP ERROR 500 Response Header Fields Too Large</h2>";
private Server server;
@BeforeEach

View File

@ -255,6 +255,11 @@ public abstract class AbstractProxyServlet extends HttpServlet
* <td>The response buffer size, see {@link HttpClient#setResponseBufferSize(int)}</td>
* </tr>
* <tr>
* <td>maxResponseHeadersSize</td>
* <td>HttpClient's default</td>
* <td>The maximum response headers size, see {@link HttpClient#setMaxResponseHeadersSize(int)}</td>
* </tr>
* <tr>
* <td>selectors</td>
* <td>cores / 2</td>
* <td>The number of NIO selectors used by {@link HttpClient}</td>
@ -322,6 +327,10 @@ public abstract class AbstractProxyServlet extends HttpServlet
if (value != null)
client.setResponseBufferSize(Integer.parseInt(value));
value = config.getInitParameter("maxResponseHeadersSize");
if (value != null)
client.setMaxResponseHeadersSize(Integer.parseInt(value));
try
{
client.start();
@ -715,7 +724,7 @@ public abstract class AbstractProxyServlet extends HttpServlet
protected void onProxyResponseFailure(HttpServletRequest clientRequest, HttpServletResponse proxyResponse, Response serverResponse, Throwable failure)
{
if (_log.isDebugEnabled())
_log.debug(getRequestId(clientRequest) + " proxying failed", failure);
_log.debug("{} proxying failed", getRequestId(clientRequest), failure);
int status = proxyResponseStatus(failure);
int serverStatus = serverResponse == null ? status : serverResponse.getStatus();
@ -812,7 +821,7 @@ public abstract class AbstractProxyServlet extends HttpServlet
_prefix = _prefix == null ? contextPath : (contextPath + _prefix);
if (proxyServlet._log.isDebugEnabled())
proxyServlet._log.debug(config.getServletName() + " @ " + _prefix + " to " + _proxyTo);
proxyServlet._log.debug("{} @ {} to {}", config.getServletName(), _prefix, _proxyTo);
}
protected String rewriteTarget(HttpServletRequest request)

View File

@ -127,32 +127,32 @@ public class ProxyServletTest
).map(Arguments::of);
}
private HttpClient client;
private Proxy clientProxy;
private HttpConfiguration httpConfig = new HttpConfiguration();
private Server server;
private ServerConnector serverConnector;
private ServerConnector tlsServerConnector;
private Server proxy;
private ServerConnector proxyConnector;
private ServletContextHandler proxyContext;
private AbstractProxyServlet proxyServlet;
private Server server;
private ServerConnector serverConnector;
private ServerConnector tlsServerConnector;
private HttpClient client;
private Proxy clientProxy;
private void startServer(HttpServlet servlet) throws Exception
{
QueuedThreadPool serverPool = new QueuedThreadPool();
serverPool.setName("server");
server = new Server(serverPool);
serverConnector = new ServerConnector(server);
HttpConnectionFactory h1 = new HttpConnectionFactory(httpConfig);
serverConnector = new ServerConnector(server, 1, 1, h1);
server.addConnector(serverConnector);
SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
String keyStorePath = MavenTestingUtils.getTestResourceFile("server_keystore.p12").getAbsolutePath();
sslContextFactory.setKeyStorePath(keyStorePath);
sslContextFactory.setKeyStorePassword("storepwd");
tlsServerConnector = new ServerConnector(server, new SslConnectionFactory(
sslContextFactory,
HttpVersion.HTTP_1_1.asString()),
new HttpConnectionFactory());
SslConnectionFactory ssl = new SslConnectionFactory(sslContextFactory, HttpVersion.HTTP_1_1.asString());
tlsServerConnector = new ServerConnector(server, 1, 1, ssl, h1);
server.addConnector(tlsServerConnector);
ServletContextHandler appCtx = new ServletContextHandler("/", true, false);
@ -1782,4 +1782,58 @@ public class ProxyServletTest
}
}
}
@ParameterizedTest
@MethodSource("transparentImpls")
public void testServerResponseHeadersTooLargeForServerConfiguration(AbstractProxyServlet proxyServletClass) throws Exception
{
int maxResponseHeadersSize = 256;
httpConfig.setResponseHeaderSize(maxResponseHeadersSize);
startServer(new HttpServlet()
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse response)
{
response.setHeader("X-Large", "A".repeat(maxResponseHeadersSize));
}
});
Map<String, String> initParams = Map.of(
"proxyTo", "http://localhost:" + serverConnector.getLocalPort()
);
startProxy(proxyServletClass, initParams);
startClient();
ContentResponse response = client.newRequest("localhost", proxyConnector.getLocalPort())
.timeout(5, TimeUnit.SECONDS)
.send();
assertEquals(HttpStatus.INTERNAL_SERVER_ERROR_500, response.getStatus());
}
@ParameterizedTest
@MethodSource("transparentImpls")
public void testServerResponseHeadersTooLargeForProxyConfiguration(AbstractProxyServlet proxyServletClass) throws Exception
{
int maxResponseHeadersSize = 256;
startServer(new HttpServlet()
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse response)
{
response.setHeader("X-Large", "A".repeat(maxResponseHeadersSize));
}
});
Map<String, String> initParams = Map.of(
"proxyTo", "http://localhost:" + serverConnector.getLocalPort(),
"maxResponseHeadersSize", String.valueOf(maxResponseHeadersSize)
);
startProxy(proxyServletClass, initParams);
startClient();
ContentResponse response = client.newRequest("localhost", proxyConnector.getLocalPort())
.timeout(5, TimeUnit.SECONDS)
.send();
assertEquals(HttpStatus.BAD_GATEWAY_502, response.getStatus());
}
}