Customize HPACK header validation#121
Merged
Merged
Conversation
Baraujo25
reviewed
Jul 10, 2026
Collaborator
There was a problem hiding this comment.
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?
Comment on lines
+105
to
+142
| 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; | ||
| }); |
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?
Comment on lines
246
to
553
| HTTP2ClientProfileConfig profileConfig) { | ||
| loadProperties(profileConfig); | ||
| lowLevelDebug(PLUGIN_BUILD_TAG); | ||
|
|
||
| // Create buffer pool first (needed for both TCP and QUIC connectors) | ||
| this.bufferPool = new ArrayByteBufferPool(); | ||
| ensureDecoderFactoriesInitialized(); | ||
|
|
||
| ClientConnector clientConnector = createClientConnector(name); | ||
|
|
||
| // Configure SSL/TLS protocol | ||
| // In Jetty 12, ALPN protocols are automatically configured by HttpClientTransportDynamic | ||
| // based on the ClientConnectionFactory.Info instances provided (http2, http11, etc.) | ||
| // We only need to set the TLS protocol here | ||
| try { | ||
| SslContextFactory.Client sslContextFactoryFromConnector = | ||
| (SslContextFactory.Client) clientConnector.getSslContextFactory(); | ||
| if (sslContextFactoryFromConnector != null) { | ||
| sslContextFactoryFromConnector.setProtocol("TLS"); | ||
| lowLevelDebug("SSL Context Factory: protocol set to TLS"); | ||
| lowLevelDebug("ALPN protocols will be automatically configured by " | ||
| + "HttpClientTransportDynamic based on provided connection factories"); | ||
| } | ||
| } catch (Exception e) { | ||
| lowLevelDebug("Could not set SSL protocol explicitly", e); | ||
| } | ||
|
|
||
| ClientConnectionFactory.Info http11 = HttpClientConnectionFactory.HTTP11; | ||
|
|
||
| HTTP2Client http2Client = new HTTP2Client(clientConnector); | ||
| // HTTP2Client defaults to 8 KiB; HttpClient defaults to -1 (no local HPACK cap). Match | ||
| // HttpClient here so parsers are not created with 8192. On start(), Jetty configure() | ||
| // re-syncs from HttpClient.getMaxResponseHeadersSize(), so this stays dynamic if callers | ||
| // change HttpClient before start(). | ||
| http2Client.setMaxResponseHeadersSize(-1); | ||
| enableFrameLoggingIfConfigured(http2Client); | ||
|
|
||
| // Add session listener to log SETTINGS frames received from server (for debugging Issue #12071) | ||
| // This helps identify if the server sends a lower SETTINGS_MAX_HEADER_LIST_SIZE | ||
| try { | ||
| // Use reflection to add Session.Listener if available | ||
| Class<?> sessionListenerClass = Class.forName("org.eclipse.jetty.http2.api.Session$Listener"); | ||
|
|
||
| Object sessionListener = java.lang.reflect.Proxy.newProxyInstance( | ||
| sessionListenerClass.getClassLoader(), | ||
| new Class<?>[] {sessionListenerClass}, | ||
| (proxy, method, args) -> { | ||
| if ("onSettings".equals(method.getName()) && args.length >= 2) { | ||
| // Log SETTINGS frame received from server | ||
| Object settingsFrame = args[1]; | ||
| try { | ||
| // Try to get settings map from SettingsFrame | ||
| java.lang.reflect.Method getSettingsMethod = | ||
| settingsFrame.getClass().getMethod("getSettings"); | ||
| @SuppressWarnings("unchecked") | ||
| java.util.Map<Integer, Integer> settings = | ||
| (java.util.Map<Integer, Integer>) getSettingsMethod.invoke(settingsFrame); | ||
|
|
||
| if (settings != null) { | ||
| // SETTINGS_MAX_HEADER_LIST_SIZE = 0x6 | ||
| Integer maxHeaderListSize = settings.get(0x6); | ||
| if (maxHeaderListSize != null) { | ||
| lowLevelDebug("HTTP/2 SETTINGS frame received from server: " | ||
| + "SETTINGS_MAX_HEADER_LIST_SIZE={}", maxHeaderListSize); | ||
| if (maxHeaderListSize < settingsMaxHeaderListSize) { | ||
| LOG.warn("Server SETTINGS_MAX_HEADER_LIST_SIZE ({}) is lower than " | ||
| + "client setting ({}). This may cause protocol_error if headers " | ||
| + "exceed server limit (Issue #12071).", | ||
| maxHeaderListSize, settingsMaxHeaderListSize); | ||
| } | ||
| } | ||
| // Log other relevant SETTINGS | ||
| Integer maxFrameSize = settings.get(0x5); // SETTINGS_MAX_FRAME_SIZE | ||
| Integer initialWindowSize = settings.get(0x4); // INITIAL_WINDOW_SIZE | ||
| Integer maxConcurrentStreams = settings.get(0x3); // MAX_CONCURRENT_STREAMS | ||
|
|
||
| if (maxFrameSize != null) { | ||
| lowLevelDebug("HTTP/2 SETTINGS: SETTINGS_MAX_FRAME_SIZE={}", maxFrameSize); | ||
| } | ||
| if (initialWindowSize != null) { | ||
| lowLevelDebug("HTTP/2 SETTINGS: SETTINGS_INITIAL_WINDOW_SIZE={}", | ||
| initialWindowSize); | ||
| } | ||
| if (maxConcurrentStreams != null) { | ||
| lowLevelDebug("HTTP/2 SETTINGS: SETTINGS_MAX_CONCURRENT_STREAMS={}", | ||
| maxConcurrentStreams); | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| lowLevelDebug("Could not extract SETTINGS from frame", e); | ||
| } | ||
| } | ||
| return null; // Session.Listener methods return void | ||
| }); | ||
|
|
||
| // Add the listener to HTTP2Client | ||
| java.lang.reflect.Method addSessionListenerMethod = | ||
| http2Client.getClass().getMethod("addSessionListener", sessionListenerClass); | ||
| addSessionListenerMethod.invoke(http2Client, sessionListener); | ||
| lowLevelDebug("HTTP2Client: Session listener added to log SETTINGS frames from server"); | ||
| } catch (Exception e) { | ||
| lowLevelDebug("Could not add Session.Listener to HTTP2Client " | ||
| + "(may not be available in this Jetty version)", e); | ||
| } | ||
|
|
||
| CustomClientConnectionFactoryOverHTTP2.HTTP2 http2 = | ||
| new CustomClientConnectionFactoryOverHTTP2.HTTP2(http2Client); | ||
|
|
||
| // Configure server push (can be disabled for compatibility) | ||
| if (disableServerPush) { | ||
| http2Client.setMaxConcurrentPushedStreams(0); | ||
| lowLevelDebug("HTTP2Client: Server push disabled for compatibility"); | ||
| } else { | ||
| http2Client.setMaxConcurrentPushedStreams(maxConcurrentPushedStreams); | ||
| } | ||
| if (alpnEnabled) { | ||
| http2Client.setApplicationProtocols(Arrays.asList("h2", "http/1.1")); | ||
| } | ||
| http2Client.setUseALPN(alpnEnabled); | ||
|
|
||
| // Diagnostic toggle: skip custom HTTP/2 SETTINGS configuration. | ||
| boolean skipHttp2Settings = Boolean.getBoolean("blazemeter.http.skipHttp2Settings"); | ||
| if (skipHttp2Settings) { | ||
| lowLevelDebug("HTTP2Client: skipping custom SETTINGS configuration"); | ||
| } else { | ||
| // Configure HTTP/2 SETTINGS frame parameters | ||
| // These parameters are sent in the SETTINGS frame during HTTP/2 connection establishment | ||
| // Some servers may reject HTTP/2 if these values are not compatible | ||
| // Using reflection to access methods that may not be available in all Jetty versions | ||
| // Values can be configured via properties for specific server compatibility | ||
|
|
||
| // SETTINGS_INITIAL_WINDOW_SIZE: Initial window size for flow control | ||
| try { | ||
| if (hasMethod(http2Client.getClass(), "setInitialStreamWindowSize", int.class)) { | ||
| http2Client.getClass().getMethod("setInitialStreamWindowSize", int.class) | ||
| .invoke(http2Client, settingsInitialWindowSize); | ||
| lowLevelDebug("HTTP2Client: setInitialStreamWindowSize={} (SETTINGS_INITIAL_WINDOW_SIZE)", | ||
| settingsInitialWindowSize); | ||
| } | ||
| } catch (Exception e) { | ||
| lowLevelDebug("HTTP2Client: setInitialStreamWindowSize not available", e); | ||
| } | ||
|
|
||
| // SETTINGS_MAX_FRAME_SIZE: Maximum size of a frame | ||
| try { | ||
| if (hasMethod(http2Client.getClass(), "setMaxFrameSize", int.class)) { | ||
| http2Client.getClass().getMethod("setMaxFrameSize", int.class) | ||
| .invoke(http2Client, settingsMaxFrameSize); | ||
| lowLevelDebug("HTTP2Client: setMaxFrameSize={} (SETTINGS_MAX_FRAME_SIZE)", | ||
| settingsMaxFrameSize); | ||
| } | ||
| } catch (Exception e) { | ||
| lowLevelDebug("HTTP2Client: setMaxFrameSize not available", e); | ||
| } | ||
|
|
||
| // SETTINGS_MAX_CONCURRENT_STREAMS: Maximum number of concurrent streams | ||
| // Note: 0 means no limit (RFC 7540) | ||
| try { | ||
| if (hasMethod(http2Client.getClass(), "setMaxConcurrentStreams", int.class)) { | ||
| http2Client.getClass().getMethod("setMaxConcurrentStreams", int.class) | ||
| .invoke(http2Client, settingsMaxConcurrentStreams); | ||
| lowLevelDebug("HTTP2Client: setMaxConcurrentStreams={} (SETTINGS_MAX_CONCURRENT_STREAMS)", | ||
| settingsMaxConcurrentStreams); | ||
| } | ||
| } catch (Exception e) { | ||
| lowLevelDebug("HTTP2Client: setMaxConcurrentStreams not available", e); | ||
| } | ||
|
|
||
| // SETTINGS_MAX_HEADER_LIST_SIZE: Maximum size of header list | ||
| // Note: 0 means no limit (RFC 7540) | ||
| try { | ||
| if (hasMethod(http2Client.getClass(), "setMaxHeaderListSize", int.class)) { | ||
| http2Client.getClass().getMethod("setMaxHeaderListSize", int.class) | ||
| .invoke(http2Client, settingsMaxHeaderListSize); | ||
| lowLevelDebug("HTTP2Client: setMaxHeaderListSize={} (SETTINGS_MAX_HEADER_LIST_SIZE)", | ||
| settingsMaxHeaderListSize); | ||
| } | ||
| } catch (Exception e) { | ||
| lowLevelDebug("HTTP2Client: setMaxHeaderListSize not available", e); | ||
| } | ||
|
|
||
| // SETTINGS_HEADER_TABLE_SIZE: Maximum size of header compression table (HPACK) | ||
| try { | ||
| if (hasMethod(http2Client.getClass(), "setHeaderTableSize", int.class)) { | ||
| http2Client.getClass().getMethod("setHeaderTableSize", int.class) | ||
| .invoke(http2Client, settingsHeaderTableSize); | ||
| lowLevelDebug("HTTP2Client: setHeaderTableSize={} (SETTINGS_HEADER_TABLE_SIZE)", | ||
| settingsHeaderTableSize); | ||
| } | ||
| } catch (Exception e) { | ||
| lowLevelDebug("HTTP2Client: setHeaderTableSize not available", e); | ||
| } | ||
| } | ||
|
|
||
| lowLevelDebug("HTTP2Client configured: ALPN={}, maxConcurrentPushedStreams={}, " | ||
| + "http1UpgradeRequired={}", alpnEnabled, maxConcurrentPushedStreams, http1UpgradeRequired); | ||
| lowLevelDebug("HTTP2Client SETTINGS frame parameters configured " | ||
| + "(via reflection where available)"); | ||
| // Note: setProtocols() was removed in Jetty 12.1.5, protocols are configured via ALPN | ||
|
|
||
| // Configure HTTP/3 and QUIC (temporarily disabled for diagnostic runs) | ||
| ClientConnectionFactory.Info http3 = null; | ||
| if (!FORCE_HTTP2_ONLY && enableHttp3) { | ||
| try { | ||
| ClientConnector quicConnector = createClientConnector(name + "-quic"); | ||
| quicConnector.setIdleTimeout(Duration.ofMillis(quicMaxIdleTimeout)); | ||
|
|
||
| QuicheClientQuicConfiguration quicConfig = | ||
| HTTP3ClientQuicConfiguration.configure(new QuicheClientQuicConfiguration()); | ||
| HTTP3Client http3Client = new HTTP3Client(quicConfig, quicConnector); | ||
| http3Client.setUseALPN(true); | ||
|
|
||
| if (hasMethod(http3Client.getClass(), "setMaxConcurrentPushedStreams", int.class)) { | ||
| http3Client.getClass().getMethod("setMaxConcurrentPushedStreams", int.class) | ||
| .invoke(http3Client, maxConcurrentPushedStreams); | ||
| } | ||
|
|
||
| Transport quicTransport = new QuicheTransport(quicConfig); | ||
| http3 = new CustomClientConnectionFactoryOverHTTP3.HTTP3(http3Client, quicTransport); | ||
|
|
||
| lowLevelDebug("HTTP/3 and QUIC support enabled"); | ||
| } catch (Exception e) { | ||
| throw new IllegalStateException( | ||
| "Failed to initialize HTTP/3/QUIC support; dependencies must be available at runtime.", | ||
| e); | ||
| } | ||
| } else if (FORCE_HTTP2_ONLY) { | ||
| lowLevelDebug("HTTP/3 disabled (forced HTTP/2 only)"); | ||
| } else { | ||
| lowLevelDebug("HTTP/3 disabled (profile configuration)"); | ||
| } | ||
|
|
||
| // If ALPN could not negotiate HTTP2, it tries in the order of protocols indicated | ||
| // Include HTTP/3 if available | ||
| // NOTE: In Jetty 12.1.5, the order in HttpClientTransportDynamic affects ALPN negotiation. | ||
| // Some servers (Google, demoblaze.com, blazedemo.com) reject HTTP/2 frames from Jetty 12.1.5 | ||
| // even though ALPN negotiates HTTP/2 successfully. This is a regression from Jetty 11. | ||
| // We try HTTP/2 first, then fallback to HTTP/1.1 if needed. | ||
| ClientConnectionFactory.Info[] mainProtocols = buildMainProtocols(http3, http2, http11); | ||
| HttpClientTransport transport = new HttpClientTransportDynamic(clientConnector, mainProtocols); | ||
| mainProtocolsSnapshot = protocolList(mainProtocols); | ||
| lowLevelDebug("HttpClientTransportDynamic configured with protocols: {}", | ||
| mainProtocolsSnapshot); | ||
|
|
||
| configureTransport(transport); | ||
|
|
||
| this.httpClient = new HttpClient(transport); | ||
| configureHttpClient(this.httpClient, clientConnector); | ||
|
|
||
| if (FORCE_HTTP2_ONLY || !enableHttp3) { | ||
| this.httpClientNoH3 = this.httpClient; | ||
| } else { | ||
| ClientConnector noH3Connector = createClientConnector(name + "-noh3"); | ||
| ClientConnectionFactory.Info[] noH3Protocols = buildNoH3Protocols(http2, http11); | ||
| HttpClientTransport noH3Transport = | ||
| new HttpClientTransportDynamic(noH3Connector, noH3Protocols); | ||
| configureTransport(noH3Transport); | ||
| this.httpClientNoH3 = new HttpClient(noH3Transport); | ||
| configureHttpClient(this.httpClientNoH3, noH3Connector); | ||
| } | ||
| ClientConnector http1Connector = createClientConnector(name + "-http1"); | ||
| HttpClientTransport http1Transport = new HttpClientTransportDynamic(http1Connector, http11); | ||
| // HTTP/1.1 has no multiplexing (Jetty rejects a 2nd in-flight exchange per connection). | ||
| configureTransport(http1Transport, 1); | ||
| this.httpClientHttp1Only = new HttpClient(http1Transport); | ||
| configureHttpClient(this.httpClientHttp1Only, http1Connector); | ||
|
|
||
| ClientConnector h2cUpgradeConnector = createClientConnector(name + "-h2c-upgrade"); | ||
| HTTP2Client http2cUpgradeClient = new HTTP2Client(h2cUpgradeConnector); | ||
| http2cUpgradeClient.setMaxResponseHeadersSize(-1); | ||
| http2cUpgradeClient.setUseALPN(false); | ||
| if (disableServerPush) { | ||
| http2cUpgradeClient.setMaxConcurrentPushedStreams(0); | ||
| } else { | ||
| http2cUpgradeClient.setMaxConcurrentPushedStreams(maxConcurrentPushedStreams); | ||
| } | ||
| CustomClientConnectionFactoryOverHTTP2.HTTP2C http2cUpgrade = | ||
| new CustomClientConnectionFactoryOverHTTP2.HTTP2C(http2cUpgradeClient); | ||
| ClientConnectionFactory.Info[] h2cUpgradeProtocols = | ||
| buildH2cUpgradeProtocols(http11, http2cUpgrade); | ||
| HttpClientTransport h2cUpgradeTransport = | ||
| new HttpClientTransportDynamic(h2cUpgradeConnector, h2cUpgradeProtocols); | ||
| configureTransport(h2cUpgradeTransport); | ||
| this.httpClientH2cUpgrade = new HttpClient(h2cUpgradeTransport); | ||
| configureHttpClient(this.httpClientH2cUpgrade, h2cUpgradeConnector); | ||
|
|
||
| ClientConnector h2cConnector = createClientConnector(name + "-h2c"); | ||
| HTTP2Client http2cClient = new HTTP2Client(h2cConnector); | ||
| http2cClient.setMaxResponseHeadersSize(-1); | ||
| http2cClient.setUseALPN(false); | ||
| if (disableServerPush) { | ||
| http2cClient.setMaxConcurrentPushedStreams(0); | ||
| } else { | ||
| http2cClient.setMaxConcurrentPushedStreams(maxConcurrentPushedStreams); | ||
| } | ||
| HttpClientTransport h2cTransport = new HttpClientTransportOverHTTP2(http2cClient); | ||
| HttpClientTransport h2cTransport = new CustomHttpClientTransportOverHTTP2(http2cClient); | ||
| configureTransport(h2cTransport); | ||
| this.httpClientH2cPrior = new HttpClient(h2cTransport); | ||
| configureHttpClient(this.httpClientH2cPrior, h2cConnector); | ||
| this.http1UpgradeRequired = http1UpgradeRequired; | ||
| this.httpClient.setName(name); | ||
| if (httpClientNoH3 != httpClient) { | ||
| this.httpClientNoH3.setName(name + "-noh3"); | ||
| } | ||
| this.httpClientHttp1Only.setName(name + "-http1"); | ||
| this.httpClientH2cPrior.setName(name + "-h2c"); | ||
| this.httpClientH2cUpgrade.setName(name + "-h2c-upgrade"); | ||
| } |
Collaborator
There was a problem hiding this comment.
I know that this is not an addition in this PR but we might have to think of ways to avoid this type of initializations, what do you think? It's a +300L with tons of logic inside a instance constructor. I believe that we could start thinking on a Strategy for protocol/behavior variation, Factory/Builder for wiring Jetty objects and maybe a Decorator if we want to wrap the standard behavior with tolerance/custom parsing
| } | ||
|
|
||
| @Override | ||
| public MetaData decode(ByteBuffer buffer) |
Collaborator
There was a problem hiding this comment.
This works, but decode(...) is getting very hard to follow as a single method. It currently mixes HPACK state-machine parsing, header normalization, validation, field construction, dynamic table updates, and error handling all in one place, which makes the behavior difficult to reason about and risky to modify later.
Would you consider splitting it into a few smaller steps/helpers? For example: decode indexed field vs literal field, decode name/value, normalize + validate, build HttpField, and update the dynamic table. Even if the wire-format flow stays the same, giving those responsibilities names would make the method much easier to review and maintain.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This pull request introduces a set of changes to improve HTTP/2 header handling and compatibility in the Jetty-based HTTP/2 client, inspired by Firefox's more permissive behavior. The main focus is on softening header validation to allow for better load-testing robustness, updating dependencies, and integrating custom connection and parser classes for HTTP/2.
HTTP/2 Header Handling Improvements
CustomHttp2HeaderNormalizerto provide Firefox-inspired soft header normalization, allowing the client to recover or log and trim illegal header values instead of failing outright. This helps with load-testing scenarios where some servers send non-strict headers.CustomParser, which installs a custom HPACK decoder (CustomHpackDecoder) via reflection, enabling the use of the new header normalization logic during header parsing.Custom Jetty HTTP/2 Integration
CustomHTTP2ClientConnectionFactoryas a replacement for Jetty's default connection factory, ensuring the custom parser and header handling are used for all HTTP/2 connections.CustomClientConnectionFactoryOverHTTP2to use the newCustomHTTP2ClientConnectionFactoryinstead of Jetty's default, ensuring the custom logic is integrated into the connection pipeline.CustomHttpClientTransportOverHTTP2to wrap the new connection factory, and updatedHTTP2JettyClientto use this transport for HTTP/2 connections. [1] [2] [3] [4]Dependency Updates
pom.xmlto match the customizations and ensure compatibility with the latest bug fixes and features.