Skip to content

fix(http): stale skip_bytes in cache-write consumer after 100 Continue with transform#12906

Open
JakeChampion wants to merge 1 commit intoapache:masterfrom
JakeChampion:jake/fix-master
Open

fix(http): stale skip_bytes in cache-write consumer after 100 Continue with transform#12906
JakeChampion wants to merge 1 commit intoapache:masterfrom
JakeChampion:jake/fix-master

Conversation

@JakeChampion
Copy link
Contributor

@JakeChampion JakeChampion commented Feb 23, 2026

When an origin sends 100 Continue before a compressible response, setup_100_continue_transfer() sets client_response_hdr_bytes to the intermediate header size. setup_server_transfer_to_transform() then creates the tunnel buffer with hdr_size=0 - body only, but does not reset client_response_hdr_bytes for non-chunked responses. perform_cache_write_action() passes the stale value as skip_bytes to the cache-write consumer, causing ink_release_assert in producer_run when the response body is smaller than the 100 Continue headers.

Fix: Set cache_write_skip to 0 when a transform is present, since setup_server_transfer_to_transform() never writes headers into the tunnel buffer.


Standard flow with no 100 Continue, no compress plugin transform:

ATS creates a tunnel buffer that contains [response headers][response body]
When writing to cache, it needs to skip past the headers so only the body gets stored
It uses a variable called skip_bytes for this
skip_bytes = size of response headers

What goes wrong with 100 Continue + compress transform:

1 - Origin sends 100 Continue:

Origin -> ATS: HTTP/1.1 100 Continue\r\n\r\n (25 bytes)
ATS forwards this to the client and records client_response_hdr_bytes = 25 (the size of those headers)
This is used later to know how much header data was written to the tunnel buffer

2 - Origin sends the real 200 OK response:
Because compress plugin is active, ATS sets up a server-to-transform tunnel
The buffer for this tunnel contains only the body, no headers go into this buffer at all, since the compress transform handles headers separately
client_response_hdr_bytes is not reset here for non-chunked responses, it is still 25 bytes from Step 1

3 - Cache write:
perform_cache_write_action() says wants to skip past the headers in the tunnel buffer before writing to cache and uses client_response_hdr_bytes as it's skip_bytes
In this scenario that means it tries to skip 25 bytes

But the tunnel buffer only has the body bytes in it, making the assertion fire and crash ATS

@JakeChampion JakeChampion force-pushed the jake/fix-master branch 2 times, most recently from bafc3a0 to ad3418f Compare February 23, 2026 16:59
@JakeChampion
Copy link
Contributor Author

I think AuTest 1of4 was a flakey fail

@JakeChampion JakeChampion marked this pull request as ready for review February 23, 2026 17:27
@bryancall bryancall modified the milestone: 11.0.0 Feb 23, 2026
@bryancall bryancall added the Bug label Feb 23, 2026
@ezelkow1
Copy link
Member

[approve ci autest 1]

Copy link
Contributor

Copilot AI left a comment

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 a crash in HttpTunnel::producer_run that occurs when an origin server sends a 100 Continue response before a compressible, non-chunked final response that triggers a transform (like the compress plugin with cache=true for untransformed caching).

The root cause is that setup_100_continue_transfer() sets client_response_hdr_bytes to the size of the 100 Continue headers. Later, when setup_server_transfer_to_transform() creates the tunnel buffer for the final response, it calls server_transfer_init(buf, 0) with hdr_size=0, meaning no headers are written to the buffer. However, for non-chunked responses, client_response_hdr_bytes is not reset. When perform_cache_write_action() passes this stale value as skip_bytes to the cache-write consumer, and the response body is smaller than the 100 Continue headers, the assertion skip_bytes <= read_avail() fails.

Changes:

  • Fixed the cache write skip_bytes calculation to account for transforms
  • Added comprehensive regression test with custom origin and client scripts

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/proxy/http/HttpSM.cc Fixed perform_cache_write_action() to set cache_write_skip=0 when transform is present, since setup_server_transfer_to_transform() creates buffers without headers
tests/gold_tests/pluginTest/compress/compress-cache-untransformed.test.py Regression test that reproduces the crash scenario with 100 Continue + compress transform + untransformed caching
tests/gold_tests/pluginTest/compress/compress_100_continue_origin.py Custom origin server that sends 100 Continue followed by a small compressible response
tests/gold_tests/pluginTest/compress/compress_100_continue_client.py Client that sends POST with Expect: 100-continue and Accept-Encoding: gzip
tests/gold_tests/pluginTest/compress/etc/cache-true-untransformed.config Compress plugin config with cache=false to enable untransformed caching only

…nue with transform

When an origin sends 100 Continue before a compressible response, `setup_100_continue_transfer()` sets `client_response_hdr_bytes` to the intermediate header size.
`setup_server_transfer_to_transform()` then creates the tunnel buffer with `hdr_size=0` - body only, but does not reset `client_response_hdr_bytes` for non-chunked responses.
`perform_cache_write_action()` passes the stale value as `skip_bytes` to the cache-write consumer, causing `ink_release_assert` in `producer_run` when the response body is smaller than the 100 Continue headers.

Fix: Set `cache_write_skip` to `0` when a transform is present, since `setup_server_transfer_to_transform()` never writes headers into the tunnel buffer.
@masaori335 masaori335 added the compress compress plugin label Mar 2, 2026
@masaori335
Copy link
Contributor

[approve ci autest]

return

# Send 100 Continue immediately.
conn.sendall(b'HTTP/1.1 100 Continue\r\n\r\n')
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding this test case. But I guess Proxy-Verifier supports 100 Continue support. ( we can merge this first and change it later )

@bneradt can we cover this case with proxy-verifier?

Copy link
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

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

Makes sense. Thank you!

@bryancall
Copy link
Contributor

[approve ci]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug compress compress plugin HTTP

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants