Make sure to load a fresh copy of the phishing configuration#297
Merged
danfinlay merged 4 commits intoMetaMask:developfrom Nov 17, 2020
Merged
Make sure to load a fresh copy of the phishing configuration#297danfinlay merged 4 commits intoMetaMask:developfrom
danfinlay merged 4 commits intoMetaMask:developfrom
Conversation
Gudahtt
reviewed
Nov 9, 2020
Member
Gudahtt
left a comment
There was a problem hiding this comment.
Wouldn't this always skip the cache? Not just upon initialization, but on every request.
It seems that the default cache mode used here is no-cache, which already seems ideal here. From the MDN docs:
The browser looks for a matching request in its HTTP cache.
- If there is a match, fresh or stale, the browser will make a conditional request to the remote server. If the server indicates that the resource has not changed, it will be returned from the cache. Otherwise the resource will be downloaded from the server and the cache will be updated.
- If there is no match, the browser will make a normal request, and will update the cache with the downloaded resource.
This should guarantee that we're getting an up-to-date copy, even if there is a cached copy.
Contributor
|
Closing per @Gudahtt's feedback. |
danfinlay
reviewed
Nov 17, 2020
danfinlay
reviewed
Nov 17, 2020
danfinlay
approved these changes
Nov 17, 2020
Gudahtt
added a commit
to MetaMask/metamask-extension
that referenced
this pull request
Dec 17, 2020
This update comes with a breaking change to the Approval controller. It now requires a `defaultApprovalType` parameter. I don't think we have any use for a default approval type, but I've added a "NO_TYPE" one for now because it's a strict requirement. We should consider making this parameter optional in the future, for cases like this where it's not needed. This update will hopefully address some caching issues we've been seeing with our phishing configuration. See here for more details: MetaMask/core#297
Member
|
It seems I was mistaken about the default behaviour. By default The change made in this PR (using cache mode Note that even with this change, the phishing config is still cached for long periods on the server. This will only prevent stale client caches. But it's a start! |
Gudahtt
added a commit
to MetaMask/metamask-extension
that referenced
this pull request
Dec 17, 2020
This update comes with a breaking change to the Approval controller. It now requires a `defaultApprovalType` parameter. I don't think we have any use for a default approval type, but I've added a "NO_TYPE" one for now because it's a strict requirement. We should consider making this parameter optional in the future, for cases like this where it's not needed. This update will hopefully address some caching issues we've been seeing with our phishing configuration. See here for more details: MetaMask/core#297
Gudahtt
added a commit
to MetaMask/metamask-extension
that referenced
this pull request
Dec 17, 2020
This update comes with a breaking change to the Approval controller. It now requires a `defaultApprovalType` parameter. I don't think we have any use for a default approval type, but I've added a "NO_TYPE" one for now because it's a strict requirement. We should consider making this parameter optional in the future, for cases like this where it's not needed. This update will hopefully address some caching issues we've been seeing with our phishing configuration. See here for more details: MetaMask/core#297
Gudahtt
added a commit
to MetaMask/metamask-extension
that referenced
this pull request
Dec 18, 2020
This update comes with a breaking change to the Approval controller. It now requires a `defaultApprovalType` parameter. I don't think we have any use for a default approval type, but I've added a "NO_TYPE" one for now because it's a strict requirement. We should consider making this parameter optional in the future, for cases like this where it's not needed. This update will hopefully address some caching issues we've been seeing with our phishing configuration. See here for more details: MetaMask/core#297
MajorLift
pushed a commit
that referenced
this pull request
Oct 11, 2023
* Make sure to load a fresh copy of the configuration * Update src/third-party/PhishingController.ts * Update src/third-party/PhishingController.ts Co-authored-by: Dan Finlay <542863+danfinlay@users.noreply.github.com>
MajorLift
pushed a commit
that referenced
this pull request
Oct 11, 2023
* Make sure to load a fresh copy of the configuration * Update src/third-party/PhishingController.ts * Update src/third-party/PhishingController.ts Co-authored-by: Dan Finlay <542863+danfinlay@users.noreply.github.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
I've noticed that the phishing configuration could be fetched from the disk cache in Chrome due to the initial cache policy when it's fetched for the first time - this makes sure that polling will be done over the network to have a fresh copy.