Skip to content

Commit

Permalink
#9900 backport Accurate implementation of H3 Request.beginNanoTime()
Browse files Browse the repository at this point in the history
Signed-off-by: Ludovic Orban <[email protected]>
  • Loading branch information
lorban committed Nov 23, 2023
1 parent f82844e commit 542f08e
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 5 deletions.
19 changes: 18 additions & 1 deletion jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,13 @@ public Request(String method, String scheme, HostPortHttpField authority, String
version, fields, contentLength);
}

public Request(long beginNanoTime, String method, String scheme, HostPortHttpField authority, String uri, HttpVersion version, HttpFields fields, long contentLength)
{
this(beginNanoTime, method,
HttpURI.build().scheme(scheme).host(authority == null ? null : authority.getHost()).port(authority == null ? -1 : authority.getPort()).pathQuery(uri),
version, fields, contentLength);
}

public Request(String method, HttpURI uri, HttpVersion version, HttpFields fields, long contentLength, Supplier<HttpFields> trailers)
{
this(NanoTime.now(), method, uri, version, fields, contentLength, trailers);
Expand Down Expand Up @@ -222,9 +229,19 @@ public ConnectRequest(HttpScheme scheme, HostPortHttpField authority, String pat
this(scheme == null ? null : scheme.asString(), authority, path, fields, protocol);
}

public ConnectRequest(long beginNanoTime, HttpScheme scheme, HostPortHttpField authority, String path, HttpFields fields, String protocol)
{
this(beginNanoTime, scheme == null ? null : scheme.asString(), authority, path, fields, protocol);
}

public ConnectRequest(String scheme, HostPortHttpField authority, String path, HttpFields fields, String protocol)
{
super(HttpMethod.CONNECT.asString(),
this(NanoTime.now(), scheme, authority, path, fields, protocol);
}

public ConnectRequest(long beginNanoTime, String scheme, HostPortHttpField authority, String path, HttpFields fields, String protocol)
{
super(beginNanoTime, HttpMethod.CONNECT.asString(),
HttpURI.build().scheme(scheme).host(authority == null ? null : authority.getHost()).port(authority == null ? -1 : authority.getPort()).pathQuery(path),
HttpVersion.HTTP_2, fields, Long.MIN_VALUE, null);
_protocol = protocol;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.eclipse.jetty.http3.internal.HTTP3ErrorCode;
import org.eclipse.jetty.http3.qpack.QpackDecoder;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.NanoTime;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -43,12 +44,15 @@ public class MessageParser
private final BooleanSupplier isLast;
private BodyParser unknownBodyParser;
private State state = State.HEADER;
protected boolean dataMode;
private boolean dataMode;
private long beginNanoTime;
private boolean beginNanoTimeStored;

public MessageParser(ParserListener listener, QpackDecoder decoder, long streamId, BooleanSupplier isLast)
{
this.listener = listener;
this.decoder = decoder;
decoder.setBeginNanoTimeSupplier(this::getBeginNanoTime);
this.streamId = streamId;
this.isLast = isLast;
}
Expand All @@ -66,6 +70,21 @@ private void reset()
{
headerParser.reset();
state = State.HEADER;
beginNanoTimeStored = false;
}

private void storeBeginNanoTime()
{
if (!beginNanoTimeStored)
{
beginNanoTime = NanoTime.now();
beginNanoTimeStored = true;
}
}

private long getBeginNanoTime()
{
return beginNanoTime;
}

public ParserListener getListener()
Expand Down Expand Up @@ -101,6 +120,7 @@ public Result parse(ByteBuffer buffer)
{
case HEADER:
{
storeBeginNanoTime();
if (headerParser.parse(buffer))
{
state = State.BODY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.eclipse.jetty.http3.qpack.QpackEncoder;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.NullByteBufferPool;
import org.eclipse.jetty.util.NanoTime;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -54,6 +55,7 @@ public void testGenerateParse()

QpackDecoder decoder = new QpackDecoder(instructions -> {});
decoder.setMaxHeadersSize(4 * 1024);
decoder.setBeginNanoTimeSupplier(NanoTime::now);
List<HeadersFrame> frames = new ArrayList<>();
MessageParser parser = new MessageParser(new ParserListener()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.LongSupplier;

import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.MetaData;
Expand Down Expand Up @@ -59,6 +60,7 @@ public class QpackDecoder implements Dumpable
private int _maxHeadersSize;
private int _maxBlockedStreams;
private int _maxTableCapacity;
private LongSupplier _beginNanoTimeSupplier;

private static class MetaDataNotification
{
Expand Down Expand Up @@ -103,6 +105,11 @@ public int getMaxHeadersSize()
return _maxHeadersSize;
}

public void setBeginNanoTimeSupplier(LongSupplier beginNanoTimeSupplier)
{
_beginNanoTimeSupplier = beginNanoTimeSupplier;
}

/**
* @param maxHeadersSize The maximum allowed size of a headers block, expressed as total of all name and value characters, plus 32 per field
*/
Expand Down Expand Up @@ -180,7 +187,7 @@ public boolean decode(long streamId, ByteBuffer buffer, Handler handler) throws
{
// Parse the buffer into an Encoded Field Section.
int base = signBit ? requiredInsertCount - deltaBase - 1 : requiredInsertCount + deltaBase;
EncodedFieldSection encodedFieldSection = new EncodedFieldSection(streamId, handler, requiredInsertCount, base, buffer);
EncodedFieldSection encodedFieldSection = new EncodedFieldSection(streamId, handler, requiredInsertCount, base, buffer, _beginNanoTimeSupplier.getAsLong());

// Decode it straight away if we can, otherwise add it to the list of EncodedFieldSections.
if (requiredInsertCount <= insertCount)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.http3.qpack.QpackException;
import org.eclipse.jetty.util.NanoTime;

import static org.eclipse.jetty.http3.qpack.QpackException.H3_GENERAL_PROTOCOL_ERROR;

Expand All @@ -40,6 +41,7 @@ public class MetaDataBuilder
private QpackException.StreamException _streamException;
private boolean _request;
private boolean _response;
private long _beginNanoTime = Long.MIN_VALUE;

/**
* @param maxHeadersSize The maximum size of the headers, expressed as total name and value characters.
Expand All @@ -59,6 +61,13 @@ public int getMaxSize()
return _maxSize;
}

public void setBeginNanoTime(long beginNanoTime)
{
if (beginNanoTime == Long.MIN_VALUE)
beginNanoTime++;
_beginNanoTime = beginNanoTime;
}

/**
* Get the size.
*
Expand Down Expand Up @@ -246,10 +255,13 @@ public MetaData build() throws QpackException.StreamException
if (_path == null)
throw new QpackException.StreamException(H3_GENERAL_PROTOCOL_ERROR, "No Path");
}
long nanoTime = _beginNanoTime == Long.MIN_VALUE ? NanoTime.now() : _beginNanoTime;
_beginNanoTime = Long.MIN_VALUE;
if (isConnect)
return new MetaData.ConnectRequest(_scheme, _authority, _path, fields, _protocol);
return new MetaData.ConnectRequest(nanoTime, _scheme, _authority, _path, fields, _protocol);
else
return new MetaData.Request(
nanoTime,
_method,
_scheme.asString(),
_authority,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,15 @@ public class EncodedFieldSection
private final int _requiredInsertCount;
private final int _base;
private final QpackDecoder.Handler _handler;
private final long _beginNanoTime;

public EncodedFieldSection(long streamId, QpackDecoder.Handler handler, int requiredInsertCount, int base, ByteBuffer content) throws QpackException
public EncodedFieldSection(long streamId, QpackDecoder.Handler handler, int requiredInsertCount, int base, ByteBuffer content, long beginNanoTime) throws QpackException
{
_streamId = streamId;
_requiredInsertCount = requiredInsertCount;
_base = base;
_handler = handler;
_beginNanoTime = beginNanoTime;

try
{
Expand Down Expand Up @@ -104,6 +106,7 @@ public MetaData decode(QpackContext context, int maxHeaderSize) throws QpackExce
HttpField decodedField = encodedField.decode(context);
metaDataBuilder.emit(decodedField);
}
metaDataBuilder.setBeginNanoTime(_beginNanoTime);
return metaDataBuilder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.eclipse.jetty.http3.qpack.internal.instruction.SectionAcknowledgmentInstruction;
import org.eclipse.jetty.http3.qpack.internal.instruction.SetCapacityInstruction;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.NanoTime;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -56,6 +57,7 @@ public void before()
_decoderHandler = new TestDecoderHandler();
_encoder = new QpackEncoder(_encoderHandler, MAX_BLOCKED_STREAMS);
_decoder = new QpackDecoder(_decoderHandler, MAX_HEADER_SIZE);
_decoder.setBeginNanoTimeSupplier(NanoTime::now);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.eclipse.jetty.http3.qpack.internal.parser.DecoderInstructionParser;
import org.eclipse.jetty.http3.qpack.internal.parser.EncoderInstructionParser;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.NanoTime;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -65,6 +66,7 @@ protected boolean shouldHuffmanEncode(HttpField httpField)
}
};
_decoder = new QpackDecoder(_decoderHandler, MAX_HEADER_SIZE);
_decoder.setBeginNanoTimeSupplier(NanoTime::now);

_encoderInstructionParser = new EncoderInstructionParser(new EncoderParserDebugHandler(_encoder));
_decoderInstructionParser = new DecoderInstructionParser(new DecoderParserDebugHandler(_decoder));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.util.NanoTime;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand All @@ -40,6 +41,7 @@ public void before()
_decoder = new QpackDecoder(_decoderHandler);
_decoder.setMaxHeadersSize(1024);
_decoder.setMaxTableCapacity(4 * 1024);
_decoder.setBeginNanoTimeSupplier(NanoTime::now);

_encoder = new QpackEncoder(_encoderHandler)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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.eclipse.jetty.util.NanoTime;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -48,6 +49,7 @@ public void before()
_decoderHandler = new TestDecoderHandler();
_encoder = new QpackEncoder(_encoderHandler, MAX_BLOCKED_STREAMS);
_decoder = new QpackDecoder(_decoderHandler, MAX_HEADER_SIZE);
_decoder.setBeginNanoTimeSupplier(NanoTime::now);
}

@Test
Expand Down

0 comments on commit 542f08e

Please sign in to comment.