Skip to content

Commit

Permalink
Merge pull request #7802 from eclipse/jetty-10.0.x-qpack-sectionAck
Browse files Browse the repository at this point in the history
HTTP/3 QPACK - do not expect section ack for zero required insert count
  • Loading branch information
lachlan-roberts committed May 15, 2022
2 parents 6d38e55 + 2e98a26 commit 07af5d7
Show file tree
Hide file tree
Showing 4 changed files with 157 additions and 3 deletions.
Expand Up @@ -166,7 +166,8 @@ public boolean decode(long streamId, ByteBuffer buffer, Handler handler) throws
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
{
Expand Down Expand Up @@ -236,15 +237,17 @@ private void checkEncodedFieldSections() throws QpackException
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);
if (LOG.isDebugEnabled())
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));
}
}
}
Expand Down
Expand Up @@ -193,7 +193,15 @@ public void encode(ByteBuffer buffer, long streamId, MetaData metadata) throws Q
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;
Expand Down
Expand Up @@ -16,6 +16,10 @@
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;
Expand All @@ -27,6 +31,23 @@

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+", "");
Expand Down Expand Up @@ -56,4 +77,43 @@ public static String toHexString(Instruction instruction)
{
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);
}
}
@@ -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"));
}
}

0 comments on commit 07af5d7

Please sign in to comment.