Revisit headers load fix #751
Merged
Conversation
This file contains 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
- use http headers from headers record! - parse records on initial lookup, as may need to use http headers from headers record - possible fix for sul-dlss/was-pywb#64
tests: update test to use original record, revisit contains no content-length
|
Thanks for investigating this! |
ikreymer
added a commit
that referenced
this pull request
Aug 19, 2022
- if a revisit is of a redirect (3xx response) and revisit has http headers, return the http headers with empty payload -- don't bother loading the original record builds on changes in #751
8 tasks
ikreymer
added a commit
that referenced
this pull request
Aug 20, 2022
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.
Description
Revisit records may contain HTTP headers, or they may contain no headers. If they do contain headers, the replay should use the HTTP headers from the revisit record, and they payload from the original record referenced by the revisit.
Unfortunately, this was not working (possibly ever?) as expected!
It appears that pywb was always using the HTTP headers from the original record.
Generally, this is fine, except in the case where the revisit record is of a redirect, and the HTTP headers have changed between the revisit and the original (while the payload is the same, and generally ignored).
Motivation and Context
This fixes an issue originally found in sul-dlss/was-pywb#64
Added test case which I believe replicates this issue.
cc: @edsu
Screenshots (if appropriate):
Types of changes
Checklist: