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

Ensure invalidate doesn't mutate response x-status key #25

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

timdef
Copy link
Contributor

@timdef timdef commented Apr 4, 2024

I ran into an issue where our application was serving responses with HTTP status code 0. I noticed that some of our metastore cache entries didn't have an x-status key.

I found that this occurs on invalidate when at least one entry is fresh. In this case all stale entries are written back to the cache without x-status.

It seem that the method to restore responses mutates the x-status key in the header hash (resp), and then the same, modified object is written back to the cache.

status = hash.delete('x-status').to_i

This change removes that possibility by always using the rehydrated response when mapping over the entries to invalidate.

This surfaced in my app during the upgrade to Rails 6.1, but I haven't figured out that part of the equation yet.

When invalidate is called the method to restore response mutates the x-status
key in the header hash.
Previously, in the case where one entry is fresh and others are not, the not-
fresh entries could be persisted without any x-status header, potentially later
serving requests with status code 0.
This change removes that possibility by always using the rehydrated response
when mapping over entries to invalidate.
@ioquatix
Copy link
Member

ioquatix commented Apr 5, 2024

I ran the failing test locally to confirm the issue.

@ioquatix ioquatix merged commit 8fd6098 into rack:main Apr 5, 2024
32 of 33 checks passed
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

Successfully merging this pull request may close these issues.

2 participants