Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The handle_cache_operation_on_forward_server_response function doesn't handle proper range operations for mismatched server and cache responses #10772

Open
vmamidi opened this issue Nov 13, 2023 · 3 comments
Assignees

Comments

@vmamidi
Copy link
Contributor

vmamidi commented Nov 13, 2023

handle_cache_operation_on_forward_server_response sets RANGE_NOT_TRANSFORM_REQUESTED even if the server returns a response that doesn't match with the cached response. This causes ATS to send incorrect client responses.

For example, while revalidating the cached 200 response, if ATS receives a 404 response, ATS transforms the 404 response to a 206 response and sends it to the client

@vmamidi vmamidi self-assigned this Nov 13, 2023
@vmamidi vmamidi changed the title The handle_cache_operation_on_forward_server_response function doesn't handle range operations for mismatched server and cache responses The handle_cache_operation_on_forward_server_response function doesn't handle proper range operations for mismatched server and cache responses Nov 13, 2023
@SolidWallOfCode
Copy link
Member

The question is whether the 404 should invalidate the cached object. Is the behavior you expect is that an error response from the upstream should invalidate the cached object? The PR consensus was this seems reasonable but we wanted clarity on the desired behavior.

@mlibbey
Copy link
Contributor

mlibbey commented Nov 14, 2023

What are the corner cases to consider? If the origin says that an object no longer exists during revalidation, when would we want to keep serving old content in cache?

@vmamidi
Copy link
Contributor Author

vmamidi commented Nov 15, 2023

The question is whether the 404 should invalidate the cached object. Is the behavior you expect is that an error response from the upstream should invalidate the cached object? The PR consensus was this seems reasonable but we wanted clarity on the desired behavior.

The 404 response already invalidates the cache, but ATS serves a 206 response for the request that triggers cache invalidation. Subsequent responses are then served from the cache as 404.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants