Customize HPACK header validation #121
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| package com.blazemeter.jmeter.http2.core.jetty.custom.http2; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import org.eclipse.jetty.http2.FlowControlStrategy; | ||
| import org.eclipse.jetty.http2.HTTP2Connection; | ||
| import org.eclipse.jetty.http2.HTTP2Session; | ||
| import org.eclipse.jetty.http2.api.Session; | ||
| import org.eclipse.jetty.http2.client.CustomHttp2SessionContainerAccessor; | ||
| import org.eclipse.jetty.http2.client.HTTP2Client; | ||
| import org.eclipse.jetty.http2.client.internal.HTTP2ClientSession; | ||
| import org.eclipse.jetty.http2.frames.Frame; | ||
| import org.eclipse.jetty.http2.frames.PrefaceFrame; | ||
| import org.eclipse.jetty.http2.frames.SettingsFrame; | ||
| import org.eclipse.jetty.http2.frames.WindowUpdateFrame; | ||
| import org.eclipse.jetty.http2.generator.Generator; | ||
| import org.eclipse.jetty.http2.hpack.HpackContext; | ||
| import org.eclipse.jetty.io.ByteBufferPool; | ||
| import org.eclipse.jetty.io.ClientConnectionFactory; | ||
| import org.eclipse.jetty.io.Connection; | ||
| import org.eclipse.jetty.io.EndPoint; | ||
| import org.eclipse.jetty.util.Callback; | ||
| import org.eclipse.jetty.util.Promise; | ||
|
|
||
| /** | ||
| * Jetty 12.1.11 {@code HTTP2ClientConnectionFactory} variant that installs | ||
| * {@link CustomParser} and {@link org.eclipse.jetty.http2.hpack.CustomHpackDecoder} | ||
| * for Firefox-inspired soft header handling. | ||
| */ | ||
| public class CustomHTTP2ClientConnectionFactory implements ClientConnectionFactory { | ||
|
|
||
| @Override | ||
| public Connection newConnection(EndPoint endPoint, Map<String, Object> context) { | ||
| HTTP2Client client = (HTTP2Client) context.get(HTTP2Client.CONTEXT_KEY); | ||
| ByteBufferPool bufferPool = client.getByteBufferPool(); | ||
| Session.Listener listener = | ||
| (Session.Listener) context.get(HTTP2Client.SESSION_LISTENER_CONTEXT_KEY); | ||
| @SuppressWarnings("unchecked") | ||
| Promise<Session> sessionPromise = | ||
| (Promise<Session>) context.get(HTTP2Client.SESSION_PROMISE_CONTEXT_KEY); | ||
|
|
||
| Generator generator = new Generator(bufferPool, client.isUseOutputDirectByteBuffers(), | ||
| client.getMaxHeaderBlockFragment()); | ||
| generator.getHpackEncoder().setMaxHeaderListSize(client.getMaxRequestHeadersSize()); | ||
|
|
||
| FlowControlStrategy flowControl = client.getFlowControlStrategyFactory() | ||
| .newFlowControlStrategy(); | ||
|
|
||
| CustomParser parser = new CustomParser(bufferPool, client.getMaxResponseHeadersSize()); | ||
| parser.setMaxSettingsKeys(client.getMaxSettingsKeys()); | ||
|
|
||
| HTTP2ClientSession session = new HTTP2ClientSession(client.getScheduler(), endPoint, parser, | ||
| generator, listener, flowControl); | ||
| session.setMaxLocalStreams(client.getMaxLocalStreams()); | ||
| session.setMaxRemoteStreams(client.getMaxConcurrentPushedStreams()); | ||
| session.setMaxEncoderTableCapacity(client.getMaxEncoderTableCapacity()); | ||
| long streamIdleTimeout = client.getStreamIdleTimeout(); | ||
| if (streamIdleTimeout > 0) { | ||
| session.setStreamIdleTimeout(streamIdleTimeout); | ||
| } | ||
|
|
||
| HTTP2ClientConnection connection = | ||
| new HTTP2ClientConnection(client, endPoint, session, sessionPromise, listener); | ||
| context.put(HTTP2Connection.class.getName(), connection); | ||
| connection.addEventListener(CustomHttp2SessionContainerAccessor.getSessionContainer(client)); | ||
| client.getEventListeners().forEach(session::addEventListener); | ||
| parser.init(connection); | ||
|
|
||
| return customize(connection, context); | ||
| } | ||
|
|
||
| private static class HTTP2ClientConnection extends HTTP2Connection implements Callback { | ||
| private final HTTP2Client client; | ||
| private final Promise<Session> promise; | ||
| private final Session.Listener listener; | ||
|
|
||
| private HTTP2ClientConnection(HTTP2Client client, EndPoint endpoint, | ||
| HTTP2ClientSession session, Promise<Session> sessionPromise, | ||
| Session.Listener listener) { | ||
| super(client.getByteBufferPool(), client.getExecutor(), endpoint, session, | ||
| client.getInputBufferSize(), -1); | ||
| this.client = client; | ||
| this.promise = sessionPromise; | ||
| this.listener = listener; | ||
| setUseInputDirectByteBuffers(client.isUseInputDirectByteBuffers()); | ||
| setUseOutputDirectByteBuffers(client.isUseOutputDirectByteBuffers()); | ||
| } | ||
|
|
||
| @Override | ||
| public void onOpen() { | ||
| HTTP2Session session = getSession(); | ||
| session.notifyLifeCycleOpen(); | ||
|
|
||
| Map<Integer, Integer> settings = listener.onPreface(session); | ||
| settings = settings == null ? new HashMap<>() : new HashMap<>(settings); | ||
|
|
||
| settings.compute(SettingsFrame.HEADER_TABLE_SIZE, (k, v) -> { | ||
| if (v == null) { | ||
| v = client.getMaxDecoderTableCapacity(); | ||
| if (v == HpackContext.DEFAULT_MAX_TABLE_CAPACITY) { | ||
| v = null; | ||
| } | ||
| } | ||
| return v; | ||
| }); | ||
| settings.computeIfAbsent(SettingsFrame.MAX_CONCURRENT_STREAMS, | ||
| k -> client.getMaxConcurrentPushedStreams()); | ||
| settings.compute(SettingsFrame.INITIAL_WINDOW_SIZE, (k, v) -> { | ||
| if (v == null) { | ||
| v = client.getInitialStreamRecvWindow(); | ||
| if (v == FlowControlStrategy.DEFAULT_WINDOW_SIZE) { | ||
| v = null; | ||
| } | ||
| } | ||
| return v; | ||
| }); | ||
| settings.compute(SettingsFrame.MAX_FRAME_SIZE, (k, v) -> { | ||
| if (v == null) { | ||
| v = client.getMaxFrameSize(); | ||
| if (v == Frame.DEFAULT_MAX_SIZE) { | ||
| v = null; | ||
| } | ||
| } | ||
| return v; | ||
| }); | ||
| settings.compute(SettingsFrame.MAX_HEADER_LIST_SIZE, (k, v) -> { | ||
| if (v == null) { | ||
| v = client.getMaxResponseHeadersSize(); | ||
| if (v <= 0) { | ||
| v = null; | ||
| } | ||
| } | ||
| return v; | ||
| }); | ||
|
Comment on lines
+98
to
+135
Collaborator
There was a problem hiding this comment. What do you think about modulizing these ones as well? I think that we could benefit of some nice function names in roder to better understand what are these compute doing rather than slowing down to read the actual implementation? |
||
|
|
||
| PrefaceFrame prefaceFrame = new PrefaceFrame(); | ||
| SettingsFrame settingsFrame = new SettingsFrame(settings, false); | ||
|
|
||
| int windowDelta = | ||
| client.getInitialSessionRecvWindow() - FlowControlStrategy.DEFAULT_WINDOW_SIZE; | ||
| session.updateRecvWindow(windowDelta); | ||
| if (windowDelta > 0) { | ||
| session.frames(null, | ||
| List.of(prefaceFrame, settingsFrame, new WindowUpdateFrame(0, windowDelta)), this); | ||
| } else { | ||
| session.frames(null, List.of(prefaceFrame, settingsFrame), this); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void succeeded() { | ||
| super.onOpen(); | ||
| promise.succeeded(getSession()); | ||
| produce(); | ||
| } | ||
|
|
||
| @Override | ||
| public void failed(Throwable ex) { | ||
| close(); | ||
| promise.failed(ex); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| package com.blazemeter.jmeter.http2.core.jetty.custom.http2; | ||
|
|
||
| import org.eclipse.jetty.http.HttpHeader; | ||
| import org.eclipse.jetty.http.HttpTokens; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| /** | ||
| * Firefox-inspired HTTP/2 header softening for load-testing clients. | ||
| * | ||
| * <p>Browsers (especially Chrome) reject non-numeric {@code :status} values. This helper is more | ||
| * permissive: it extracts a leading status code (Chrome HTTP/1 {@code ParseStatus} style) and | ||
| * trims soft-illegal field values so sampling can continue while logging the anomaly. | ||
| */ | ||
| public final class CustomHttp2HeaderNormalizer { | ||
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(CustomHttp2HeaderNormalizer.class); | ||
|
|
||
| private CustomHttp2HeaderNormalizer() { | ||
| } | ||
|
|
||
| /** | ||
| * Normalizes a decoded header value before Jetty builds/emits the field. | ||
| * | ||
| * @return the value to use (possibly unchanged) | ||
| */ | ||
| public static String normalize(HttpHeader header, String name, String value) { | ||
| if (value == null) { | ||
| return null; | ||
| } | ||
| if (isStatusPseudoHeader(header, name)) { | ||
| return normalizeStatus(value); | ||
| } | ||
| return softenFieldValue(name, value); | ||
| } | ||
|
|
||
| static boolean isStatusPseudoHeader(HttpHeader header, String name) { | ||
| if (header == HttpHeader.C_STATUS) { | ||
| return true; | ||
| } | ||
| return name != null && ":status".equalsIgnoreCase(name); | ||
| } | ||
|
|
||
| /** | ||
| * Accepts {@code 299}, {@code 299 Akamai}, or leading spaces before digits. | ||
| * Returns the original value when no leading status code can be recovered. | ||
| */ | ||
| static String normalizeStatus(String value) { | ||
| String trimmedLeading = trimLeadingSpaces(value); | ||
| int digits = 0; | ||
| while (digits < trimmedLeading.length() | ||
| && trimmedLeading.charAt(digits) >= '0' | ||
| && trimmedLeading.charAt(digits) <= '9') { | ||
| digits++; | ||
| } | ||
| if (digits == 0) { | ||
| return value; | ||
| } | ||
| String code = trimmedLeading.substring(0, digits); | ||
| if (code.equals(value)) { | ||
| return value; | ||
| } | ||
| String reason = trimmedLeading.substring(digits).trim(); | ||
| if (reason.isEmpty() && code.equals(trimmedLeading)) { | ||
| if (!code.equals(value)) { | ||
| LOG.warn("Normalized HTTP/2 :status from [{}] to [{}]", value, code); | ||
| } | ||
| return code; | ||
| } | ||
| LOG.warn("Normalized HTTP/2 :status from [{}] to [{}] (ignored reason [{}])", | ||
| value, code, reason); | ||
| return code; | ||
| } | ||
|
|
||
| /** | ||
| * Softens values that fail only because of leading/trailing SP/HTAB (RFC 9113). | ||
| * Control characters and other illegal octets are left untouched so Jetty can still reject them. | ||
| */ | ||
| static String softenFieldValue(String name, String value) { | ||
| if (HttpTokens.isLegalFieldValue(value)) { | ||
| return value; | ||
| } | ||
| String trimmed = trimHttpWhitespace(value); | ||
| if (trimmed.equals(value)) { | ||
| return value; | ||
| } | ||
| if (HttpTokens.isLegalFieldValue(trimmed)) { | ||
| LOG.warn("Trimmed illegal HTTP/2 header value for [{}]: [{}] -> [{}]", | ||
| name, value, trimmed); | ||
| return trimmed; | ||
| } | ||
| return value; | ||
| } | ||
|
|
||
| private static String trimLeadingSpaces(String value) { | ||
| int i = 0; | ||
| while (i < value.length() && value.charAt(i) == ' ') { | ||
| i++; | ||
| } | ||
| return i == 0 ? value : value.substring(i); | ||
| } | ||
|
|
||
| private static String trimHttpWhitespace(String value) { | ||
| int start = 0; | ||
| int end = value.length(); | ||
| while (start < end && isHttpWhitespace(value.charAt(start))) { | ||
| start++; | ||
| } | ||
| while (end > start && isHttpWhitespace(value.charAt(end - 1))) { | ||
| end--; | ||
| } | ||
| return (start == 0 && end == value.length()) ? value : value.substring(start, end); | ||
| } | ||
|
|
||
| private static boolean isHttpWhitespace(char c) { | ||
| return c == ' ' || c == '\t'; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package com.blazemeter.jmeter.http2.core.jetty.custom.http2; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Map; | ||
| import org.eclipse.jetty.http2.client.HTTP2Client; | ||
| import org.eclipse.jetty.http2.client.transport.HttpClientTransportOverHTTP2; | ||
| import org.eclipse.jetty.io.Connection; | ||
| import org.eclipse.jetty.io.EndPoint; | ||
|
|
||
| /** | ||
| * Custom HTTP/2 client transport that uses {@link CustomHTTP2ClientConnectionFactory}. | ||
| */ | ||
| public class CustomHttpClientTransportOverHTTP2 extends HttpClientTransportOverHTTP2 { | ||
|
|
||
| private final CustomHTTP2ClientConnectionFactory connectionFactory = | ||
| new CustomHTTP2ClientConnectionFactory(); | ||
|
|
||
| public CustomHttpClientTransportOverHTTP2(HTTP2Client http2Client) { | ||
| super(http2Client); | ||
| } | ||
|
|
||
| @Override | ||
| public Connection newConnection(EndPoint endPoint, Map<String, Object> context) | ||
| throws IOException { | ||
| return connectionFactory.newConnection(endPoint, context); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about modularizing this a bit? Maybe initialization and configuration of required instances could be managed separately so we can have a quick overview of what is needed for a
newConnection?