Filter out connection specific headers for Http3Fields

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/Http3Fields.java

@@ -1,9 +1,12 @@
 package org.eclipse.jetty.http3.qpack;
 
 import java.util.ArrayList;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
-import java.util.NoSuchElementException;
+import java.util.Set;
 import java.util.stream.Stream;
 
 import org.eclipse.jetty.http.HttpField;
@@ -11,17 +14,37 @@ import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpScheme;
+import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.MetaData;
-
+import org.eclipse.jetty.http.PreEncodedHttpField;
-import static org.eclipse.jetty.http3.qpack.QpackEncoder.C_METHODS;
+import org.eclipse.jetty.util.StringUtil;
-import static org.eclipse.jetty.http3.qpack.QpackEncoder.C_SCHEME_HTTP;
-import static org.eclipse.jetty.http3.qpack.QpackEncoder.C_SCHEME_HTTPS;
-import static org.eclipse.jetty.http3.qpack.QpackEncoder.STATUSES;
 
 public class Http3Fields implements HttpFields
 {
+    public static final HttpField[] STATUSES = new HttpField[599];
+    private static final EnumSet<HttpHeader> IGNORED_HEADERS = EnumSet.of(HttpHeader.CONNECTION, HttpHeader.KEEP_ALIVE,
+        HttpHeader.PROXY_CONNECTION, HttpHeader.TRANSFER_ENCODING, HttpHeader.UPGRADE);
+    public static final PreEncodedHttpField TE_TRAILERS = new PreEncodedHttpField(HttpHeader.TE, "trailers");
+    public static final PreEncodedHttpField C_SCHEME_HTTP = new PreEncodedHttpField(HttpHeader.C_SCHEME, "http");
+    public static final PreEncodedHttpField C_SCHEME_HTTPS = new PreEncodedHttpField(HttpHeader.C_SCHEME, "https");
+    public static final EnumMap<HttpMethod, PreEncodedHttpField> C_METHODS = new EnumMap<>(HttpMethod.class);
+
+    static
+    {
+        for (HttpStatus.Code code : HttpStatus.Code.values())
+        {
+            STATUSES[code.getCode()] = new PreEncodedHttpField(HttpHeader.C_STATUS, Integer.toString(code.getCode()));
+        }
+        for (HttpMethod method : HttpMethod.values())
+        {
+            C_METHODS.put(method, new PreEncodedHttpField(HttpHeader.C_METHOD, method.asString()));
+        }
+    }
+
     private final List<HttpField> pseudoHeaders = new ArrayList<>(8);
     private final HttpFields httpFields;
+    private Set<String> hopHeaders;
+    private HttpField contentLengthHeader;
 
     public Http3Fields(MetaData metadata)
     {
@@ -55,6 +78,27 @@ public class Http3Fields implements HttpFields
                 status = new HttpField.IntValueHttpField(HttpHeader.C_STATUS, code);
             pseudoHeaders.add(status);
         }
+
+        if (httpFields != null)
+        {
+            // Remove the headers specified in the Connection header,
+            // for example: Connection: Close, TE, Upgrade, Custom.
+            for (String value : httpFields.getCSV(HttpHeader.CONNECTION, false))
+            {
+                if (hopHeaders == null)
+                    hopHeaders = new HashSet<>();
+                hopHeaders.add(StringUtil.asciiToLowerCase(value));
+            }
+
+            // If the HttpFields doesn't have content-length we will add it at the end from the metadata.
+            if (httpFields.getField(HttpHeader.CONTENT_LENGTH) == null)
+            {
+                long contentLength = metadata.getContentLength();
+                if (contentLength >= 0)
+                    contentLengthHeader = new HttpField(HttpHeader.CONTENT_LENGTH, String.valueOf(contentLength));
+            }
+        }
+
     }
 
     @Override
@@ -66,28 +110,43 @@ public class Http3Fields implements HttpFields
     @Override
     public HttpField getField(int index)
     {
-        if (index < pseudoHeaders.size())
+        return stream().skip(index).findFirst().orElse(null);
-            return pseudoHeaders.get(index);
-        else if (httpFields != null)
-            return httpFields.getField(index - pseudoHeaders.size());
-        else
-            throw new NoSuchElementException();
     }
 
     @Override
     public int size()
     {
-        if (httpFields == null)
+        return Math.toIntExact(stream().count());
-            return pseudoHeaders.size();
-        return pseudoHeaders.size() + httpFields.size();
     }
 
     @Override
     public Stream<HttpField> stream()
     {
+        Stream<HttpField> pseudoHeadersStream = pseudoHeaders.stream();
         if (httpFields == null)
-            return pseudoHeaders.stream();
+            return pseudoHeadersStream;
-        return Stream.concat(pseudoHeaders.stream(), httpFields.stream());
+
+        Stream<HttpField> httpFieldStream = httpFields.stream().filter(field ->
+        {
+            HttpHeader header = field.getHeader();
+
+            // If the header is specifically ignored skip it (Connection Specific Headers).
+            if (header != null && IGNORED_HEADERS.contains(header))
+                return false;
+
+            // If this is the TE header field it can only have the value "trailers".
+            if ((header == HttpHeader.TE) && !field.contains("trailers"))
+                return false;
+
+            // Remove the headers nominated by the Connection header field.
+            String name = field.getLowerCaseName();
+            return hopHeaders == null || !hopHeaders.contains(name);
+        });
+
+        if (contentLengthHeader != null)
+            return Stream.concat(pseudoHeadersStream, Stream.concat(httpFieldStream, Stream.of(contentLengthHeader)));
+        else
+            return Stream.concat(pseudoHeadersStream, httpFieldStream);
     }
 
     @Override

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java

@@ -16,7 +16,6 @@ package org.eclipse.jetty.http3.qpack;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.EnumMap;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.List;
@@ -24,8 +23,6 @@ import java.util.Map;
 
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpHeader;
-import org.eclipse.jetty.http.HttpMethod;
-import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.MetaData;
 import org.eclipse.jetty.http.PreEncodedHttpField;
 import org.eclipse.jetty.http3.qpack.internal.EncodableEntry;
@@ -49,7 +46,6 @@ import org.slf4j.LoggerFactory;
 public class QpackEncoder implements Dumpable
 {
     private static final Logger LOG = LoggerFactory.getLogger(QpackEncoder.class);
-    public static final HttpField[] STATUSES = new HttpField[599];
     public static final EnumSet<HttpHeader> DO_NOT_HUFFMAN =
         EnumSet.of(
             HttpHeader.AUTHORIZATION,
@@ -81,24 +77,6 @@ public class QpackEncoder implements Dumpable
             HttpHeader.AUTHORIZATION,
             HttpHeader.SET_COOKIE,
             HttpHeader.SET_COOKIE2);
-    private static final EnumSet<HttpHeader> IGNORED_HEADERS = EnumSet.of(HttpHeader.CONNECTION, HttpHeader.KEEP_ALIVE,
-        HttpHeader.PROXY_CONNECTION, HttpHeader.TRANSFER_ENCODING, HttpHeader.UPGRADE);
-    public static final PreEncodedHttpField TE_TRAILERS = new PreEncodedHttpField(HttpHeader.TE, "trailers");
-    public static final PreEncodedHttpField C_SCHEME_HTTP = new PreEncodedHttpField(HttpHeader.C_SCHEME, "http");
-    public static final PreEncodedHttpField C_SCHEME_HTTPS = new PreEncodedHttpField(HttpHeader.C_SCHEME, "https");
-    public static final EnumMap<HttpMethod, PreEncodedHttpField> C_METHODS = new EnumMap<>(HttpMethod.class);
-
-    static
-    {
-        for (HttpStatus.Code code : HttpStatus.Code.values())
-        {
-            STATUSES[code.getCode()] = new PreEncodedHttpField(HttpHeader.C_STATUS, Integer.toString(code.getCode()));
-        }
-        for (HttpMethod method : HttpMethod.values())
-        {
-            C_METHODS.put(method, new PreEncodedHttpField(HttpHeader.C_METHOD, method.asString()));
-        }
-    }
 
     public interface Handler
     {
@@ -148,97 +126,6 @@ public class QpackEncoder implements Dumpable
         }
     }
 
-    public void parseInstruction(ByteBuffer buffer) throws QpackException
-    {
-        _parser.parse(buffer);
-    }
-
-    void insertCountIncrement(int increment) throws QpackException
-    {
-        synchronized (this)
-        {
-            int insertCount = _context.getDynamicTable().getInsertCount();
-            if (_knownInsertCount + increment > insertCount)
-                throw new QpackException.StreamException("KnownInsertCount incremented over InsertCount");
-            _knownInsertCount += increment;
-        }
-    }
-
-    void sectionAcknowledgement(int streamId) throws QpackException
-    {
-        synchronized (this)
-        {
-            StreamInfo streamInfo = _streamInfoMap.get(streamId);
-            if (streamInfo == null)
-                throw new QpackException.StreamException("No StreamInfo for " + streamId);
-
-            // The KnownInsertCount should be updated to the earliest sent RequiredInsertCount on that stream.
-            StreamInfo.SectionInfo sectionInfo = streamInfo.acknowledge();
-            sectionInfo.release();
-            _knownInsertCount = Math.max(_knownInsertCount, sectionInfo.getRequiredInsertCount());
-
-            // If we have no more outstanding section acknowledgments remove the StreamInfo.
-            if (streamInfo.isEmpty())
-                _streamInfoMap.remove(streamId);
-        }
-    }
-
-    void streamCancellation(int streamId) throws QpackException
-    {
-        synchronized (this)
-        {
-            StreamInfo streamInfo = _streamInfoMap.remove(streamId);
-            if (streamInfo == null)
-                throw new QpackException.StreamException("No StreamInfo for " + streamId);
-
-            // Release all referenced entries outstanding on the stream that was cancelled.
-            for (StreamInfo.SectionInfo sectionInfo : streamInfo)
-            {
-                sectionInfo.release();
-            }
-        }
-    }
-
-    private boolean referenceEntry(Entry entry, StreamInfo streamInfo)
-    {
-        if (entry == null)
-            return false;
-
-        if (entry.isStatic())
-            return true;
-
-        boolean inEvictionZone = !_context.getDynamicTable().canReference(entry);
-        if (inEvictionZone)
-            return false;
-
-        StreamInfo.SectionInfo sectionInfo = streamInfo.getCurrentSectionInfo();
-
-        // If they have already acknowledged this entry we can reference it straight away.
-        if (_knownInsertCount >= entry.getIndex() + 1)
-        {
-            sectionInfo.reference(entry);
-            return true;
-        }
-
-        // We may need to risk blocking the stream in order to reference it.
-        if (streamInfo.isBlocked())
-        {
-            sectionInfo.block();
-            sectionInfo.reference(entry);
-            return true;
-        }
-
-        if (_blockedStreams < _maxBlockedStreams)
-        {
-            _blockedStreams++;
-            sectionInfo.block();
-            sectionInfo.reference(entry);
-            return true;
-        }
-
-        return false;
-    }
-
     protected boolean shouldIndex(HttpField httpField)
     {
         return !DO_NOT_INDEX.contains(httpField.getHeader());
@@ -249,7 +136,50 @@ public class QpackEncoder implements Dumpable
         return !DO_NOT_HUFFMAN.contains(httpField.getHeader());
     }
 
-    // TODO: Pass in buffer.
+    public void parseInstruction(ByteBuffer buffer) throws QpackException
+    {
+        _parser.parse(buffer);
+    }
+
+    public boolean insert(HttpField field) throws QpackException
+    {
+        synchronized (this)
+        {
+            DynamicTable dynamicTable = _context.getDynamicTable();
+
+            if (field.getValue() == null)
+                field = new HttpField(field.getHeader(), field.getName(), "");
+
+            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.
+            Entry entry = _context.get(field);
+            if (entry != null)
+            {
+                int index = _context.indexOf(entry);
+                dynamicTable.add(new Entry(field));
+                _handler.onInstruction(new DuplicateInstruction(index));
+                return true;
+            }
+
+            boolean huffman = shouldHuffmanEncode(field);
+            Entry nameEntry = _context.get(field.getName());
+            if (nameEntry != null)
+            {
+                int index = _context.indexOf(nameEntry);
+                dynamicTable.add(new Entry(field));
+                _handler.onInstruction(new IndexedNameEntryInstruction(!nameEntry.isStatic(), index, huffman, field.getValue()));
+                return true;
+            }
+
+            dynamicTable.add(new Entry(field));
+            _handler.onInstruction(new LiteralNameEntryInstruction(huffman, field.getName(), huffman, field.getValue()));
+            return true;
+        }
+    }
+
     public ByteBuffer encode(int streamId, MetaData metadata) throws QpackException
     {
         // Verify that we can encode without errors.
@@ -402,43 +332,90 @@ public class QpackEncoder implements Dumpable
         }
     }
 
-    public boolean insert(HttpField field) throws QpackException
+    void insertCountIncrement(int increment) throws QpackException
     {
         synchronized (this)
         {
-            DynamicTable dynamicTable = _context.getDynamicTable();
+            int insertCount = _context.getDynamicTable().getInsertCount();
+            if (_knownInsertCount + increment > insertCount)
+                throw new QpackException.StreamException("KnownInsertCount incremented over InsertCount");
+            _knownInsertCount += increment;
+        }
+    }
 
-            if (field.getValue() == null)
+    void sectionAcknowledgement(int streamId) throws QpackException
-                field = new HttpField(field.getHeader(), field.getName(), "");
+    {
+        synchronized (this)
+        {
+            StreamInfo streamInfo = _streamInfoMap.get(streamId);
+            if (streamInfo == null)
+                throw new QpackException.StreamException("No StreamInfo for " + streamId);
 
-            boolean canCreateEntry = shouldIndex(field) &&  dynamicTable.canInsert(field);
+            // The KnownInsertCount should be updated to the earliest sent RequiredInsertCount on that stream.
-            if (!canCreateEntry)
+            StreamInfo.SectionInfo sectionInfo = streamInfo.acknowledge();
+            sectionInfo.release();
+            _knownInsertCount = Math.max(_knownInsertCount, sectionInfo.getRequiredInsertCount());
+
+            // If we have no more outstanding section acknowledgments remove the StreamInfo.
+            if (streamInfo.isEmpty())
+                _streamInfoMap.remove(streamId);
+        }
+    }
+
+    void streamCancellation(int streamId) throws QpackException
+    {
+        synchronized (this)
+        {
+            StreamInfo streamInfo = _streamInfoMap.remove(streamId);
+            if (streamInfo == null)
+                throw new QpackException.StreamException("No StreamInfo for " + streamId);
+
+            // Release all referenced entries outstanding on the stream that was cancelled.
+            for (StreamInfo.SectionInfo sectionInfo : streamInfo)
+            {
+                sectionInfo.release();
+            }
+        }
+    }
+
+    private boolean referenceEntry(Entry entry, StreamInfo streamInfo)
+    {
+        if (entry == null)
             return false;
 
-            // We can always reference on insertion as it will always arrive before any eviction.
+        if (entry.isStatic())
-            Entry entry = _context.get(field);
+            return true;
-            if (entry != null)
+
+        boolean inEvictionZone = !_context.getDynamicTable().canReference(entry);
+        if (inEvictionZone)
+            return false;
+
+        StreamInfo.SectionInfo sectionInfo = streamInfo.getCurrentSectionInfo();
+
+        // If they have already acknowledged this entry we can reference it straight away.
+        if (_knownInsertCount >= entry.getIndex() + 1)
         {
-                int index = _context.indexOf(entry);
+            sectionInfo.reference(entry);
-                dynamicTable.add(new Entry(field));
-                _handler.onInstruction(new DuplicateInstruction(index));
             return true;
         }
 
-            boolean huffman = shouldHuffmanEncode(field);
+        // We may need to risk blocking the stream in order to reference it.
-            Entry nameEntry = _context.get(field.getName());
+        if (streamInfo.isBlocked())
-            if (nameEntry != null)
         {
-                int index = _context.indexOf(nameEntry);
+            sectionInfo.block();
-                dynamicTable.add(new Entry(field));
+            sectionInfo.reference(entry);
-                _handler.onInstruction(new IndexedNameEntryInstruction(!nameEntry.isStatic(), index, huffman, field.getValue()));
             return true;
         }
 
-            dynamicTable.add(new Entry(field));
+        if (_blockedStreams < _maxBlockedStreams)
-            _handler.onInstruction(new LiteralNameEntryInstruction(huffman, field.getName(), huffman, field.getValue()));
+        {
+            _blockedStreams++;
+            sectionInfo.block();
+            sectionInfo.reference(entry);
             return true;
         }
+
+        return false;
     }
 
     private static int encodeInsertCount(int reqInsertCount, int maxTableCapacity)