Skip to content

Commit

Permalink
#9900 backport Accurate implementation of H2 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 8b6c920
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 29 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 @@ -30,6 +30,7 @@
import org.eclipse.jetty.http2.frames.WindowUpdateFrame;
import org.eclipse.jetty.http2.hpack.HpackDecoder;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.util.NanoTime;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -52,6 +53,8 @@ public class Parser
private int maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS;
private boolean continuation;
private State state = State.HEADER;
private long beginNanoTime;
private boolean nanoTimeStored;

@Deprecated
public Parser(ByteBufferPool byteBufferPool, int maxTableCapacity, int maxHeaderSize)
Expand All @@ -74,7 +77,7 @@ public Parser(ByteBufferPool byteBufferPool, int maxHeaderSize, RateControl rate
{
this.byteBufferPool = byteBufferPool;
this.headerParser = new HeaderParser(rateControl == null ? RateControl.NO_RATE_CONTROL : rateControl);
this.hpackDecoder = new HpackDecoder(maxHeaderSize);
this.hpackDecoder = new HpackDecoder(maxHeaderSize, this::getBeginNanoTime);
this.bodyParsers = new BodyParser[FrameType.values().length];
}

Expand Down Expand Up @@ -114,6 +117,25 @@ private void reset()
state = State.HEADER;
}

public long getBeginNanoTime()
{
return beginNanoTime;
}

private void clearBeginNanoTime()
{
nanoTimeStored = false;
}

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

/**
* <p>Parses the given {@code buffer} bytes and emit events to a {@link Listener}.</p>
* <p>When this method returns, the buffer may not be fully consumed, so invocations
Expand All @@ -135,6 +157,7 @@ public void parse(ByteBuffer buffer)
{
case HEADER:
{
storeBeginNanoTime();
if (!parseHeader(buffer))
return;
break;
Expand All @@ -143,6 +166,8 @@ public void parse(ByteBuffer buffer)
{
if (!parseBody(buffer))
return;
if (!continuation)
clearBeginNanoTime();
break;
}
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,102 @@ public void onConnectionFailure(int error, String reason)
}
}

@Test
public void testBeginNanoTime() throws Exception
{
ByteBufferPool bufferPool = new MappedByteBufferPool(128);
HeadersGenerator generator = new HeadersGenerator(new HeaderGenerator(), new HpackEncoder());

final List<HeadersFrame> frames = new ArrayList<>();
Parser parser = new Parser(bufferPool, 8192);
parser.init(new Parser.Listener.Adapter()
{
@Override
public void onHeaders(HeadersFrame frame)
{
frames.add(frame);
}

@Override
public void onConnectionFailure(int error, String reason)
{
frames.add(new HeadersFrame(null, null, false));
}
});

int streamId = 13;
HttpFields fields = HttpFields.build()
.put("Accept", "text/html")
.put("User-Agent", "Jetty");
MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP.asString(), new HostPortHttpField("localhost:8080"), "/path", HttpVersion.HTTP_2, fields, -1);

ByteBufferPool.Lease lease = new ByteBufferPool.Lease(bufferPool);
generator.generateHeaders(lease, streamId, metaData, null, true);

List<ByteBuffer> byteBuffers = lease.getByteBuffers();
assertEquals(2, byteBuffers.size());

ByteBuffer headersBody = byteBuffers.remove(1);
int start = headersBody.position();
int length = headersBody.remaining();
int firstHalf = length / 2;
int lastHalf = length - firstHalf;

// Adjust the length of the HEADERS frame.
ByteBuffer headersHeader = byteBuffers.get(0);
headersHeader.put(0, (byte)((firstHalf >>> 16) & 0xFF));
headersHeader.put(1, (byte)((firstHalf >>> 8) & 0xFF));
headersHeader.put(2, (byte)(firstHalf & 0xFF));

// Remove the END_HEADERS flag from the HEADERS header.
headersHeader.put(4, (byte)(headersHeader.get(4) & ~Flags.END_HEADERS));

// New HEADERS body.
headersBody.position(start);
headersBody.limit(start + firstHalf);
byteBuffers.add(headersBody.slice());

// Split the rest of the HEADERS body into a CONTINUATION frame.
byte[] continuationHeader = new byte[9];
continuationHeader[0] = (byte)((lastHalf >>> 16) & 0xFF);
continuationHeader[1] = (byte)((lastHalf >>> 8) & 0xFF);
continuationHeader[2] = (byte)(lastHalf & 0xFF);
continuationHeader[3] = (byte)FrameType.CONTINUATION.getType();
continuationHeader[4] = Flags.END_HEADERS;
continuationHeader[5] = 0x00;
continuationHeader[6] = 0x00;
continuationHeader[7] = 0x00;
continuationHeader[8] = (byte)streamId;
byteBuffers.add(ByteBuffer.wrap(continuationHeader));
// CONTINUATION body.
headersBody.position(start + firstHalf);
headersBody.limit(start + length);
byteBuffers.add(headersBody.slice());

assertEquals(4, byteBuffers.size());
parser.parse(byteBuffers.get(0));
long beginNanoTime = parser.getBeginNanoTime();
parser.parse(byteBuffers.get(1));
parser.parse(byteBuffers.get(2));
parser.parse(byteBuffers.get(3));

assertEquals(1, frames.size());
HeadersFrame frame = frames.get(0);
assertEquals(streamId, frame.getStreamId());
assertTrue(frame.isEndStream());
MetaData.Request request = (MetaData.Request)frame.getMetaData();
assertEquals(metaData.getMethod(), request.getMethod());
assertEquals(metaData.getURIString(), request.getURIString());
for (int j = 0; j < fields.size(); ++j)
{
HttpField field = fields.getField(j);
assertTrue(request.getFields().contains(field));
}
PriorityFrame priority = frame.getPriority();
assertNull(priority);
assertEquals(beginNanoTime, request.getBeginNanoTime());
}

@Test
public void testLargeHeadersBlock() throws Exception
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package org.eclipse.jetty.http2.hpack;

import java.nio.ByteBuffer;
import java.util.function.LongSupplier;

import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader;
Expand Down Expand Up @@ -42,14 +43,18 @@ public class HpackDecoder
private final MetaDataBuilder _builder;
private final HuffmanDecoder _huffmanDecoder;
private final NBitIntegerDecoder _integerDecoder;
private final LongSupplier _beginNanoTimeSupplier;
private int _maxTableCapacity;

/**
* @param maxHeaderSize The maximum allowed size of a decoded headers block,
* expressed as total of all name and value bytes, plus 32 bytes per field
* @param beginNanoTimeSupplier The supplier of a nano timestamp taken at
* the time the first byte was read
*/
public HpackDecoder(int maxHeaderSize)
public HpackDecoder(int maxHeaderSize, LongSupplier beginNanoTimeSupplier)
{
_beginNanoTimeSupplier = beginNanoTimeSupplier;
_context = new HpackContext(HpackContext.DEFAULT_MAX_TABLE_CAPACITY);
_builder = new MetaDataBuilder(maxHeaderSize);
_huffmanDecoder = new HuffmanDecoder();
Expand Down Expand Up @@ -305,6 +310,7 @@ public MetaData decode(ByteBuffer buffer) throws HpackException.SessionException
}
}

_builder.setBeginNanoTime(_beginNanoTimeSupplier.getAsLong());
return _builder.build();
}

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.http2.hpack.HpackException.SessionException;
import org.eclipse.jetty.util.NanoTime;

public class MetaDataBuilder
{
Expand All @@ -38,6 +39,7 @@ public class MetaDataBuilder
private HpackException.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 @@ -60,6 +62,13 @@ public void setMaxSize(int maxSize)
_maxSize = maxSize;
}

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

/**
* Get the size.
*
Expand Down Expand Up @@ -248,10 +257,13 @@ public MetaData build() throws HpackException.StreamException
if (_path == null)
throw new HpackException.StreamException("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
Loading

0 comments on commit 8b6c920

Please sign in to comment.