Issue #4541 Large response header

Fix #4541 by initially allocated a header buffer of `min(_config.getResponseHeaderSize(), _config.getOutputBufferSize())`
Only allocate a buffer of `getResponseHeaderSize` if an overflow results.

This should have no effect on the majority of responses where `getOutputBufferSize` is greater than `getResponseHeaderSize` other than the cost of a min operation.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2020-02-05 10:29:41 +01:00
parent 13458ab515
commit 1b59e42294
3 changed files with 61 additions and 3 deletions

View File

@ -445,7 +445,8 @@ public class HttpGenerator
}
catch (BufferOverflowException e)
{
throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Response header too large", e);
LOG.ignore(e);
return Result.NEED_HEADER;
}
catch (Exception e)
{

View File

@ -25,6 +25,7 @@ import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.LongAdder;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpGenerator;
@ -47,6 +48,8 @@ import org.eclipse.jetty.util.IteratingCallback;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import static org.eclipse.jetty.http.HttpStatus.INTERNAL_SERVER_ERROR_500;
/**
* <p>A {@link Connection} that handles the HTTP protocol.</p>
*/
@ -754,8 +757,19 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
case NEED_HEADER:
{
_header = _bufferPool.acquire(_config.getResponseHeaderSize(), HEADER_BUFFER_DIRECT);
int size;
if (_header == null)
{
size = Math.min(_config.getResponseHeaderSize(), _config.getOutputBufferSize());
}
else
{
if (_header.capacity() >= _config.getResponseHeaderSize())
throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Response header too large");
size = _config.getResponseHeaderSize();
_bufferPool.release(_header);
}
_header = _bufferPool.acquire(size, HEADER_BUFFER_DIRECT);
continue;
}
case NEED_CHUNK:

View File

@ -30,6 +30,7 @@ import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintWriter;
import java.io.StringReader;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
@ -1258,6 +1259,48 @@ public class HttpConnectionTest
}
}
@Test
public void testAllowedLargeResponse() throws Exception
{
connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setResponseHeaderSize(16 * 1024);
connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setOutputBufferSize(8 * 1024);
byte[] bytes = new byte[12 * 1024];
Arrays.fill(bytes, (byte)'X');
final String longstr = "thisisastringthatshouldreachover12kbytes-" + new String(bytes, StandardCharsets.ISO_8859_1) + "_Z_";
final CountDownLatch checkError = new CountDownLatch(1);
server.stop();
server.setHandler(new AbstractHandler()
{
@SuppressWarnings("unused")
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
response.setHeader(HttpHeader.CONTENT_TYPE.toString(), MimeTypes.Type.TEXT_HTML.toString());
response.setHeader("LongStr", longstr);
PrintWriter writer = response.getWriter();
writer.write("<html><h1>FOO</h1></html>");
writer.flush();
if (writer.checkError())
checkError.countDown();
response.flushBuffer();
}
});
server.start();
String response = null;
response = connector.getResponse("GET / HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"\r\n"
);
checkContains(response, 0, "HTTP/1.1 200");
checkContains(response, 0, "LongStr: thisisastringthatshouldreachover12kbytes");
checkContains(response, 0, "XXX_Z_");
assertThat(checkError.getCount(), is(1L));
}
@Test
public void testAsterisk() throws Exception
{