Closed Bug 1363879 Opened 3 years ago Closed 3 years ago
Sort gethash prefixes to hide noise entries
59 bytes, text/x-review-board-request
The hash completer ultimately adds the partial hashes it receives to an array: https://searchfox.org/mozilla-central/rev/d66b9f27d5630a90b2fce4d70d4e9050f43df9b4/toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js#304 which will ultimately (for V4) be put into a Set for de-duplication purposes: https://searchfox.org/mozilla-central/rev/d66b9f27d5630a90b2fce4d70d4e9050f43df9b4/toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js#472-475 but into a randomized array (for V2): https://searchfox.org/mozilla-central/rev/d66b9f27d5630a90b2fce4d70d4e9050f43df9b4/toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js#502-510 to make sure the noise entry is not always in the same place. We should shuffle the V4 array in the same way as the V2 one. Ideally refactoring as much of that code into a common function.
GCP, do you remember why you decided to shuffle the gethash array in bug 807852 instead of simply sorting it? If the array is sorted, the hash entries will be arbitrary mixed in (in a deterministic way) with the real entry and so it won't be possible to know which is which, no?
I probably didn't think of sorting. Shuffling may well be faster here, though it's a tough call due to JS being involved.
Summary: Randomize the order of gethash prefixes to hide noise entries → Sort gethash prefixes to hide noise entries
In V2 we shuffled the hash entries before sending the request to obscure the real entry from noises. We should do the same to V4, sort() is enough for both because the array almost contains 5 entries MozReview-Commit-ID: 4uOXIF83KQL
Attachment #8868442 - Attachment is obsolete: true
Comment on attachment 8868446 [details] Bug 1363879 - Sort gethash prefixes to hide noise entries https://reviewboard.mozilla.org/r/140040/#review143684 ::: commit-message-e66de:5 (Diff revision 1) > +Bug 1363879 - Sort gethash prefixes to hide noise entries > + > +In V2 we shuffled the hash entries before sending the request to obscure the real > +entry from noises. We should also hide the real entry in V4. Using sort() is > +enough for both V2 and V4 because the array contains 5 entries in almost cases "contains exactly 5 entries in almost all cases"
Attachment #8868446 - Flags: review?(francois) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/5ff7323da7ec Sort gethash prefixes to hide noise entries r=francois
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ab715ee9ef99 Add Advanced Layers to the reftest sandbox. (bug 1363879 part 24, r=mattwoodrow)
Backout by firstname.lastname@example.org: https://hg.mozilla.org/mozilla-central/rev/d08fac2a15fb Backed out changeset ab715ee9ef99
The commit from comment 12, which just got backed out, had nothing to do with this bug. It had a typo in the bug number. David, make sure you fix the typo in the commit message before you re-land this :)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla55 → ---
Fixed for real by commit in comment 11.
Status: REOPENED → RESOLVED
Closed: 3 years ago → 3 years ago
Resolution: --- → FIXED
Version: unspecified → 55 Branch
You need to log in before you can comment on or make changes to this bug.