Skip to content

Fix the handling of bad Last-Modified headers #277

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

Merged
merged 1 commit into from
Jun 13, 2023
Merged

Conversation

sirosen
Copy link
Member

@sirosen sirosen commented Jun 13, 2023

Originally found via an OverflowError on some Windows usage where the default Last-Modified header was used, there are several issues which were not handled correctly by the downloader.

  1. OverflowError: a last mod date outside of mktime's supported range causes this
  2. ValueError: a malformed header causes strptime to fail
  3. LookupError/KeyError: the header is missing

(3) was previously handled by falling back to a default time which was then strptime parsed. This is now caught as a LookupError and handled with the fixed default of 0.0 used for all of these bad-parse cases. The semantics are the same, but the implementation is simpler and more uniform.

Originally found via an OverflowError on some Windows usage where the
default Last-Modified header was used, there are several issues which
were not handled correctly by the downloader.
1. OverflowError: a last mod date outside of `mktime`'s supported
   range causes this
2. ValueError: a malformed header causes `strptime` to fail
3. LookupError/KeyError: the header is missing

(3) was previously handled by falling back to a default time which was
then strptime parsed. This is now caught as a LookupError and handled
with the fixed default of `0.0` used for all of these bad-parse cases.
The semantics are the same, but the implementation is simpler and more
uniform.
@sirosen sirosen merged commit 75e9db7 into main Jun 13, 2023
@sirosen sirosen deleted the fix-bad-lastmod branch June 13, 2023 16:59
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.

1 participant