Fix RangeTransform on stale-revalidate#13158
Open
masaori335 wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes incorrect Range response headers/body accounting when serving Range requests through the stale-revalidate (cache REPLACE) path and the origin’s fresh Content-Length differs from the stale cached object size.
Changes:
- Recompute range parsing/output length against the fresh origin
Content-Lengthduring stale revalidate (REPLACE + 200 OK) before installing the Range transform. - Fix RangeTransform input progress tracking by updating
m_write_vio.ndoneas bytes are skipped/sent. - Add an AuTest replay covering “shrunk” and “grown” origin bodies across stale revalidations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/gold_tests/headers/replays/range_transform.replay.yaml | New replay covering stale-revalidate RangeTransform correctness when origin size shrinks/grows. |
| tests/gold_tests/headers/range_transform.test.py | New test wrapper to run the replay. |
| src/proxy/Transform.cc | RangeTransform now increments m_write_vio.ndone when consuming input. |
| src/proxy/http/HttpSM.cc | Re-parse Range headers against fresh origin Content-Length on stale-revalidate REPLACE path. |
Comment on lines
+5116
to
+5118
| // Re-parse yielded e.g. RANGE_NOT_SATISFIABLE (entire range | ||
| // past fresh body); let downstream handling take over without | ||
| // installing the transform. |
Comment on lines
+5096
to
+5100
| // differs, re-parse the Range against the fresh value so the outgoing Content-Length/Content-Range match the body | ||
| // actually being sent. Without this, Content-Length/Content-Range advertise the stale cached size. | ||
| const int64_t fresh_cl = t_state.hdr_info.server_response.get_content_length(); | ||
| const int64_t cached_cl = t_state.cache_info.object_read ? t_state.cache_info.object_read->object_size_get() : -1; | ||
| if (fresh_cl > 0 && fresh_cl != cached_cl) { |
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
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
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.
Fix RangeTransform on the stale-revalidate path when the fresh origin body is differs from the stale object size.
For example, in the added AuTest, original object size was
64097and updated size was40000, the second (revalidated) response from ATS was below without this patch:while the actual response body size was 40000 bytes.
To fix the issue,
m_write_vio.ndoneinRangeTransform::transform_to_range().Content-Lengthfrom origin server.