477087 - Enforce that the preface contains a SETTINGS frame.

Now the ServerParser enforces that the preface bytes are followed by
a SETTINGS frame.
This commit is contained in:
Simone Bordet 2015-09-10 19:04:36 +02:00
parent 7957ed06ef
commit f2a3bd8f05
5 changed files with 163 additions and 122 deletions

View File

@ -101,61 +101,14 @@ public class Parser
{
case HEADER:
{
if (!headerParser.parse(buffer))
if (!parseHeader(buffer))
return;
if (continuation)
{
if (headerParser.getFrameType() != FrameType.CONTINUATION.getType())
{
// SPEC: CONTINUATION frames must be consecutive.
BufferUtil.clear(buffer);
notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "continuation_frame_expected");
return;
}
if (headerParser.hasFlag(Flags.END_HEADERS))
{
continuation = false;
}
}
else
{
if (headerParser.getFrameType() == FrameType.HEADERS.getType() &&
!headerParser.hasFlag(Flags.END_HEADERS))
{
continuation = true;
}
}
state = State.BODY;
break;
}
case BODY:
{
int type = headerParser.getFrameType();
if (LOG.isDebugEnabled())
LOG.debug("Parsing {} frame", FrameType.from(type));
if (type < 0 || type >= bodyParsers.length)
{
BufferUtil.clear(buffer);
notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unknown_frame_type_" + type);
if (!parseBody(buffer))
return;
}
BodyParser bodyParser = bodyParsers[type];
if (headerParser.getLength() == 0)
{
bodyParser.emptyBody(buffer);
reset();
if (!buffer.hasRemaining())
return;
}
else
{
if (!bodyParser.parse(buffer))
return;
if (LOG.isDebugEnabled())
LOG.debug("Parsed {} frame", FrameType.from(type));
reset();
}
break;
}
default:
@ -174,6 +127,76 @@ public class Parser
}
}
protected boolean parseHeader(ByteBuffer buffer)
{
if (!headerParser.parse(buffer))
return false;
int frameType = getFrameType();
if (LOG.isDebugEnabled())
LOG.debug("Parsed {} frame header", FrameType.from(frameType));
if (continuation)
{
if (frameType != FrameType.CONTINUATION.getType())
{
// SPEC: CONTINUATION frames must be consecutive.
BufferUtil.clear(buffer);
notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "continuation_frame_expected");
return false;
}
if (headerParser.hasFlag(Flags.END_HEADERS))
{
continuation = false;
}
}
else
{
if (frameType == FrameType.HEADERS.getType() &&
!headerParser.hasFlag(Flags.END_HEADERS))
{
continuation = true;
}
}
state = State.BODY;
return true;
}
protected boolean parseBody(ByteBuffer buffer)
{
int type = getFrameType();
if (type < 0 || type >= bodyParsers.length)
{
BufferUtil.clear(buffer);
notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unknown_frame_type_" + type);
return false;
}
FrameType frameType = FrameType.from(type);
if (LOG.isDebugEnabled())
LOG.debug("Parsing {} frame", frameType);
BodyParser bodyParser = bodyParsers[type];
if (headerParser.getLength() == 0)
{
bodyParser.emptyBody(buffer);
}
else
{
if (!bodyParser.parse(buffer))
return false;
}
if (LOG.isDebugEnabled())
LOG.debug("Parsed {} frame", frameType);
reset();
return true;
}
protected int getFrameType()
{
return headerParser.getFrameType();
}
protected void notifyConnectionFailure(int error, String reason)
{
try

View File

@ -21,6 +21,7 @@ package org.eclipse.jetty.http2.parser;
import java.nio.ByteBuffer;
import org.eclipse.jetty.http2.ErrorCode;
import org.eclipse.jetty.http2.frames.FrameType;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.log.Log;
@ -76,6 +77,21 @@ public class ServerParser extends Parser
{
if (!prefaceParser.parse(buffer))
return;
state = State.SETTINGS;
break;
}
case SETTINGS:
{
if (!parseHeader(buffer))
return;
if (getFrameType() != FrameType.SETTINGS.getType())
{
BufferUtil.clear(buffer);
notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "invalid_preface");
return;
}
if (!parseBody(buffer))
return;
onPreface();
state = State.FRAMES;
break;
@ -133,6 +149,6 @@ public class ServerParser extends Parser
private enum State
{
PREFACE, FRAMES
PREFACE, SETTINGS, FRAMES
}
}

View File

@ -22,6 +22,7 @@ import java.io.IOException;
import java.io.OutputStream;
import java.net.Socket;
import java.nio.ByteBuffer;
import java.util.HashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
@ -37,6 +38,7 @@ import org.eclipse.jetty.http2.api.server.ServerSessionListener;
import org.eclipse.jetty.http2.frames.GoAwayFrame;
import org.eclipse.jetty.http2.frames.HeadersFrame;
import org.eclipse.jetty.http2.frames.PrefaceFrame;
import org.eclipse.jetty.http2.frames.SettingsFrame;
import org.eclipse.jetty.http2.parser.Parser;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.RuntimeIOException;
@ -75,6 +77,7 @@ public class CloseTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
@ -134,6 +137,7 @@ public class CloseTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
generator.control(lease, new GoAwayFrame(1, ErrorCode.NO_ERROR.code, "OK".getBytes("UTF-8")));
@ -199,6 +203,7 @@ public class CloseTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));

View File

@ -23,6 +23,7 @@ import java.io.OutputStream;
import java.net.Socket;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
@ -184,6 +185,7 @@ public class HTTP2CServerTest extends AbstractServerTest
latchRef.set(new CountDownLatch(2));
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP, new HostPortHttpField("localhost:" + _port), "/two", HttpVersion.HTTP_2, new HttpFields());
generator.control(lease, new HeadersFrame(3, metaData, null, true));
for (ByteBuffer buffer : lease.getByteBuffers())
@ -219,8 +221,8 @@ public class HTTP2CServerTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP, new HostPortHttpField("localhost:" + _port), "/test", HttpVersion.HTTP_2, new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
try (Socket client = new Socket("localhost", _port))

View File

@ -26,6 +26,7 @@ import java.nio.ByteBuffer;
import java.nio.channels.SelectionKey;
import java.nio.channels.SocketChannel;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
@ -114,6 +115,7 @@ public class HTTP2ServerTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
@ -170,6 +172,7 @@ public class HTTP2ServerTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
@ -228,9 +231,10 @@ public class HTTP2ServerTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
generator.control(lease, new PingFrame(new byte[8], false));
// Modify the length of the frame to a wrong one.
lease.getByteBuffers().get(1).putShort(0, (short)7);
lease.getByteBuffers().get(2).putShort(0, (short)7);
final CountDownLatch latch = new CountDownLatch(1);
try (Socket client = new Socket("localhost", connector.getLocalPort()))
@ -264,9 +268,10 @@ public class HTTP2ServerTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
generator.control(lease, new PingFrame(new byte[8], false));
// Modify the streamId of the frame to non zero.
lease.getByteBuffers().get(1).putInt(4, 1);
lease.getByteBuffers().get(2).putInt(4, 1);
final CountDownLatch latch = new CountDownLatch(1);
try (Socket client = new Socket("localhost", connector.getLocalPort()))
@ -340,6 +345,7 @@ public class HTTP2ServerTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
try (Socket client = new Socket("localhost", connector2.getLocalPort()))
@ -375,6 +381,7 @@ public class HTTP2ServerTest extends AbstractServerTest
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
@ -400,81 +407,70 @@ public class HTTP2ServerTest extends AbstractServerTest
@Test
public void testRequestWithContinuationFrames() throws Exception
{
testRequestWithContinuationFrames(new Callable<ByteBufferPool.Lease>()
{
@Override
public ByteBufferPool.Lease call() throws Exception
testRequestWithContinuationFrames(() ->
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
return lease;
}
});
}
@Test
public void testRequestWithContinuationFramesWithEmptyHeadersFrame() throws Exception
{
testRequestWithContinuationFrames(new Callable<ByteBufferPool.Lease>()
{
@Override
public ByteBufferPool.Lease call() throws Exception
testRequestWithContinuationFrames(() ->
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
// Take the HeadersFrame header and set the length to zero.
List<ByteBuffer> buffers = lease.getByteBuffers();
ByteBuffer headersFrameHeader = buffers.get(1);
ByteBuffer headersFrameHeader = buffers.get(2);
headersFrameHeader.put(0, (byte)0);
headersFrameHeader.putShort(1, (short)0);
// Insert a CONTINUATION frame header for the body of the HEADERS frame.
lease.insert(2, buffers.get(3).slice(), false);
lease.insert(3, buffers.get(4).slice(), false);
return lease;
}
});
}
@Test
public void testRequestWithContinuationFramesWithEmptyContinuationFrame() throws Exception
{
testRequestWithContinuationFrames(new Callable<ByteBufferPool.Lease>()
{
@Override
public ByteBufferPool.Lease call() throws Exception
testRequestWithContinuationFrames(() ->
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
// Take the ContinuationFrame header, duplicate it, and set the length to zero.
List<ByteBuffer> buffers = lease.getByteBuffers();
ByteBuffer continuationFrameHeader = buffers.get(3);
ByteBuffer continuationFrameHeader = buffers.get(4);
ByteBuffer duplicate = ByteBuffer.allocate(continuationFrameHeader.remaining());
duplicate.put(continuationFrameHeader).flip();
continuationFrameHeader.flip();
continuationFrameHeader.put(0, (byte)0);
continuationFrameHeader.putShort(1, (short)0);
// Insert a CONTINUATION frame header for the body of the previous CONTINUATION frame.
lease.insert(4, duplicate, false);
lease.insert(5, duplicate, false);
return lease;
}
});
}
@Test
public void testRequestWithContinuationFramesWithEmptyLastContinuationFrame() throws Exception
{
testRequestWithContinuationFrames(new Callable<ByteBufferPool.Lease>()
{
@Override
public ByteBufferPool.Lease call() throws Exception
testRequestWithContinuationFrames(() ->
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
generator.control(lease, new SettingsFrame(new HashMap<>(), false));
MetaData.Request metaData = newRequest("GET", new HttpFields());
generator.control(lease, new HeadersFrame(1, metaData, null, true));
// Take the last CONTINUATION frame and reset the flag.
@ -490,7 +486,6 @@ public class HTTP2ServerTest extends AbstractServerTest
});
lease.append(last, false);
return lease;
}
});
}