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

StaticFiles: Fix cache validation bug for deleted files in html mode #1023

Merged
merged 3 commits into from
Feb 5, 2021

Conversation

jvstme
Copy link
Contributor

@jvstme jvstme commented Aug 9, 2020

Currently, when StaticFiles(html=True) fails to find the requested file and decides to return the 404 error page, it compares the If-Modified-Since of the requested file with the Last-Modified of 404.html. Therefore, if a requested file was deleted but has had the same Last-Modified as that of 404.html, StaticFiles would still return 304 Not Modified, while the logical behavior is to return the error page.

To reproduce the issue manually, create the app and the html files:

echo "from starlette.staticfiles import StaticFiles; app = StaticFiles(directory='.', html=True)" > main.py
echo "<p>some file</p>" > some.html && echo "<p>404 file</p>" > 404.html

(I will assume some.html and 404.html have the same Last-Modified, since they are created by the same command); then start main:app and open some.html in the browser. If you then delete some.html and reload the page, the file should still be there.

This PR fixes the issue by returning the 404 response directly from StaticFiles.get_response, rather then calling StaticFiles.file_response.

Previously StaticFiles would return 304 for a deleted file if its
Last-Modified date was the same as that of 404.html
@JayH5 JayH5 self-assigned this Feb 4, 2021
@JayH5 JayH5 added the bug Something isn't working label Feb 4, 2021
@JayH5 JayH5 added the staticfiles Static file serving label Feb 5, 2021
Copy link
Member

@JayH5 JayH5 left a comment

Choose a reason for hiding this comment

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

Thanks, this seems like a potentially quite frustrating bug 😓

@JayH5 JayH5 merged commit 0ac60bb into encode:master Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working staticfiles Static file serving
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants