Closed
Bug 997872
Opened 10 years ago
Closed 10 years ago
Make delay before first safebrowsing download smaller/configurable.
Categories
(Toolkit :: Safe Browsing, defect)
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: mmc, Assigned: gcp)
References
Details
Attachments
(1 file)
2.94 KB,
patch
|
mmc
:
review+
|
Details | Diff | Splinter Review |
Reporter | ||
Comment 1•10 years ago
|
||
gcp, can you take this one? I think you know this code better than me.
Flags: needinfo?(gpascutto)
Assignee | ||
Comment 2•10 years ago
|
||
So it turns out the code already does the right thing: // If the user has never downloaded tables, do the check now. // If the user has tables, add a fuzz of a few minutes. var initialUpdateDelay = 3000; if (tableData != "") { // Add a fuzz of 0-5 minutes. initialUpdateDelay += Math.floor(Math.random() * (5 * 60 * 1000)); So on an empty profile, we'll start the first update after 3 seconds already! But wait...that doesn't actually happen, does it? afeBrowsing: initializing safe browsing URLs, client id Firefox [listmanager] Set update url: https://safebrowsing.google.com/safebrowsing/downloads?client=Firefox&appver=31.0a1&pver=2.2&key=no-google-api-key [listmanager] Set gethash url: https://safebrowsing.google.com/safebrowsing/gethash?client=Firefox&appver=31.0a1&pver=2.2 SafeBrowsing: addMozEntries: n:1000 i:test-malware-simple ad:1 a:1:32:40 itisatrap.org/firefox/its-an-attack.htmln:1000 i:test-phish-simple ad:1 a:1:32:37 itisatrap.org/firefox/its-a-trap.html SafeBrowsing: phishingEnabled: true malwareEnabled: true [listmanager] Enabling table updates for goog-phish-shavar [listmanager] Starting managing lists [listmanager] startUpdateChecker repeatingUpdateDelay: 2393213 [listmanager] Enabling table updates for goog-malware-shavar [listmanager] Starting managing lists [listmanager] startUpdateChecker repeatingUpdateDelay: 1763766 [listmanager] Enabling table updates for goog-badbinurl-shavar [listmanager] Starting managing lists [listmanager] startUpdateChecker repeatingUpdateDelay: 1127278 [listmanager] Enabling table updates for [listmanager] Starting managing lists [listmanager] startUpdateChecker repeatingUpdateDelay: 2122796 SafeBrowsing: init() finished [listmanager] kickoffUpdate tableData: test-malware-simple;a:1 test-phish-simple;a:1 [listmanager] kickoffUpdate delay: 187418 Nope. Because we insert the test tables, SafeBrowsing considers we already have tables, and will apply the 0-5 minute delay.
Flags: needinfo?(gpascutto)
Assignee | ||
Comment 3•10 years ago
|
||
>[listmanager] Enabling table updates for Uh, this stinks of a bug in the patch for bug 997759 :( :(
Assignee | ||
Comment 4•10 years ago
|
||
1. Fix the registering of "" lists when the pref is unset. 2. Don't consider tables-we-are-not-updating when checking if this is a first update or not. This doesn't do what the bug describes, but it turns out that if we fix this but that won't be needed: the first update will already happen 3 seconds after startup.
Assignee: nobody → gpascutto
Attachment #8409001 -
Flags: review?(mmc)
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8409001 [details] [diff] [review] Patch 1. Fix delay logic for first update Review of attachment 8409001 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/url-classifier/SafeBrowsing.jsm @@ +15,5 @@ > function getLists(prefName) { > + let pref = Services.prefs.getCharPref(prefName); > + if (!pref) { > + return []; > + } else { No "else" needed, you are already returning ::: toolkit/components/url-classifier/content/listmanager.js @@ +187,5 @@ > // actually updating. > this.maybeToggleUpdateChecking(); > } > > +PROT_ListManager.prototype.kickoffUpdate_ = function (onDiskTableData) Can you add a comment saying that this implements nsIUrlClassifierCallback?
Attachment #8409001 -
Flags: review?(mmc) → review+
Assignee | ||
Comment 6•10 years ago
|
||
>Can you add a comment saying that this implements nsIUrlClassifierCallback? I had to read https://developer.mozilla.org/en-US/docs/Creating_JavaScript_callbacks_in_components to make sense of it. I've added a comment saying "it acts as nsIUrlClassifierCallback for getTables".
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a9610eb15b2
Summary: Make delay before downloading safebrowsing lists configurable. → Make delay before first safebrowsing download smaller/configurable.
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2a9610eb15b2
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Comment hidden (off-topic, typo) |
You need to log in
before you can comment on or make changes to this bug.
Description
•