478434 - Priority weights should be between 1 and 256 inclusive.
Added required +1 in parsers and -1 in generators to convert from unsigned byte to weight.
This commit is contained in:
parent
3dcdb9f802
commit
83c5105e71
|
@ -34,6 +34,7 @@ public class HeadersGenerator extends FrameGenerator
|
|||
{
|
||||
private final HpackEncoder encoder;
|
||||
private final int maxHeaderBlockFragment;
|
||||
private final PriorityGenerator priorityGenerator;
|
||||
|
||||
public HeadersGenerator(HeaderGenerator headerGenerator, HpackEncoder encoder)
|
||||
{
|
||||
|
@ -45,6 +46,7 @@ public class HeadersGenerator extends FrameGenerator
|
|||
super(headerGenerator);
|
||||
this.encoder = encoder;
|
||||
this.maxHeaderBlockFragment = maxHeaderBlockFragment;
|
||||
this.priorityGenerator = new PriorityGenerator(headerGenerator);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -62,17 +64,7 @@ public class HeadersGenerator extends FrameGenerator
|
|||
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);
|
||||
|
@ -92,11 +84,10 @@ public class HeadersGenerator extends FrameGenerator
|
|||
length += PriorityFrame.PRIORITY_LENGTH;
|
||||
|
||||
ByteBuffer header = generateHeader(lease, FrameType.HEADERS, length, flags, streamId);
|
||||
generatePriority(header, priority);
|
||||
BufferUtil.flipToFlush(header, 0);
|
||||
lease.append(header, true);
|
||||
|
||||
generatePriority(lease, priority);
|
||||
|
||||
hpacked.limit(maxHeaderBlockFragment);
|
||||
lease.append(hpacked.slice(), false);
|
||||
|
||||
|
@ -130,27 +121,19 @@ public class HeadersGenerator extends FrameGenerator
|
|||
length += PriorityFrame.PRIORITY_LENGTH;
|
||||
|
||||
ByteBuffer header = generateHeader(lease, FrameType.HEADERS, length, flags, streamId);
|
||||
generatePriority(header, priority);
|
||||
BufferUtil.flipToFlush(header, 0);
|
||||
lease.append(header, true);
|
||||
|
||||
generatePriority(lease, priority);
|
||||
|
||||
lease.append(hpacked, true);
|
||||
}
|
||||
}
|
||||
|
||||
private void generatePriority(ByteBufferPool.Lease lease, PriorityFrame priority)
|
||||
private void generatePriority(ByteBuffer header, 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);
|
||||
priorityGenerator.generatePriorityBody(header, priority.getStreamId(),
|
||||
priority.getParentStreamId(), priority.getWeight(), priority.isExclusive());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -42,22 +42,27 @@ public class PriorityGenerator extends FrameGenerator
|
|||
}
|
||||
|
||||
public void generatePriority(ByteBufferPool.Lease lease, int streamId, int parentStreamId, int weight, boolean exclusive)
|
||||
{
|
||||
ByteBuffer header = generateHeader(lease, FrameType.PRIORITY, PriorityFrame.PRIORITY_LENGTH, Flags.NONE, streamId);
|
||||
generatePriorityBody(header, streamId, parentStreamId, weight, exclusive);
|
||||
BufferUtil.flipToFlush(header, 0);
|
||||
lease.append(header, true);
|
||||
}
|
||||
|
||||
public void generatePriorityBody(ByteBuffer header, int streamId, int parentStreamId, int weight, boolean exclusive)
|
||||
{
|
||||
if (streamId < 0)
|
||||
throw new IllegalArgumentException("Invalid stream id: " + streamId);
|
||||
if (parentStreamId < 0)
|
||||
throw new IllegalArgumentException("Invalid parent stream id: " + parentStreamId);
|
||||
|
||||
ByteBuffer header = generateHeader(lease, FrameType.PRIORITY, 5, Flags.NONE, streamId);
|
||||
if (parentStreamId == streamId)
|
||||
throw new IllegalArgumentException("Stream " + streamId + " cannot depend on stream " + parentStreamId);
|
||||
if (weight < 1 || weight > 256)
|
||||
throw new IllegalArgumentException("Invalid weight: " + weight);
|
||||
|
||||
if (exclusive)
|
||||
parentStreamId |= 0x80_00_00_00;
|
||||
|
||||
header.putInt(parentStreamId);
|
||||
|
||||
header.put((byte)weight);
|
||||
|
||||
BufferUtil.flipToFlush(header, 0);
|
||||
lease.append(header, true);
|
||||
header.put((byte)(weight - 1));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -161,7 +161,7 @@ public class HeadersBodyParser extends BodyParser
|
|||
}
|
||||
case WEIGHT:
|
||||
{
|
||||
weight = buffer.get() & 0xFF;
|
||||
weight = (buffer.get() & 0xFF) + 1;
|
||||
--length;
|
||||
state = State.HEADERS;
|
||||
loop = length == 0;
|
||||
|
|
|
@ -103,7 +103,7 @@ public class PriorityBodyParser extends BodyParser
|
|||
if (getStreamId() == parentStreamId)
|
||||
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_priority_frame");
|
||||
|
||||
int weight = buffer.get() & 0xFF;
|
||||
int weight = (buffer.get() & 0xFF) + 1;
|
||||
return onPriority(parentStreamId, weight, exclusive);
|
||||
}
|
||||
default:
|
||||
|
|
|
@ -51,7 +51,7 @@ public class PriorityGenerateParseTest
|
|||
|
||||
int streamId = 13;
|
||||
int parentStreamId = 17;
|
||||
int weight = 3;
|
||||
int weight = 256;
|
||||
boolean exclusive = true;
|
||||
|
||||
// Iterate a few times to be sure generator and parser are properly reset.
|
||||
|
|
|
@ -475,7 +475,7 @@ public class HTTP2ServerTest extends AbstractServerTest
|
|||
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);
|
||||
lease.insert(3, buffers.get(4).slice(), false);
|
||||
return lease;
|
||||
});
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue