Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

478275 - Priority information in HEADERS frame is not sent. 

Now the priority information present in HEADERS frame is correctly
generated and sent as part of the HEADERS frame.
This commit is contained in:
Simone Bordet 2015-09-25 13:00:26 +02:00
parent 394c105d5d
commit 0c9eba0485
7 changed files with 192 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/PriorityTest.java
View File

@ -141,4 +141,44 @@ public class PriorityTest extends AbstractTest
Assert.assertTrue(afterRequests.await(5, TimeUnit.SECONDS));
Assert.assertTrue(responses.await(5, TimeUnit.SECONDS));
}
@Test
public void testHeadersWithPriority() throws Exception
{
PriorityFrame priorityFrame = new PriorityFrame(13, 200, true);
CountDownLatch latch = new CountDownLatch(2);
start(new ServerSessionListener.Adapter()
{
@Override
public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
{
PriorityFrame priority = frame.getPriority();
Assert.assertNotNull(priority);
Assert.assertEquals(priorityFrame.getParentStreamId(), priority.getParentStreamId());
Assert.assertEquals(priorityFrame.getWeight(), priority.getWeight());
Assert.assertEquals(priorityFrame.isExclusive(), priority.isExclusive());
latch.countDown();
MetaData.Response metaData = new MetaData.Response(HttpVersion.HTTP_2, 200, new HttpFields());
HeadersFrame responseFrame = new HeadersFrame(stream.getId(), metaData, null, true);
stream.headers(responseFrame, Callback.NOOP);
return null;
}
});
Session session = newClient(new Session.Listener.Adapter());
MetaData metaData = newRequest("GET", "/one", new HttpFields());
HeadersFrame headersFrame = new HeadersFrame(metaData, priorityFrame, true);
session.newStream(headersFrame, new Promise.Adapter<>(), new Stream.Listener.Adapter()
{
@Override
public void onHeaders(Stream stream, HeadersFrame frame)
{
if (frame.isEndStream())
latch.countDown();
}
});
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
}
}

2
jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/PriorityFrame.java
View File

@ -20,6 +20,8 @@ package org.eclipse.jetty.http2.frames;
public class PriorityFrame extends Frame
{
public static final int PRIORITY_LENGTH = 5;
private final int streamId;
private final int parentStreamId;
private final int weight;

63
jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/HeadersGenerator.java
View File

@ -25,6 +25,7 @@ import org.eclipse.jetty.http2.Flags;
import org.eclipse.jetty.http2.frames.Frame;
import org.eclipse.jetty.http2.frames.FrameType;
import org.eclipse.jetty.http2.frames.HeadersFrame;
import org.eclipse.jetty.http2.frames.PriorityFrame;
import org.eclipse.jetty.http2.hpack.HpackEncoder;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.util.BufferUtil;
@ -50,16 +51,30 @@ public class HeadersGenerator extends FrameGenerator
public void generate(ByteBufferPool.Lease lease, Frame frame)
{
HeadersFrame headersFrame = (HeadersFrame)frame;
generateHeaders(lease, headersFrame.getStreamId(), headersFrame.getMetaData(), !headersFrame.isEndStream());
generateHeaders(lease, headersFrame.getStreamId(), headersFrame.getMetaData(), headersFrame.getPriority(), headersFrame.isEndStream());
}
public void generateHeaders(ByteBufferPool.Lease lease, int streamId, MetaData metaData, boolean contentFollows)
public void generateHeaders(ByteBufferPool.Lease lease, int streamId, MetaData metaData, PriorityFrame priority, boolean endStream)
{
if (streamId < 0)
throw new IllegalArgumentException("Invalid stream id: " + streamId);
int maxFrameSize = getMaxFrameSize();
int flags = Flags.NONE;
if (priority != null)
{
flags = Flags.PRIORITY;
int parentStreamId = priority.getParentStreamId();
if (parentStreamId < 0)
throw new IllegalArgumentException("Invalid parent stream id: " + parentStreamId);
if (parentStreamId == streamId)
throw new IllegalArgumentException("Stream " + streamId + " cannot depend on stream " + parentStreamId);
int weight = priority.getWeight();
if (weight > 255)
throw new IllegalArgumentException("Invalid weight: " + weight);
}
int maxFrameSize = getMaxFrameSize();
ByteBuffer hpacked = lease.acquire(maxFrameSize, false);
BufferUtil.clearToFill(hpacked);
encoder.encode(hpacked, metaData);
@ -69,10 +84,19 @@ public class HeadersGenerator extends FrameGenerator
// Split into CONTINUATION frames if necessary.
if (maxHeaderBlockFragment > 0 && hpackedLength > maxHeaderBlockFragment)
{
int flags = contentFollows ? Flags.NONE : Flags.END_STREAM;
ByteBuffer header = generateHeader(lease, FrameType.HEADERS, maxHeaderBlockFragment, flags, streamId);
if (endStream)
flags |= Flags.END_STREAM;
int length = maxHeaderBlockFragment;
if (priority != null)
length += PriorityFrame.PRIORITY_LENGTH;
ByteBuffer header = generateHeader(lease, FrameType.HEADERS, length, flags, streamId);
BufferUtil.flipToFlush(header, 0);
lease.append(header, true);
generatePriority(lease, priority);
hpacked.limit(maxHeaderBlockFragment);
lease.append(hpacked.slice(), false);
@ -97,13 +121,36 @@ public class HeadersGenerator extends FrameGenerator
}
else
{
int flags = Flags.END_HEADERS;
if (!contentFollows)
flags |= Flags.END_HEADERS;
if (endStream)
flags |= Flags.END_STREAM;
ByteBuffer header = generateHeader(lease, FrameType.HEADERS, hpackedLength, flags, streamId);
int length = hpackedLength;
if (priority != null)
length += PriorityFrame.PRIORITY_LENGTH;
ByteBuffer header = generateHeader(lease, FrameType.HEADERS, length, flags, streamId);
BufferUtil.flipToFlush(header, 0);
lease.append(header, true);
generatePriority(lease, priority);
lease.append(hpacked, true);
}
}
private void generatePriority(ByteBufferPool.Lease lease, PriorityFrame priority)
{
if (priority != null)
{
int parentStreamId = priority.getParentStreamId() & 0x7F_FF_FF_FF;
if (priority.isExclusive())
parentStreamId |= 0x80_00_00_00;
ByteBuffer buffer = lease.acquire(PriorityFrame.PRIORITY_LENGTH, true);
buffer.putInt(parentStreamId);
buffer.put((byte)(priority.getWeight() & 0xFF));
BufferUtil.flipToFlush(buffer, 0);
lease.append(buffer, true);
}
}
}

1
jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/ContinuationBodyParser.java
View File

@ -42,7 +42,6 @@ public class ContinuationBodyParser extends BodyParser
@Override
protected void emptyBody(ByteBuffer buffer)
{
reset();
if (hasFlag(Flags.END_HEADERS))
onHeaders();
}

33
jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java
View File

@ -36,7 +36,7 @@ public class HeadersBodyParser extends BodyParser
private int length;
private int paddingLength;
private boolean exclusive;
private int streamId;
private int parentStreamId;
private int weight;
public HeadersBodyParser(HeaderParser headerParser, Parser.Listener listener, HeaderBlockParser headerBlockParser, HeaderBlockFragments headerBlockFragments)
@ -53,7 +53,7 @@ public class HeadersBodyParser extends BodyParser
length = 0;
paddingLength = 0;
exclusive = false;
streamId = 0;
parentStreamId = 0;
weight = 0;
}
@ -70,9 +70,8 @@ public class HeadersBodyParser extends BodyParser
headerBlockFragments.setStreamId(getStreamId());
headerBlockFragments.setEndStream(isEndStream());
if (hasFlag(Flags.PRIORITY))
headerBlockFragments.setPriorityFrame(new PriorityFrame(streamId, getStreamId(), weight, exclusive));
connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_headers_priority_frame");
}
reset();
}
@Override
@ -122,15 +121,15 @@ public class HeadersBodyParser extends BodyParser
// because the 31 least significant bits represent the stream id.
int currByte = buffer.get(buffer.position());
exclusive = (currByte & 0x80) == 0x80;
state = State.STREAM_ID;
state = State.PARENT_STREAM_ID;
break;
}
case STREAM_ID:
case PARENT_STREAM_ID:
{
if (buffer.remaining() >= 4)
{
streamId = buffer.getInt();
streamId &= 0x7F_FF_FF_FF;
parentStreamId = buffer.getInt();
parentStreamId &= 0x7F_FF_FF_FF;
length -= 4;
state = State.WEIGHT;
if (length < 1)
@ -138,22 +137,22 @@ public class HeadersBodyParser extends BodyParser
}
else
{
state = State.STREAM_ID_BYTES;
state = State.PARENT_STREAM_ID_BYTES;
cursor = 4;
}
break;
}
case STREAM_ID_BYTES:
case PARENT_STREAM_ID_BYTES:
{
int currByte = buffer.get() & 0xFF;
--cursor;
streamId += currByte << (8 * cursor);
parentStreamId += currByte << (8 * cursor);
--length;
if (cursor > 0 && length <= 0)
return connectionFailure(buffer, ErrorCode.FRAME_SIZE_ERROR.code, "invalid_headers_frame");
if (cursor == 0)
{
streamId &= 0x7F_FF_FF_FF;
parentStreamId &= 0x7F_FF_FF_FF;
state = State.WEIGHT;
if (length < 1)
return connectionFailure(buffer, ErrorCode.FRAME_SIZE_ERROR.code, "invalid_headers_frame");
@ -177,7 +176,7 @@ public class HeadersBodyParser extends BodyParser
{
state = State.PADDING;
loop = paddingLength == 0;
onHeaders(streamId, weight, exclusive, metaData);
onHeaders(parentStreamId, weight, exclusive, metaData);
}
}
else
@ -193,7 +192,7 @@ public class HeadersBodyParser extends BodyParser
headerBlockFragments.setStreamId(getStreamId());
headerBlockFragments.setEndStream(isEndStream());
if (hasFlag(Flags.PRIORITY))
headerBlockFragments.setPriorityFrame(new PriorityFrame(streamId, getStreamId(), weight, exclusive));
headerBlockFragments.setPriorityFrame(new PriorityFrame(getStreamId(), parentStreamId, weight, exclusive));
headerBlockFragments.storeFragment(buffer, length, false);
state = State.PADDING;
loop = paddingLength == 0;
@ -222,17 +221,17 @@ public class HeadersBodyParser extends BodyParser
return false;
}
private void onHeaders(int streamId, int weight, boolean exclusive, MetaData metaData)
private void onHeaders(int parentStreamId, int weight, boolean exclusive, MetaData metaData)
{
PriorityFrame priorityFrame = null;
if (hasFlag(Flags.PRIORITY))
priorityFrame = new PriorityFrame(streamId, getStreamId(), weight, exclusive);
priorityFrame = new PriorityFrame(getStreamId(), parentStreamId, weight, exclusive);
HeadersFrame frame = new HeadersFrame(getStreamId(), metaData, priorityFrame, isEndStream());
notifyHeaders(frame);
}
private enum State
{
PREPARE, PADDING_LENGTH, EXCLUSIVE, STREAM_ID, STREAM_ID_BYTES, WEIGHT, HEADERS, PADDING
PREPARE, PADDING_LENGTH, EXCLUSIVE, PARENT_STREAM_ID, PARENT_STREAM_ID_BYTES, WEIGHT, HEADERS, PADDING
}
}

18
jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/HeadersGenerateParseTest.java
View File

@ -66,7 +66,8 @@ public class HeadersGenerateParseTest
for (int i = 0; i < 2; ++i)
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.generateHeaders(lease, streamId, metaData, false);
PriorityFrame priorityFrame = new PriorityFrame(streamId, 3 * streamId, 200, true);
generator.generateHeaders(lease, streamId, metaData, priorityFrame, true);
frames.clear();
for (ByteBuffer buffer : lease.getByteBuffers())
@ -89,6 +90,12 @@ public class HeadersGenerateParseTest
HttpField field = fields.getField(j);
Assert.assertTrue(request.getFields().contains(field));
}
PriorityFrame priority = frame.getPriority();
Assert.assertNotNull(priority);
Assert.assertEquals(priorityFrame.getStreamId(), priority.getStreamId());
Assert.assertEquals(priorityFrame.getParentStreamId(), priority.getParentStreamId());
Assert.assertEquals(priorityFrame.getWeight(), priority.getWeight());
Assert.assertEquals(priorityFrame.isExclusive(), priority.isExclusive());
}
}
@ -114,7 +121,8 @@ public class HeadersGenerateParseTest
MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP, new HostPortHttpField("localhost:8080"), "/path", HttpVersion.HTTP_2, fields);
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.generateHeaders(lease, streamId, metaData, false);
PriorityFrame priorityFrame = new PriorityFrame(streamId, 3 * streamId, 200, true);
generator.generateHeaders(lease, streamId, metaData, priorityFrame, true);
for (ByteBuffer buffer : lease.getByteBuffers())
{
@ -136,5 +144,11 @@ public class HeadersGenerateParseTest
HttpField field = fields.getField(j);
Assert.assertTrue(request.getFields().contains(field));
}
PriorityFrame priority = frame.getPriority();
Assert.assertNotNull(priority);
Assert.assertEquals(priorityFrame.getStreamId(), priority.getStreamId());
Assert.assertEquals(priorityFrame.getParentStreamId(), priority.getParentStreamId());
Assert.assertEquals(priorityFrame.getWeight(), priority.getWeight());
Assert.assertEquals(priorityFrame.isExclusive(), priority.isExclusive());
}
}

70
jetty-http2/http2-server/src/test/java/org/eclipse/jetty/http2/server/HTTP2ServerTest.java
View File

@ -40,15 +40,19 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.http2.ErrorCode;
import org.eclipse.jetty.http2.Flags;
import org.eclipse.jetty.http2.api.Stream;
import org.eclipse.jetty.http2.api.server.ServerSessionListener;
import org.eclipse.jetty.http2.frames.DataFrame;
import org.eclipse.jetty.http2.frames.FrameType;
import org.eclipse.jetty.http2.frames.GoAwayFrame;
import org.eclipse.jetty.http2.frames.HeadersFrame;
import org.eclipse.jetty.http2.frames.PingFrame;
import org.eclipse.jetty.http2.frames.PrefaceFrame;
import org.eclipse.jetty.http2.frames.PriorityFrame;
import org.eclipse.jetty.http2.frames.SettingsFrame;
import org.eclipse.jetty.http2.generator.Generator;
import org.eclipse.jetty.http2.parser.Parser;
@ -407,7 +411,7 @@ public class HTTP2ServerTest extends AbstractServerTest
@Test
public void testRequestWithContinuationFrames() throws Exception
{
testRequestWithContinuationFrames(() ->
testRequestWithContinuationFrames(null, () ->
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
@ -418,10 +422,25 @@ public class HTTP2ServerTest extends AbstractServerTest
});
}
@Test
public void testRequestWithPriorityWithContinuationFrames() throws Exception
{
PriorityFrame priority = new PriorityFrame(1, 13, 200, true);
testRequestWithContinuationFrames(priority, () ->
{
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, priority, true));
return lease;
});
}
@Test
public void testRequestWithContinuationFramesWithEmptyHeadersFrame() throws Exception
{
testRequestWithContinuationFrames(() ->
testRequestWithContinuationFrames(null, () ->
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
@ -439,10 +458,32 @@ public class HTTP2ServerTest extends AbstractServerTest
});
}
@Test
public void testRequestWithPriorityWithContinuationFramesWithEmptyHeadersFrame() throws Exception
{
PriorityFrame priority = new PriorityFrame(1, 13, 200, true);
testRequestWithContinuationFrames(null, () ->
{
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, priority, true));
// Take the HeadersFrame header and set the length to just the priority frame.
List<ByteBuffer> buffers = lease.getByteBuffers();
ByteBuffer headersFrameHeader = buffers.get(2);
headersFrameHeader.put(0, (byte)0);
headersFrameHeader.putShort(1, (short)PriorityFrame.PRIORITY_LENGTH);
// Insert a CONTINUATION frame header for the body of the HEADERS frame.
lease.insert(4, buffers.get(5).slice(), false);
return lease;
});
}
@Test
public void testRequestWithContinuationFramesWithEmptyContinuationFrame() throws Exception
{
testRequestWithContinuationFrames(() ->
testRequestWithContinuationFrames(null, () ->
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
@ -466,7 +507,7 @@ public class HTTP2ServerTest extends AbstractServerTest
@Test
public void testRequestWithContinuationFramesWithEmptyLastContinuationFrame() throws Exception
{
testRequestWithContinuationFrames(() ->
testRequestWithContinuationFrames(null, () ->
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.control(lease, new PrefaceFrame());
@ -489,15 +530,30 @@ public class HTTP2ServerTest extends AbstractServerTest
});
}
private void testRequestWithContinuationFrames(Callable<ByteBufferPool.Lease> frames) throws Exception
private void testRequestWithContinuationFrames(PriorityFrame priorityFrame, Callable<ByteBufferPool.Lease> frames) throws Exception
{
final CountDownLatch serverLatch = new CountDownLatch(1);
startServer(new HttpServlet()
startServer(new ServerSessionListener.Adapter()
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
{
if (priorityFrame != null)
{
PriorityFrame priority = frame.getPriority();
Assert.assertNotNull(priority);
Assert.assertEquals(priorityFrame.getStreamId(), priority.getStreamId());
Assert.assertEquals(priorityFrame.getParentStreamId(), priority.getParentStreamId());
Assert.assertEquals(priorityFrame.getWeight(), priority.getWeight());
Assert.assertEquals(priorityFrame.isExclusive(), priority.isExclusive());
}
serverLatch.countDown();
MetaData.Response metaData = new MetaData.Response(HttpVersion.HTTP_2, 200, new HttpFields());
HeadersFrame responseFrame = new HeadersFrame(stream.getId(), metaData, null, true);
stream.headers(responseFrame, Callback.NOOP);
return null;
}
});
generator = new Generator(byteBufferPool, 4096, 4);