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

Remove If-None-Match header from phishing config requests #277

Merged
merged 1 commit into from
Sep 15, 2020

Conversation

whymarrh
Copy link
Contributor

Refs #256

This PR removes the If-None-Match header from the requests for the phishing list. The header was a holder from the old implementation that used GitHub's content API (#232) and is no longer needed. (The header also caused the request to fail when a UA enforced Access-Control-Allow-Headers, which is missing on the jsDelivr response.)

Removing this header reverts the fetch cache mode to default:

"default"

Fetch will inspect the HTTP cache on the way to the network. If the HTTP cache contains a matching fresh response it will be returned. If the HTTP cache contains a matching stale-while-revalidate response it will be returned, and a conditional network fetch will be made to update the entry in the HTTP cache. If the HTTP cache contains a matching stale response, a conditional network fetch will be returned to update the entry in the HTTP cache. Otherwise, a non-conditional network fetch will be returned to update the entry in the HTTP cache.

As a result the browser will take care of the conditional network fetch.

This—as a bonus—removes the CORS preflight request as there are no custom options anymore.

@whymarrh whymarrh requested a review from a team as a code owner September 15, 2020 20:17
@whymarrh
Copy link
Contributor Author

I can confirm that this works in the extension.

@whymarrh whymarrh mentioned this pull request Sep 15, 2020
Copy link
Member

@rickycodes rickycodes left a comment

Choose a reason for hiding this comment

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

LGTM

nice to see a /* istanbul ignore next */ go away

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