Skip to content

Fix H2 origin payload handling#13363

Open
bneradt wants to merge 1 commit into
apache:masterfrom
bneradt:h2-origin-payload-handling
Open

Fix H2 origin payload handling#13363
bneradt wants to merge 1 commit into
apache:masterfrom
bneradt:h2-origin-payload-handling

Conversation

@bneradt

@bneradt bneradt commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

This breaks out HTTP/2 work from this otherwise HTTP/3 focused PR:
#13213


HTTP/2 origin responses can legally carry a non-zero Content-Length on
responses that do not carry a payload, such as responses to HEAD
requests. ATS was validating these responses without retaining the
request method that had been sent on the outbound H2 stream, so it could
mistake a valid no-body response for a payload length error.

Outbound H2 DATA generation also needs to respect the transaction write
VIO byte count when a buffer reader has more bytes available than should
be sent on the stream. This caps the frame payload length to the
remaining write VIO bytes and still marks END_STREAM when that payload
finishes the transaction body.

This extends the H2 origin replay coverage with a HEAD response that has
a non-zero Content-Length and no body, plus a large PUT request and
response over an H2 client and H2 origin path.

Copilot AI review requested due to automatic review settings July 6, 2026 16:19
@bneradt bneradt added this to the 11.0.0 milestone Jul 6, 2026
@bneradt bneradt self-assigned this Jul 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes HTTP/2 origin-side edge cases in payload handling by preserving the outbound request method for response validation (e.g., HEAD with non-zero Content-Length but no body) and by capping outbound DATA frame payloads to the remaining write VIO byte count. It also extends the existing H2 origin replay gold tests to cover these regressions.

Changes:

  • Track the sent outbound request method on an H2 stream and use it when deciding whether Content-Length vs. received payload-length mismatches are valid for no-body responses.
  • Limit outbound H2 DATA frame payload length to the remaining write VIO bytes while still properly emitting END_STREAM when the transaction body is complete.
  • Extend H2 origin replay coverage (HEAD no-body w/ non-zero Content-Length, large PUT + response) and update expected metric outputs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/proxy/http2/Http2ConnectionState.cc Caps DATA frame payload by remaining write VIO bytes; records the outbound request method when sending request headers.
include/proxy/http2/Http2Stream.h Adds write_vio_ntodo() accessor; uses stored sent request method for payload-length validation logic.
tests/gold_tests/h2/replay_h2origin/h2-origin.yaml Adds new replay scenarios for HEAD (non-zero Content-Length, no body) and large PUT request/response.
tests/gold_tests/h2/h2origin.test.py Updates squid log expectations for the new UUID ranges.
tests/gold_tests/h2/gold/http-request-method-metrics.gold Updates the expected PUT request metric count to reflect the new PUT test.
Comments suppressed due to low confidence (1)

include/proxy/http2/Http2Stream.h:429

  • Typo in the Warning message key: "data_legnth" should be "data_length" to make the log output searchable/consistent.
  if (content_length != 0 && !is_payload_precluded && content_length != data_length) {
    Warning("Bad payload length content_length=%d data_legnth=%d session_id=%" PRId64, content_length,
            static_cast<int>(data_length), _proxy_ssn->connection_id());

Comment thread tests/gold_tests/h2/h2origin.test.py
HTTP/2 origin responses can legally carry a non-zero Content-Length on
responses that do not carry a payload, such as responses to HEAD
requests. ATS was validating these responses without retaining the
request method that had been sent on the outbound H2 stream, so it could
mistake a valid no-body response for a payload length error.

Outbound H2 DATA generation also needs to respect the transaction write
VIO byte count when a buffer reader has more bytes available than should
be sent on the stream. This caps the frame payload length to the
remaining write VIO bytes and still marks END_STREAM when that payload
finishes the transaction body.

This extends the H2 origin replay coverage with a HEAD response that has
a non-zero Content-Length and no body, plus a large PUT request and
response over an H2 client and H2 origin path.
@bneradt bneradt force-pushed the h2-origin-payload-handling branch from c99484e to 4d912fa Compare July 6, 2026 17:34
@bneradt

bneradt commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

[approve ci]

@bryancall bryancall requested a review from maskit July 6, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants