Closed Bug 998370 Opened 7 years ago Closed 7 years ago

gethash completions broken in Beta

Categories

(Toolkit :: Safe Browsing, defect)

x86
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 29
Tracking Status
firefox29 + verified

People

(Reporter: mmc, Assigned: mmc)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Assignee: nobody → mmc
Status: NEW → ASSIGNED
Comment on attachment 8409026 [details] [diff] [review]
Rollback bugs 997759, 989232, 985720, 985623 in beta (

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bugs 997759, 989232, 985720, 985623 collectively converted some preferences to comma-separated lists, which is breaking gethash completions.

User impact if declined: Phishing and malware protection broken in 29

Testing completed (on m-c, etc.): In progress

Risk to taking this patch (and alternatives if risky): It is risky, but the most reasonable thing we can do at this point. GCP found the bug but there's not time to really test it any more. This resets the safebrowsing state to FF 28.

String or IDL/UUID changes made by this patch: None.
Attachment #8409026 - Flags: approval-mozilla-beta?
mDisallowCompletionsTables is correctly constructed by splitting the multiple tables, but the other preferences aren't, even though they (can) also contain multiple lists:
http://hg.mozilla.org/mozilla-central/file/45ba19361b97/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#l1090

We can do a proper fix in Aurora and up.
Attachment #8409026 - Attachment is obsolete: true
Attachment #8409026 - Flags: approval-mozilla-beta?
Comment on attachment 8409045 [details] [diff] [review]
Rollback bugs 997759, 989232, 985720, 985623 in beta (

[Approval Request Comment]
Comment 4.

I tested this patch manually by going to phishtank.com and checking that firefox caught some of the phishing URLs there. Also updates still work.

mchew@mchew-12604:~/mozilla-beta$ ls -al ~/.cache/mozilla/firefox/pepruq2g.safebrowsing-test/safebrowsing/
total 1772
drwxr-xr-x 2 mchew mchew   4096 Apr 18 10:52 .
drwx------ 6 mchew mchew   4096 Apr 18 10:52 ..
-rw-r--r-- 1 mchew mchew     12 Apr 18 10:52 goog-badbinurl-shavar.cache
-rw-r--r-- 1 mchew mchew 244552 Apr 18 10:52 goog-badbinurl-shavar.pset
-rw-r--r-- 1 mchew mchew 258669 Apr 18 10:52 goog-badbinurl-shavar.sbstore
-rw-r--r-- 1 mchew mchew  12332 Apr 18 10:52 goog-downloadwhite-digest256.cache
-rw-r--r-- 1 mchew mchew     16 Apr 18 10:52 goog-downloadwhite-digest256.pset
-rw-r--r-- 1 mchew mchew  14888 Apr 18 10:52 goog-downloadwhite-digest256.sbstore
-rw-r--r-- 1 mchew mchew     12 Apr 18 10:52 goog-malware-shavar.cache
-rw-r--r-- 1 mchew mchew 281784 Apr 18 10:52 goog-malware-shavar.pset
-rw-r--r-- 1 mchew mchew 379563 Apr 18 10:52 goog-malware-shavar.sbstore
-rw-r--r-- 1 mchew mchew     12 Apr 18 10:52 goog-phish-shavar.cache
-rw-r--r-- 1 mchew mchew 335920 Apr 18 10:52 goog-phish-shavar.pset
-rw-r--r-- 1 mchew mchew 218498 Apr 18 10:52 goog-phish-shavar.sbstore
-rw-r--r-- 1 mchew mchew     44 Apr 18 10:52 test-malware-simple.cache
-rw-r--r-- 1 mchew mchew     16 Apr 18 10:52 test-malware-simple.pset
-rw-r--r-- 1 mchew mchew    232 Apr 18 10:52 test-malware-simple.sbstore
-rw-r--r-- 1 mchew mchew     44 Apr 18 10:52 test-phish-simple.cache
-rw-r--r-- 1 mchew mchew     16 Apr 18 10:52 test-phish-simple.pset
-rw-r--r-- 1 mchew mchew    232 Apr 18 10:52 test-phish-simple.sbstore
Attachment #8409045 - Flags: approval-mozilla-beta?
Btw, the latest version of the patch includes the hotfix code to not request problematic tables from Google: bug 985627
Comment on attachment 8409045 [details] [diff] [review]
Rollback bugs 997759, 989232, 985720, 985623 in beta (

Review of attachment 8409045 [details] [diff] [review]:
-----------------------------------------------------------------

Verified by applying to mozilla-beta and diffing the resulting SafeBrowsing code against mozilla-release. This points to the only difference being HTTPS support and better OOM protection (this is OK and expected, those changes did not produce problems during beta). The Application Reputation code is gone as wanted.

As discussed with mmc, the download_allow_table and download_block_table preferences must be empty. They are not so in the Firefox 28 release (source), but we hotfixed that after the release.
Attachment #8409045 - Flags: review+
Comment on attachment 8409045 [details] [diff] [review]
Rollback bugs 997759, 989232, 985720, 985623 in beta (

Approved
Attachment #8409045 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I verified the bug using the following environment:
FF 29 RC
Build Id:20140421221237
OS:
Ubuntu 14.04 x32
Mac Os 10.9.2
Windows 7 x64

I verified the bug using the information provided on comment 8

On Ubuntu and Mac I didn't find the following files:

-rw-r--r-- 1 mchew mchew     12 Apr 18 10:52 goog-badbinurl-shavar.cache
-rw-r--r-- 1 mchew mchew 244552 Apr 18 10:52 goog-badbinurl-shavar.pset
-rw-r--r-- 1 mchew mchew 258669 Apr 18 10:52 goog-badbinurl-shavar.sbstore
-rw-r--r-- 1 mchew mchew  12332 Apr 18 10:52 goog-downloadwhite-digest256.cache
-rw-r--r-- 1 mchew mchew     16 Apr 18 10:52 goog-downloadwhite-digest256.pset
-rw-r--r-- 1 mchew mchew  14888 Apr 18 10:52 goog-downloadwhite-digest256.sbstore

If this is not expected behavior please provide additional information.
Not finding those tables is the expected result (that change was made and is implied in comment 9). Just checking: when you say Ubuntu and Mac, you mean that is what you tested? Or did you find them on Windows instead?
Since Catalin verified this on my Win 7 x64 machine, I went ahead and double checked this behavior on Windows. I see that the "goog-badbinurl-shavar" and "goog-downloadwhite-digest256" files display for previous versions (e.g. Firefox 29 Beta 9), but are no longer displayed for Firefox 29 RC.
Sounds good. Given that the other branches are being fixed in bug 998396, I'll close this?
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Status: RESOLVED → VERIFIED
(In reply to Gone til 4/28 [:mmc] (please use needinfo) from comment #4)
> Bug caused by (feature/regressing bug #): bugs 997759, 989232, 985720,
> 985623 collectively converted some preferences to comma-separated lists,
> which is breaking gethash completions.

Can you correctly update those bugs to be marked as status-firefox29/30: disabled?
Flags: needinfo?(mmc)
Flags: needinfo?(gpascutto)
I set them to disabled on firefox29, where we did the backout. They are fixed in firefox30.
Flags: needinfo?(mmc)
Flags: needinfo?(gpascutto)
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.