Log malformed HTTP/2 requests by bneradt · Pull Request #13059 · apache/trafficserver

bneradt · 2026-04-04T17:44:42Z

Unlike HTTP/1 transactions, malformed HTTP/2 requests are rejected before HttpSM creation, so they bypassed the normal transaction logging path. That left malformed h2 traffic out of squid.log even when similar h1 failures were visible.

This adds a pre-transaction LogAccess path for malformed h2 request headers and emits a best-effort access log entry before resetting the stream.

Copilot

Pull request overview

This PR adds a “pre-transaction” access logging path for malformed HTTP/2 request headers that are rejected before HttpSM is created, so these failures can still be recorded in squid.log (similar to HTTP/1 behavior).

Changes:

Add LogAccess::PreTransactionLogData and extend LogAccess to support access log marshaling without an HttpSM.
Emit a best-effort access log entry from the HTTP/2 layer when rejecting malformed HEADERS/CONTINUATION input.
Add gold and unit tests validating both malformed and valid HTTP/2 request logging.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/gold_tests/connect/replays/h2_malformed_request_logging.replay.yaml	Adds replay traffic for valid HTTP/2 GET and CONNECT cases used by the gold test.
tests/gold_tests/connect/malformed_h2_request_client.py	Adds a low-level client to send deliberately malformed HTTP/2 requests on the wire.
tests/gold_tests/connect/h2_malformed_request_logging.test.py	Adds an integration test that asserts malformed HTTP/2 requests are access logged and do not reach origin.
src/proxy/logging/unit-tests/test_LogAccess.cc	Adds Catch2 unit tests covering pre-transaction `LogAccess` marshaling behavior.
src/proxy/logging/LogAccess.cc	Implements pre-transaction initialization and adds guards/fallbacks for marshaling without `HttpSM`.
src/proxy/logging/CMakeLists.txt	Registers the new `test_LogAccess` executable in CMake when testing is enabled.
src/proxy/http2/Http2ConnectionState.cc	Logs malformed decoded request headers via `Log::access()` before stream reset/GOAWAY.
include/proxy/logging/LogAccess.h	Defines `PreTransactionLogData` and adds support helpers for pre-transaction logging mode.
include/proxy/http2/Http2Stream.h	Exposes a const accessor for the decoded receive header (`get_receive_header()`).

maskit

It's good that this narrows the gap between H1 and H2 in terms of logging.

Suggestions for future improvement:

I'm pretty sure H3 has the same issue. It would be nice if ProxyTransaction could take care of this pre-transaction scenario .
A cleaner interface would be having just LogAccess(LogData *data) for both. The subclasses of LogData would be TransactionLogData that copies data from HttpSM, and PreTransactionLogData that copies data from ProxyTransaction (or Http2Stream). The subclasses would be defined in http or http2 module. Then we may be able to remove the the circular dependency (http <-> logging).

bneradt · 2026-04-08T22:28:59Z

Converting to a draft while I work on @maskit 's comments. I'll un-draft when the patch is in better shape.

Copilot

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

maskit

The copies are concerning. Otherwise, looks good.

Unlike HTTP/1 transactions, malformed HTTP/2 requests are rejected before HttpSM creation, so they bypassed the normal transaction logging path. That left malformed h2 traffic out of squid.log even when similar h1 failures were visible. This adds a pre-transaction LogAccess path for malformed h2 request headers and emits a best-effort access log entry before resetting the stream.

bneradt · 2026-04-20T18:24:19Z

10.2.x backport PR: #13105

Unlike HTTP/1 transactions, malformed HTTP/2 requests are rejected before HttpSM creation, so they bypassed the normal transaction logging path. That left malformed h2 traffic out of squid.log even when similar h1 failures were visible. This adds a pre-transaction LogAccess path for malformed h2 request headers and emits a best-effort access log entry before resetting the stream. (cherry picked from commit 05554ca)

cmcfarlen · 2026-04-21T20:57:17Z

Added to milestone 10.2.0 via #13105

This addresses a performance regression added by apache#13059. Malformed pre-transaction logging introduced an extra virtual data interface on the normal access log path. That made every completed transaction pay for indirection that is only needed for rare protocol-layer failures. This replaces the virtual hierarchy with a concrete composed TransactionLogData wrapper. This keeps LogAccess using one data object while routing the common HttpSM path through direct getters and falling back to owned pre-transaction data only when no HttpSM exists.

This addresses a performance regression added by #13059. Malformed pre-transaction logging introduced an extra virtual data interface on the normal access log path. That made every completed transaction pay for indirection that is only needed for rare protocol-layer failures. This replaces the virtual hierarchy with a concrete composed TransactionLogData wrapper. This keeps LogAccess using one data object while routing the common HttpSM path through direct getters and falling back to owned pre-transaction data only when no HttpSM exists. (cherry picked from commit 05a916f)

This addresses a performance regression added by #13059. Malformed pre-transaction logging introduced an extra virtual data interface on the normal access log path. That made every completed transaction pay for indirection that is only needed for rare protocol-layer failures. This replaces the virtual hierarchy with a concrete composed TransactionLogData wrapper. This keeps LogAccess using one data object while routing the common HttpSM path through direct getters and falling back to owned pre-transaction data only when no HttpSM exists.

m_http_sm was removed from LogAccess by #13059 (TransactionLogData).

m_http_sm was removed from LogAccess by #13059 (TransactionLogData). (cherry picked from commit bc4bce3)

Malformed HTTP/2 parse errors can hit this path during normal bad-client traffic, and logging each one at ERROR keeps diags noisy. PR #13059 now emits transaction log entries for these malformed requests, so operators can diagnose the rejected request without this default error log noise. This downgrades the stream creation failure message to the existing HTTP/2 session debug path. Operators can still enable the http2_cs debug tag when they need the protocol-level detail.

Malformed client HTTP/2 streams can produce noisy error-level diagnostics even though the malformed parse details are now available in transaction logs via apache#13059. The original downgrade targeted the outbound session stream creation path, which can hide origin-side signals such as concurrent stream limit failures. This downgrades the inbound rcv_frame stream-error diagnostic to Http2StreamDebug while leaving outbound stream creation errors at Error level. This also updates the malformed request AuTest to look for the debug diagnostic in traffic.out.

Malformed client HTTP/2 streams can produce noisy error-level diagnostics even though the malformed parse details are now available in transaction logs via apache#13059. This downgrades the inbound rcv_frame stream-error diagnostic to Http2StreamDebug while leaving outbound stream creation errors at Error level. This also updates the malformed request AuTest to look for the debug diagnostic in traffic.out and relies on Http2StreamDebug to include the session and stream identifiers.

Malformed client HTTP/2 streams can produce noisy error-level diagnostics even though the malformed parse details are now available in transaction logs via #13059. This downgrades the inbound rcv_frame stream-error diagnostic to Http2StreamDebug while leaving outbound stream creation errors at Error level. This also updates the malformed request AuTest to look for the debug diagnostic in traffic.out and relies on Http2StreamDebug to include the session and stream identifiers.

This addresses a performance regression added by apache#13059. Malformed pre-transaction logging introduced an extra virtual data interface on the normal access log path. That made every completed transaction pay for indirection that is only needed for rare protocol-layer failures. This replaces the virtual hierarchy with a concrete composed TransactionLogData wrapper. This keeps LogAccess using one data object while routing the common HttpSM path through direct getters and falling back to owned pre-transaction data only when no HttpSM exists.

m_http_sm was removed from LogAccess by apache#13059 (TransactionLogData).

Malformed HTTP/2 parse errors can hit this path during normal bad-client traffic, and logging each one at ERROR keeps diags noisy. PR apache#13059 now emits transaction log entries for these malformed requests, so operators can diagnose the rejected request without this default error log noise. This downgrades the stream creation failure message to the existing HTTP/2 session debug path. Operators can still enable the http2_cs debug tag when they need the protocol-level detail.

Malformed client HTTP/2 streams can produce noisy error-level diagnostics even though the malformed parse details are now available in transaction logs via apache#13059. This downgrades the inbound rcv_frame stream-error diagnostic to Http2StreamDebug while leaving outbound stream creation errors at Error level. This also updates the malformed request AuTest to look for the debug diagnostic in traffic.out and relies on Http2StreamDebug to include the session and stream identifiers.

Malformed client HTTP/2 streams can produce noisy error-level diagnostics even though the malformed parse details are now available in transaction logs via apache#13059. This downgrades the inbound rcv_frame stream-error diagnostic to Http2StreamDebug while leaving outbound stream creation errors at Error level. This also updates the malformed request AuTest to look for the debug diagnostic in traffic.out and relies on Http2StreamDebug to include the session and stream identifiers. (cherry picked from commit 87cf5ec)

bneradt added this to the 11.0.0 milestone Apr 4, 2026

bneradt requested review from Copilot and maskit April 4, 2026 17:44

bneradt self-assigned this Apr 4, 2026

bneradt added HTTP/2 Logging labels Apr 4, 2026

bneradt added this to ATS v10.2.x Apr 4, 2026

Copilot started reviewing on behalf of bneradt April 4, 2026 17:45 View session

Copilot AI reviewed Apr 4, 2026

View reviewed changes

Comment thread tests/gold_tests/connect/replays/h2_malformed_request_logging.replay.yaml Outdated

Comment thread tests/gold_tests/connect/h2_malformed_request_logging.test.py

Comment thread src/proxy/http2/Http2ConnectionState.cc Outdated

Comment thread src/proxy/logging/LogAccess.cc Outdated

bneradt force-pushed the h2-malformed-request-txn-logging branch 2 times, most recently from 93a7fba to fa9178b Compare April 4, 2026 23:19

maskit previously approved these changes Apr 6, 2026

View reviewed changes

bneradt dismissed maskit’s stale review via 03e7c09 April 8, 2026 22:14

bneradt force-pushed the h2-malformed-request-txn-logging branch from fa9178b to 03e7c09 Compare April 8, 2026 22:14

bneradt marked this pull request as draft April 8, 2026 22:28

bneradt force-pushed the h2-malformed-request-txn-logging branch from 03e7c09 to 9ef2a82 Compare April 9, 2026 21:48

bneradt marked this pull request as ready for review April 9, 2026 21:49

bneradt requested a review from Copilot April 9, 2026 21:49

Copilot started reviewing on behalf of bneradt April 9, 2026 21:49 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Comment thread src/proxy/ProxyTransaction.cc Outdated

Comment thread src/proxy/ProxyTransaction.cc Outdated

bneradt force-pushed the h2-malformed-request-txn-logging branch 2 times, most recently from 45d1858 to 9801fc9 Compare April 9, 2026 23:04

maskit reviewed Apr 10, 2026

View reviewed changes

Comment thread src/proxy/logging/CMakeLists.txt Outdated

Comment thread src/proxy/http/TransactionLogData.cc Outdated

bneradt force-pushed the h2-malformed-request-txn-logging branch 3 times, most recently from 4613682 to d5ceca7 Compare April 11, 2026 00:37

bneradt requested a review from maskit April 11, 2026 00:39

bneradt force-pushed the h2-malformed-request-txn-logging branch from d5ceca7 to c374c0c Compare April 11, 2026 03:52

bneradt deleted the h2-malformed-request-txn-logging branch April 13, 2026 22:18

github-project-automation Bot moved this to For v10.2.0 in ATS v10.2.x Apr 13, 2026

bneradt mentioned this pull request Apr 20, 2026

10.2.x: Log malformed HTTP/2 requests #13105

Merged

bneradt removed this from ATS v10.2.x Apr 20, 2026

cmcfarlen removed this from the 11.0.0 milestone Apr 21, 2026

This was referenced Apr 28, 2026

Remove virtual dispatch from LogData #13123

Merged

10.2: Remove virtual dispatch from LogData #13124

Merged

zwoop mentioned this pull request Jun 5, 2026

Fix build: TLS log fields use m_data, not m_http_sm #13241

Merged

zwoop added a commit that referenced this pull request Jun 5, 2026

Fix build: TLS log fields use m_data, not m_http_sm (#13241)

bc4bce3

m_http_sm was removed from LogAccess by #13059 (TransactionLogData).

cmcfarlen pushed a commit that referenced this pull request Jun 16, 2026

Fix build: TLS log fields use m_data, not m_http_sm (#13241)

3955d91

m_http_sm was removed from LogAccess by #13059 (TransactionLogData). (cherry picked from commit bc4bce3)

bneradt mentioned this pull request Jun 18, 2026

Downgrade H2 stream error log #13298

Merged

bneradt mentioned this pull request Jun 23, 2026

Downgrade inbound H2 stream error log #13316

Merged

zwoop added this to the Backported milestone Jul 20, 2026

cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jul 29, 2026

Fix build: TLS log fields use m_data, not m_http_sm (apache#13241)

d65d667

m_http_sm was removed from LogAccess by apache#13059 (TransactionLogData).

Navigation Menu

Uh oh!

Log malformed HTTP/2 requests - #13059

Log malformed HTTP/2 requests#13059
bneradt merged 1 commit into
apache:masterfrom
bneradt:h2-malformed-request-txn-logging

bneradt commented Apr 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maskit left a comment

Uh oh!

bneradt commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

maskit left a comment

Uh oh!

Uh oh!

Uh oh!

bneradt commented Apr 20, 2026

Uh oh!

cmcfarlen commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

bneradt commented Apr 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maskit left a comment

Choose a reason for hiding this comment

Uh oh!

bneradt commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

maskit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bneradt commented Apr 20, 2026

Uh oh!

cmcfarlen commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants