Allow numBytes == 0 when calling Http2LocalFlowController.consumeBytes.
Motivation: Sometimes people use a data frame with length 0 to end a stream(such as jetty http2-server). So it is possible that data.readableBytes and padding are all 0 for a data frame, and cause an IllegalArgumentException when calling flowController.consumeBytes. Modifications: Return false when numBytes == 0 instead of throwing IllegalArgumentException. Result: Fix IllegalArgumentException.
This commit is contained in:
parent
319d745a13
commit
508d6e3b31
@ -249,9 +249,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
|
||||
throw e;
|
||||
} finally {
|
||||
// If appropriate, return the processed bytes to the flow controller.
|
||||
if (bytesToReturn > 0) {
|
||||
flowController.consumeBytes(ctx, stream, bytesToReturn);
|
||||
}
|
||||
flowController.consumeBytes(ctx, stream, bytesToReturn);
|
||||
|
||||
if (endOfStream) {
|
||||
lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture());
|
||||
|
@ -142,16 +142,18 @@ public class DefaultHttp2LocalFlowController implements Http2LocalFlowController
|
||||
@Override
|
||||
public boolean consumeBytes(ChannelHandlerContext ctx, Http2Stream stream, int numBytes)
|
||||
throws Http2Exception {
|
||||
if (numBytes < 0) {
|
||||
throw new IllegalArgumentException("numBytes must not be negative");
|
||||
}
|
||||
if (numBytes == 0) {
|
||||
return false;
|
||||
}
|
||||
// Streams automatically consume all remaining bytes when they are closed, so just ignore
|
||||
// if already closed.
|
||||
if (stream != null && !isClosed(stream)) {
|
||||
if (stream.id() == CONNECTION_STREAM_ID) {
|
||||
throw new UnsupportedOperationException("Returning bytes for the connection window is not supported");
|
||||
}
|
||||
if (numBytes <= 0) {
|
||||
throw new IllegalArgumentException("numBytes must be positive");
|
||||
}
|
||||
|
||||
boolean windowUpdateSent = connectionState().consumeBytes(ctx, numBytes);
|
||||
windowUpdateSent |= state(stream).consumeBytes(ctx, numBytes);
|
||||
return windowUpdateSent;
|
||||
|
@ -253,14 +253,13 @@ public class DefaultHttp2ConnectionDecoderTest {
|
||||
public void emptyDataFrameShouldApplyFlowControl() throws Exception {
|
||||
final ByteBuf data = EMPTY_BUFFER;
|
||||
int padding = 0;
|
||||
int processedBytes = data.readableBytes() + padding;
|
||||
mockFlowControl(processedBytes);
|
||||
mockFlowControl(0);
|
||||
try {
|
||||
decode().onDataRead(ctx, STREAM_ID, data, padding, true);
|
||||
verify(localFlow).receiveFlowControlledFrame(eq(ctx), eq(stream), eq(data), eq(padding), eq(true));
|
||||
|
||||
// No bytes were consumed, so there's no window update needed.
|
||||
verify(localFlow, never()).consumeBytes(eq(ctx), eq(stream), eq(processedBytes));
|
||||
// Now we ignore the empty bytes inside consumeBytes method, so it will be called once.
|
||||
verify(localFlow).consumeBytes(eq(ctx), eq(stream), eq(0));
|
||||
|
||||
// Verify that the empty data event was propagated to the observer.
|
||||
verify(listener).onDataRead(eq(ctx), eq(STREAM_ID), eq(data), eq(padding), eq(true));
|
||||
|
@ -247,6 +247,16 @@ public class DefaultHttp2LocalFlowControllerTest {
|
||||
testRatio(ratio, DEFAULT_WINDOW_SIZE << 1, 3, true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void consumeBytesForZeroNumBytesShouldIgnore() throws Http2Exception {
|
||||
assertFalse(controller.consumeBytes(ctx, connection.stream(STREAM_ID), 0));
|
||||
}
|
||||
|
||||
@Test(expected = IllegalArgumentException.class)
|
||||
public void consumeBytesForNegativeNumBytesShouldFail() throws Http2Exception {
|
||||
assertFalse(controller.consumeBytes(ctx, connection.stream(STREAM_ID), -1));
|
||||
}
|
||||
|
||||
private void testRatio(float ratio, int newDefaultWindowSize, int newStreamId, boolean setStreamRatio)
|
||||
throws Http2Exception {
|
||||
int delta = newDefaultWindowSize - DEFAULT_WINDOW_SIZE;
|
||||
|
Loading…
Reference in New Issue
Block a user