Merge pull request #7802 from eclipse/jetty-10.0.x-qpack-sectionAck

HTTP/3 QPACK - do not expect section ack for zero required insert count
This commit is contained in:
Lachlan 2022-05-16 09:54:45 +10:00 committed by GitHub
commit 07af5d70b7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 157 additions and 3 deletions

View File

@ -166,7 +166,8 @@ public class QpackDecoder implements Dumpable
if (LOG.isDebugEnabled())
LOG.debug("Decoded: streamId={}, metadata={}", streamId, metaData);
_metaDataNotifications.add(new MetaDataNotification(streamId, metaData, handler));
_instructions.add(new SectionAcknowledgmentInstruction(streamId));
if (requiredInsertCount > 0)
_instructions.add(new SectionAcknowledgmentInstruction(streamId));
}
else
{
@ -236,7 +237,8 @@ public class QpackDecoder implements Dumpable
int insertCount = _context.getDynamicTable().getInsertCount();
for (EncodedFieldSection encodedFieldSection : _encodedFieldSections)
{
if (encodedFieldSection.getRequiredInsertCount() <= insertCount)
int requiredInsertCount = encodedFieldSection.getRequiredInsertCount();
if (requiredInsertCount <= insertCount)
{
long streamId = encodedFieldSection.getStreamId();
MetaData metaData = encodedFieldSection.decode(_context, _maxHeaderSize);
@ -244,7 +246,8 @@ public class QpackDecoder implements Dumpable
LOG.debug("Decoded: streamId={}, metadata={}", streamId, metaData);
_metaDataNotifications.add(new MetaDataNotification(streamId, metaData, encodedFieldSection.getHandler()));
_instructions.add(new SectionAcknowledgmentInstruction(streamId));
if (requiredInsertCount > 0)
_instructions.add(new SectionAcknowledgmentInstruction(streamId));
}
}
}

View File

@ -193,7 +193,15 @@ public class QpackEncoder implements Dumpable
requiredInsertCount = entryRequiredInsertCount;
}
// We should not expect section acknowledgements for 0 required insert count.
sectionInfo.setRequiredInsertCount(requiredInsertCount);
if (requiredInsertCount == 0)
{
streamInfo.remove(sectionInfo);
if (streamInfo.isEmpty())
_streamInfoMap.remove(streamId);
}
int base = dynamicTable.getBase();
int encodedInsertCount = encodeInsertCount(requiredInsertCount, dynamicTable.getCapacity());
boolean signBit = base < requiredInsertCount;

View File

@ -16,6 +16,10 @@ package org.eclipse.jetty.http3.qpack;
import java.nio.ByteBuffer;
import java.util.List;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.NullByteBufferPool;
import org.eclipse.jetty.util.BufferUtil;
@ -27,6 +31,23 @@ import static org.hamcrest.Matchers.is;
public class QpackTestUtil
{
public static ByteBuffer toBuffer(Instruction... instructions)
{
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(new NullByteBufferPool());
for (Instruction instruction : instructions)
{
instruction.encode(lease);
}
ByteBuffer combinedBuffer = BufferUtil.allocate(Math.toIntExact(lease.getTotalLength()));
BufferUtil.clearToFill(combinedBuffer);
for (ByteBuffer buffer : lease.getByteBuffers())
{
combinedBuffer.put(buffer);
}
BufferUtil.flipToFlush(combinedBuffer, 0);
return combinedBuffer;
}
public static Matcher<String> equalsHex(String expectedString)
{
expectedString = expectedString.replaceAll("\\s+", "");
@ -56,4 +77,43 @@ public class QpackTestUtil
{
return BufferUtil.toHexString(toBuffer(List.of(instruction)));
}
public static ByteBuffer encode(QpackEncoder encoder, long streamId, MetaData metaData) throws QpackException
{
ByteBuffer buffer = BufferUtil.allocate(1024);
BufferUtil.clearToFill(buffer);
encoder.encode(buffer, streamId, metaData);
BufferUtil.flipToFlush(buffer, 0);
return buffer;
}
public static HttpFields.Mutable toHttpFields(HttpField field)
{
return HttpFields.build().add(field);
}
public static MetaData toMetaData(String name, String value)
{
return toMetaData(toHttpFields(new HttpField(name, value)));
}
public static MetaData toMetaData(String method, String path, String scheme)
{
return toMetaData(method, path, scheme, null);
}
public static MetaData toMetaData(String method, String path, String scheme, HttpFields.Mutable fields)
{
if (fields == null)
fields = HttpFields.build();
fields.put(":scheme", scheme);
fields.put(":method", method);
fields.put(":path", path);
return new MetaData(HttpVersion.HTTP_3, fields);
}
public static MetaData toMetaData(HttpFields fields)
{
return new MetaData(HttpVersion.HTTP_3, fields);
}
}

View File

@ -0,0 +1,83 @@
//
// ========================================================================
// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//
package org.eclipse.jetty.http3.qpack;
import java.nio.ByteBuffer;
import org.eclipse.jetty.http3.qpack.QpackException.SessionException;
import org.eclipse.jetty.http3.qpack.internal.instruction.SectionAcknowledgmentInstruction;
import org.eclipse.jetty.util.BufferUtil;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import static org.eclipse.jetty.http3.qpack.QpackTestUtil.encode;
import static org.eclipse.jetty.http3.qpack.QpackTestUtil.toBuffer;
import static org.eclipse.jetty.http3.qpack.QpackTestUtil.toMetaData;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
public class SectionAcknowledgmentTest
{
private static final int MAX_BLOCKED_STREAMS = 5;
private static final int MAX_HEADER_SIZE = 1024;
private QpackEncoder _encoder;
private QpackDecoder _decoder;
private TestDecoderHandler _decoderHandler;
private TestEncoderHandler _encoderHandler;
@BeforeEach
public void before()
{
_encoderHandler = new TestEncoderHandler();
_decoderHandler = new TestDecoderHandler();
_encoder = new QpackEncoder(_encoderHandler, MAX_BLOCKED_STREAMS);
_decoder = new QpackDecoder(_decoderHandler, MAX_HEADER_SIZE);
}
@Test
public void testSectionAcknowledgmentForZeroRequiredInsertCountOnDecoder() throws Exception
{
// Encode a header with only a value contained in the static table.
ByteBuffer buffer = encode(_encoder, 0, toMetaData("GET", "/", "http"));
// No instruction since no addition to table.
Instruction instruction = _encoderHandler.getInstruction();
assertNull(instruction);
// Decoding should generate no instruction.
_decoder.decode(0, buffer, _decoderHandler);
instruction = _decoderHandler.getInstruction();
assertNull(instruction);
}
@Test
public void testSectionAcknowledgmentForZeroRequiredInsertCountOnEncoder() throws Exception
{
// Encode a header with only a value contained in the static table.
ByteBuffer buffer = encode(_encoder, 0, toMetaData("GET", "/", "http"));
assertThat(BufferUtil.remaining(buffer), greaterThan(0L));
// Parsing a section ack instruction on the encoder when we are not expecting it should result in QPACK_DECODER_STREAM_ERROR.
SectionAcknowledgmentInstruction instruction = new SectionAcknowledgmentInstruction(0);
ByteBuffer instructionBuffer = toBuffer(instruction);
SessionException error = assertThrows(SessionException.class, () -> _encoder.parseInstructions(instructionBuffer));
assertThat(error.getErrorCode(), equalTo(QpackException.QPACK_ENCODER_STREAM_ERROR));
assertThat(error.getMessage(), containsString("No StreamInfo for 0"));
}
}