diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/Instruction.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/Instruction.java
index bb2d1807c8b..04d0ff46da7 100644
--- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/Instruction.java
+++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/Instruction.java
@@ -21,6 +21,12 @@ public interface Instruction
{
void encode(ByteBufferPool.Lease lease);
+ /**
+ *
A handler for instructions issued by an {@link QpackEncoder} or {@link QpackDecoder}.
+ * Note: an encoder SHOULD NOT write an instruction unless sufficient stream and connection flow control
+ * credit is available for the entire instruction, otherwise a stream containing a large instruction can become
+ * deadlocked.
+ */
interface Handler
{
void onInstructions(List instructions);
diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java
index 710d5764afa..b9535fda05e 100644
--- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java
+++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java
@@ -28,15 +28,14 @@ import org.eclipse.jetty.http3.qpack.internal.parser.EncodedFieldSection;
import org.eclipse.jetty.http3.qpack.internal.table.DynamicTable;
import org.eclipse.jetty.http3.qpack.internal.table.Entry;
import org.eclipse.jetty.http3.qpack.internal.table.StaticTable;
-import org.eclipse.jetty.http3.qpack.internal.util.EncodingException;
import org.eclipse.jetty.http3.qpack.internal.util.NBitIntegerParser;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.component.Dumpable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static org.eclipse.jetty.http3.qpack.QpackException.QPACK_DECODER_STREAM_ERROR;
import static org.eclipse.jetty.http3.qpack.QpackException.QPACK_DECOMPRESSION_FAILED;
+import static org.eclipse.jetty.http3.qpack.QpackException.QPACK_ENCODER_STREAM_ERROR;
public class QpackDecoder implements Dumpable
{
@@ -77,7 +76,7 @@ public class QpackDecoder implements Dumpable
{
_context = new QpackContext();
_handler = handler;
- _parser = new DecoderInstructionParser(new DecoderAdapter());
+ _parser = new DecoderInstructionParser(new InstructionHandler());
_maxHeaderSize = maxHeaderSize;
}
@@ -150,18 +149,20 @@ public class QpackDecoder implements Dumpable
LOG.debug("Deferred Decoding: streamId={}, encodedFieldSection={}", streamId, encodedFieldSection);
_encodedFieldSections.add(encodedFieldSection);
}
+
+ boolean hadMetaData = !_metaDataNotifications.isEmpty();
+ notifyInstructionHandler();
+ notifyMetaDataHandler();
+ return hadMetaData;
+ }
+ catch (QpackException.SessionException e)
+ {
+ throw e;
}
catch (Throwable t)
{
- notifyInstructionHandler();
- notifyMetaDataHandler();
- throw t;
+ throw new QpackException.SessionException(QPACK_ENCODER_STREAM_ERROR, t.getMessage(), t);
}
-
- boolean hadMetaData = !_metaDataNotifications.isEmpty();
- notifyInstructionHandler();
- notifyMetaDataHandler();
- return hadMetaData;
}
/**
@@ -179,17 +180,17 @@ public class QpackDecoder implements Dumpable
{
_parser.parse(buffer);
}
- }
- catch (EncodingException e)
- {
- // There was an error decoding the instruction.
- throw new QpackException.SessionException(QPACK_DECODER_STREAM_ERROR, e.getMessage(), e);
- }
- finally
- {
notifyInstructionHandler();
notifyMetaDataHandler();
}
+ catch (QpackException.SessionException e)
+ {
+ throw e;
+ }
+ catch (Throwable t)
+ {
+ throw new QpackException.SessionException(QPACK_ENCODER_STREAM_ERROR, t.getMessage(), t);
+ }
}
private void checkEncodedFieldSections() throws QpackException
@@ -302,7 +303,7 @@ public class QpackDecoder implements Dumpable
return String.format("QpackDecoder@%x{%s}", hashCode(), _context);
}
- private void notifyInstructionHandler() throws QpackException
+ private void notifyInstructionHandler()
{
if (!_instructions.isEmpty())
_handler.onInstructions(_instructions);
@@ -321,7 +322,7 @@ public class QpackDecoder implements Dumpable
/**
* This delivers notifications from the DecoderInstruction parser directly into the Decoder.
*/
- class DecoderAdapter implements DecoderInstructionParser.Handler
+ class InstructionHandler implements DecoderInstructionParser.Handler
{
@Override
public void onSetDynamicTableCapacity(int capacity)
diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java
index 97c204a29d2..9ac136fcca3 100644
--- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java
+++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java
@@ -14,6 +14,7 @@
package org.eclipse.jetty.http3.qpack;
import java.io.IOException;
+import java.nio.BufferOverflowException;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.EnumSet;
@@ -43,6 +44,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static org.eclipse.jetty.http3.qpack.QpackException.H3_GENERAL_PROTOCOL_ERROR;
+import static org.eclipse.jetty.http3.qpack.QpackException.QPACK_DECODER_STREAM_ERROR;
import static org.eclipse.jetty.http3.qpack.QpackException.QPACK_ENCODER_STREAM_ERROR;
public class QpackEncoder implements Dumpable
@@ -98,7 +100,7 @@ public class QpackEncoder implements Dumpable
_handler = handler;
_context = new QpackContext();
_maxBlockedStreams = maxBlockedStreams;
- _parser = new EncoderInstructionParser(new EncoderAdapter());
+ _parser = new EncoderInstructionParser(new InstructionHandler());
}
/**
@@ -140,6 +142,10 @@ public class QpackEncoder implements Dumpable
List encodableEntries = new ArrayList<>();
DynamicTable dynamicTable = _context.getDynamicTable();
+ // We need to remember what fields were referenced for each stream for multiple reasons:
+ // 1. We can only (potentially) block up to SETTINGS_QPACK_BLOCKED_STREAMS by referencing entries which may not have arrived.
+ // 2. We need to remember reference counts to each entry which are then acknowledged by the remote decoder, this
+ // allows us to know when we can evict an entry (when it has no un-acknowledged references).
StreamInfo streamInfo = _streamInfoMap.get(streamId);
if (streamInfo == null)
{
@@ -149,41 +155,57 @@ public class QpackEncoder implements Dumpable
StreamInfo.SectionInfo sectionInfo = new StreamInfo.SectionInfo();
streamInfo.add(sectionInfo);
- int requiredInsertCount = 0;
-
- // This will also extract pseudo headers from the metadata.
- Http3Fields httpFields = new Http3Fields(metadata);
- for (HttpField field : httpFields)
+ try
{
- EncodableEntry entry = encode(streamInfo, field);
- encodableEntries.add(entry);
+ int requiredInsertCount = 0;
+ for (HttpField field : new Http3Fields(metadata))
+ {
+ EncodableEntry entry = encode(streamInfo, field);
+ encodableEntries.add(entry);
- // Update the required InsertCount.
- int entryRequiredInsertCount = entry.getRequiredInsertCount();
- if (entryRequiredInsertCount > requiredInsertCount)
- requiredInsertCount = entryRequiredInsertCount;
+ // Update the required InsertCount.
+ int entryRequiredInsertCount = entry.getRequiredInsertCount();
+ if (entryRequiredInsertCount > requiredInsertCount)
+ requiredInsertCount = entryRequiredInsertCount;
+ }
+
+ sectionInfo.setRequiredInsertCount(requiredInsertCount);
+ int base = dynamicTable.getBase();
+ int encodedInsertCount = encodeInsertCount(requiredInsertCount, dynamicTable.getCapacity());
+ boolean signBit = base < requiredInsertCount;
+ int deltaBase = signBit ? requiredInsertCount - base - 1 : base - requiredInsertCount;
+
+ // Encode all the entries into the buffer.
+ int pos = BufferUtil.flipToFill(buffer);
+
+ // Encode the Field Section Prefix into the ByteBuffer.
+ NBitIntegerEncoder.encode(buffer, 8, encodedInsertCount);
+ buffer.put(signBit ? (byte)0x80 : (byte)0x00);
+ NBitIntegerEncoder.encode(buffer, 7, deltaBase);
+
+ // Encode the field lines into the ByteBuffer.
+ for (EncodableEntry entry : encodableEntries)
+ {
+ entry.encode(buffer, base);
+ }
+
+ BufferUtil.flipToFlush(buffer, pos);
+ notifyInstructionHandler();
}
-
- sectionInfo.setRequiredInsertCount(requiredInsertCount);
- int base = dynamicTable.getBase();
- int encodedInsertCount = encodeInsertCount(requiredInsertCount, dynamicTable.getCapacity());
- boolean signBit = base < requiredInsertCount;
- int deltaBase = signBit ? requiredInsertCount - base - 1 : base - requiredInsertCount;
-
- // Encode all the entries into the buffer.
-
- // Encode the Field Section Prefix into the ByteBuffer.
- NBitIntegerEncoder.encode(buffer, 8, encodedInsertCount);
- buffer.put(signBit ? (byte)0x80 : (byte)0x00);
- NBitIntegerEncoder.encode(buffer, 7, deltaBase);
-
- // Encode the field lines into the ByteBuffer.
- for (EncodableEntry entry : encodableEntries)
+ catch (BufferOverflowException e)
{
- entry.encode(buffer, base);
+ // TODO: We have already added to the dynamic table so we need to send the instructions to maintain correct state.
+ // Can we prevent adding to the table until we know the buffer has enough space?
+ notifyInstructionHandler();
+ streamInfo.remove(sectionInfo);
+ sectionInfo.release();
+ throw new QpackException.StreamException(H3_GENERAL_PROTOCOL_ERROR, "buffer_space_exceeded", e);
+ }
+ catch (Throwable t)
+ {
+ // We are failing the whole Session so don't need to send instructions back.
+ throw new QpackException.SessionException(H3_GENERAL_PROTOCOL_ERROR, "compression_error", t);
}
-
- notifyInstructionHandler();
}
/**
@@ -195,11 +217,22 @@ public class QpackEncoder implements Dumpable
*/
public void parseInstructionBuffer(ByteBuffer buffer) throws QpackException
{
- while (BufferUtil.hasContent(buffer))
+ try
{
- _parser.parse(buffer);
+ while (BufferUtil.hasContent(buffer))
+ {
+ _parser.parse(buffer);
+ }
+ notifyInstructionHandler();
+ }
+ catch (QpackException.SessionException e)
+ {
+ throw e;
+ }
+ catch (Throwable t)
+ {
+ throw new QpackException.SessionException(QPACK_DECODER_STREAM_ERROR, t.getMessage(), t);
}
- notifyInstructionHandler();
}
/**
@@ -211,15 +244,15 @@ public class QpackEncoder implements Dumpable
public boolean insert(HttpField field)
{
DynamicTable dynamicTable = _context.getDynamicTable();
-
if (field.getValue() == null)
field = new HttpField(field.getHeader(), field.getName(), "");
+ // If we should not index this entry or there is no room to insert it, then just return false.
boolean canCreateEntry = shouldIndex(field) && dynamicTable.canInsert(field);
if (!canCreateEntry)
return false;
- // We can always reference on insertion as it will always arrive before any eviction.
+ // Can we insert by duplicating an existing entry?
Entry entry = _context.get(field);
if (entry != null)
{
@@ -230,6 +263,7 @@ public class QpackEncoder implements Dumpable
return true;
}
+ // Can we insert by referencing a name?
boolean huffman = shouldHuffmanEncode(field);
Entry nameEntry = _context.get(field.getName());
if (nameEntry != null)
@@ -241,9 +275,9 @@ public class QpackEncoder implements Dumpable
return true;
}
+ // Add the entry without referencing an existing entry.
dynamicTable.add(new Entry(field));
_instructions.add(new LiteralNameEntryInstruction(field, huffman));
-
notifyInstructionHandler();
return true;
}
@@ -265,8 +299,6 @@ public class QpackEncoder implements Dumpable
if (field.getValue() == null)
field = new HttpField(field.getHeader(), field.getName(), "");
- // TODO: The field.getHeader() could be null.
-
if (field instanceof PreEncodedHttpField)
return EncodableEntry.getPreEncodedEntry((PreEncodedHttpField)field);
@@ -431,7 +463,7 @@ public class QpackEncoder implements Dumpable
_instructions.clear();
}
- public class EncoderAdapter implements EncoderInstructionParser.Handler
+ class InstructionHandler implements EncoderInstructionParser.Handler
{
@Override
public void onSectionAcknowledgement(long streamId) throws QpackException
diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/StreamInfo.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/StreamInfo.java
index c5a0ddd9fd6..11d3a7545cf 100644
--- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/StreamInfo.java
+++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/StreamInfo.java
@@ -41,6 +41,11 @@ public class StreamInfo implements Iterable
_sectionInfos.add(sectionInfo);
}
+ public void remove(SectionInfo sectionInfo)
+ {
+ _sectionInfos.remove(sectionInfo);
+ }
+
public SectionInfo getCurrentSectionInfo()
{
return _sectionInfos.peek();
diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/parser/EncodedFieldSection.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/parser/EncodedFieldSection.java
index 0bcbf0190ab..bdca9aab539 100644
--- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/parser/EncodedFieldSection.java
+++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/parser/EncodedFieldSection.java
@@ -187,7 +187,7 @@ public class EncodedFieldSection
public interface EncodedField
{
- HttpField decode(QpackContext context) throws QpackException;
+ HttpField decode(QpackContext context);
}
private static class LiteralField implements EncodedField
@@ -222,7 +222,7 @@ public class EncodedFieldSection
}
@Override
- public HttpField decode(QpackContext context) throws QpackException
+ public HttpField decode(QpackContext context)
{
if (_dynamicTable)
return context.getDynamicTable().getAbsolute(_base - (_index + 1)).getHttpField();
@@ -241,7 +241,7 @@ public class EncodedFieldSection
}
@Override
- public HttpField decode(QpackContext context) throws QpackException
+ public HttpField decode(QpackContext context)
{
return context.getDynamicTable().getAbsolute(_base + _index).getHttpField();
}
@@ -264,7 +264,7 @@ public class EncodedFieldSection
}
@Override
- public HttpField decode(QpackContext context) throws QpackException
+ public HttpField decode(QpackContext context)
{
HttpField field;
if (_dynamicTable)
@@ -290,7 +290,7 @@ public class EncodedFieldSection
}
@Override
- public HttpField decode(QpackContext context) throws QpackException
+ public HttpField decode(QpackContext context)
{
HttpField field = context.getDynamicTable().getAbsolute(_base + _nameIndex).getHttpField();
return new HttpField(field.getHeader(), field.getName(), _value);