Merge pull request #4544 from eclipse/jetty-9.4.x-4541-OptimalLargeResponseHeader

Issue #4541 Large response header
This commit is contained in:
Simone Bordet 2020-02-20 11:17:39 +01:00 committed by GitHub
commit 2958cb1d62
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 151 additions and 3 deletions

View File

@ -27,6 +27,7 @@ import org.eclipse.jetty.client.HttpRequest;
import org.eclipse.jetty.client.HttpRequestException;
import org.eclipse.jetty.client.HttpSender;
import org.eclipse.jetty.client.api.ContentProvider;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpGenerator;
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.MetaData;
@ -36,6 +37,8 @@ import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.IteratingCallback;
import static org.eclipse.jetty.http.HttpStatus.INTERNAL_SERVER_ERROR_500;
public class HttpSenderOverHTTP extends HttpSender
{
private final HttpGenerator generator = new HttpGenerator();
@ -225,6 +228,12 @@ public class HttpSenderOverHTTP extends HttpSender
headerBuffer = httpClient.getByteBufferPool().acquire(httpClient.getRequestBufferSize(), false);
break;
}
case HEADER_OVERFLOW:
{
httpClient.getByteBufferPool().release(headerBuffer);
headerBuffer = null;
throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Request header too large");
}
case NEED_CHUNK:
{
chunkBuffer = httpClient.getByteBufferPool().acquire(HttpGenerator.CHUNK_SIZE, false);

View File

@ -74,6 +74,7 @@ public class HttpGenerator
NEED_CHUNK, // Need a small chunk buffer of CHUNK_SIZE
NEED_INFO, // Need the request/response metadata info
NEED_HEADER, // Need a buffer to build HTTP headers into
HEADER_OVERFLOW, // The header buffer overflowed
NEED_CHUNK_TRAILER, // Need a large chunk buffer for last chunk and trailers
FLUSH, // The buffers previously generated should be flushed
CONTINUE, // Continue generating the message
@ -262,7 +263,8 @@ public class HttpGenerator
}
catch (BufferOverflowException e)
{
throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Request header too large", e);
LOG.ignore(e);
return Result.HEADER_OVERFLOW;
}
catch (Exception e)
{
@ -445,7 +447,8 @@ public class HttpGenerator
}
catch (BufferOverflowException e)
{
throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Response header too large", e);
LOG.ignore(e);
return Result.HEADER_OVERFLOW;
}
catch (Exception e)
{

View File

@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class HttpGeneratorClientTest
@ -122,6 +123,42 @@ public class HttpGeneratorClientTest
assertThat(out, Matchers.not(Matchers.containsString("Null:")));
}
@Test
public void testHeaderOverflow() throws Exception
{
HttpGenerator gen = new HttpGenerator();
Info info = new Info("GET", "/index.html");
info.getFields().add("Host", "localhost");
info.getFields().add("Field", "SomeWhatLongValue");
info.setHttpVersion(HttpVersion.HTTP_1_0);
HttpGenerator.Result result = gen.generateRequest(info, null, null, null, true);
assertEquals(HttpGenerator.Result.NEED_HEADER, result);
ByteBuffer header = BufferUtil.allocate(16);
result = gen.generateRequest(info, header, null, null, true);
assertEquals(HttpGenerator.Result.HEADER_OVERFLOW, result);
header = BufferUtil.allocate(2048);
result = gen.generateRequest(info, header, null, null, true);
assertEquals(HttpGenerator.Result.FLUSH, result);
assertEquals(HttpGenerator.State.COMPLETING, gen.getState());
assertFalse(gen.isChunking());
String out = BufferUtil.toString(header);
BufferUtil.clear(header);
result = gen.generateResponse(null, false, null, null, null, false);
assertEquals(HttpGenerator.Result.SHUTDOWN_OUT, result);
assertEquals(HttpGenerator.State.END, gen.getState());
assertFalse(gen.isChunking());
assertEquals(0, gen.getContentPrepared());
assertThat(out, Matchers.containsString("GET /index.html HTTP/1.0"));
assertThat(out, Matchers.not(Matchers.containsString("Content-Length")));
assertThat(out, Matchers.containsString("Field: SomeWhatLongValue"));
}
@Test
public void testPOSTRequestNoContent() throws Exception
{

View File

@ -155,6 +155,12 @@ public class HttpGeneratorServerHTTPTest
header = BufferUtil.allocate(2048);
continue;
case HEADER_OVERFLOW:
if (header.capacity() >= 8192)
throw new BadMessageException(500, "Header too large");
header = BufferUtil.allocate(8192);
continue;
case NEED_CHUNK:
chunk = BufferUtil.allocate(HttpGenerator.CHUNK_SIZE);
continue;

View File

@ -110,6 +110,38 @@ public class HttpGeneratorServerTest
assertThat(response, containsString("\r\n0123456789"));
}
@Test
public void testHeaderOverflow() throws Exception
{
HttpGenerator gen = new HttpGenerator();
MetaData.Response info = new MetaData.Response(HttpVersion.HTTP_1_1, 302, null, new HttpFields(), 0);
info.getFields().add("Location", "http://somewhere/else");
HttpGenerator.Result result = gen.generateResponse(info, false, null, null, null, true);
assertEquals(HttpGenerator.Result.NEED_HEADER, result);
ByteBuffer header = BufferUtil.allocate(16);
result = gen.generateResponse(info, false, header, null, null, true);
assertEquals(HttpGenerator.Result.HEADER_OVERFLOW, result);
header = BufferUtil.allocate(8096);
result = gen.generateResponse(info, false, header, null, null, true);
assertEquals(HttpGenerator.Result.FLUSH, result);
assertEquals(HttpGenerator.State.COMPLETING, gen.getState());
String response = BufferUtil.toString(header);
BufferUtil.clear(header);
result = gen.generateResponse(null, false, null, null, null, false);
assertEquals(HttpGenerator.Result.DONE, result);
assertEquals(HttpGenerator.State.END, gen.getState());
assertEquals(0, gen.getContentPrepared());
assertThat(response, containsString("HTTP/1.1 302 Found"));
assertThat(response, containsString("Location: http://somewhere/else"));
}
@Test
public void test204() throws Exception
{

View File

@ -451,6 +451,12 @@ public class HttpTester
header = BufferUtil.allocate(8192);
continue;
case HEADER_OVERFLOW:
if (header.capacity() >= 32 * 1024)
throw new BadMessageException(500, "Header too large");
header = BufferUtil.allocate(32 * 1024);
continue;
case NEED_CHUNK:
chunk = BufferUtil.allocate(HttpGenerator.CHUNK_SIZE);
continue;

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,17 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
case NEED_HEADER:
{
_header = _bufferPool.acquire(_config.getResponseHeaderSize(), HEADER_BUFFER_DIRECT);
_header = _bufferPool.acquire(Math.min(_config.getResponseHeaderSize(), _config.getOutputBufferSize()), HEADER_BUFFER_DIRECT);
continue;
}
case HEADER_OVERFLOW:
{
int capacity = _header.capacity();
_bufferPool.release(_header);
if (capacity >= _config.getResponseHeaderSize())
throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Response header too large");
_header = _bufferPool.acquire(_config.getResponseHeaderSize(), 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
{