Fixes #4541 Large Headers

Added a HEADER_OVERFLOW result as per review

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2020-02-06 21:17:58 +01:00
parent 1b59e42294
commit f50c2654b9
7 changed files with 102 additions and 15 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,10 @@ public class HttpSenderOverHTTP extends HttpSender
headerBuffer = httpClient.getByteBufferPool().acquire(httpClient.getRequestBufferSize(), false);
break;
}
case HEADER_OVERFLOW:
{
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)
{
@ -446,7 +448,7 @@ public class HttpGenerator
catch (BufferOverflowException e)
{
LOG.ignore(e);
return Result.NEED_HEADER;
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

@ -757,19 +757,16 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
case NEED_HEADER:
{
int size;
if (_header == null)
{
size = Math.min(_config.getResponseHeaderSize(), _config.getOutputBufferSize());
_header = _bufferPool.acquire(Math.min(_config.getResponseHeaderSize(), _config.getOutputBufferSize()), HEADER_BUFFER_DIRECT);
continue;
}
else
case HEADER_OVERFLOW:
{
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);
_header = _bufferPool.acquire(_config.getResponseHeaderSize(), HEADER_BUFFER_DIRECT);
continue;
}
case NEED_CHUNK: