From 0ccb8265322c3d48040b5f3b554fe4b1299ecaad Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 5 May 2021 16:45:10 +1000 Subject: [PATCH] Filter out connection specific headers for Http3Fields Signed-off-by: Lachlan Roberts --- .../jetty/http3/qpack/Http3Fields.java | 93 +++++-- .../jetty/http3/qpack/QpackEncoder.java | 255 ++++++++---------- 2 files changed, 192 insertions(+), 156 deletions(-) diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/Http3Fields.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/Http3Fields.java index 6921b394926..cc0ff12f29c 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/Http3Fields.java +++ b/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 static org.eclipse.jetty.http3.qpack.QpackEncoder.C_METHODS; -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; +import org.eclipse.jetty.http.PreEncodedHttpField; +import org.eclipse.jetty.util.StringUtil; public class Http3Fields implements HttpFields { + public static final HttpField[] STATUSES = new HttpField[599]; + private static final EnumSet 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 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 pseudoHeaders = new ArrayList<>(8); private final HttpFields httpFields; + private Set 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 pseudoHeaders.get(index); - else if (httpFields != null) - return httpFields.getField(index - pseudoHeaders.size()); - else - throw new NoSuchElementException(); + return stream().skip(index).findFirst().orElse(null); } @Override public int size() { - if (httpFields == null) - return pseudoHeaders.size(); - return pseudoHeaders.size() + httpFields.size(); + return Math.toIntExact(stream().count()); } @Override public Stream stream() { + Stream pseudoHeadersStream = pseudoHeaders.stream(); if (httpFields == null) - return pseudoHeaders.stream(); - return Stream.concat(pseudoHeaders.stream(), httpFields.stream()); + return pseudoHeadersStream; + + Stream 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 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 3889022126c..d12afe96c0b 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 @@ -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 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 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 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) - field = new HttpField(field.getHeader(), field.getName(), ""); + void sectionAcknowledgement(int streamId) throws QpackException + { + 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); - if (!canCreateEntry) - return false; + // 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()); - // We can always reference on insertion as it will always arrive before any eviction. - Entry entry = _context.get(field); - if (entry != null) + // 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) { - int index = _context.indexOf(entry); - dynamicTable.add(new Entry(field)); - _handler.onInstruction(new DuplicateInstruction(index)); - return true; + sectionInfo.release(); } + } + } - 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; - } + private boolean referenceEntry(Entry entry, StreamInfo streamInfo) + { + if (entry == null) + return false; - dynamicTable.add(new Entry(field)); - _handler.onInstruction(new LiteralNameEntryInstruction(huffman, field.getName(), huffman, field.getValue())); + 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; } private static int encodeInsertCount(int reqInsertCount, int maxTableCapacity)