Closed
Bug 1394056
Opened 7 years ago
Closed 7 years ago
goog-phish-proto cannot be used on non-official builds
Categories
(Toolkit :: Safe Browsing, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | + | verified |
firefox57 | --- | fixed |
People
(Reporter: francois, Assigned: tnguyen)
References
Details
(Keywords: regression, Whiteboard: #sbv4-m9)
Attachments
(2 files)
59 bytes,
text/x-review-board-request
|
francois
:
review+
|
Details |
1.36 KB,
patch
|
francois
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
We can only use goog-phish-proto on official builds and need to use googpub-phish-proto otherwise like we do for Safe Browsing V2 lists: https://searchfox.org/mozilla-central/rev/5696c3e525fc8222674eed6a562f5fcbe804c4c7/modules/libpref/init/all.js#5343-5349 [Tracking Requested - why for this release]: Safe Browsing will be broken on most Linux distributions unless we fix this.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → tnguyen
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8901650 -
Flags: review?(francois)
Reporter | ||
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8901650 [details] Bug 1394056 - Should not use SOCIAL_ENGINEERING threat type on non-official builds https://reviewboard.mozilla.org/r/173072/#review178644 Looks good. Thomas, can you please test by building both an official build and a non-official build? That way we can confirm that the right list is selected at build time.
Attachment #8901650 -
Flags: review?(francois) → review+
Reporter | ||
Updated•7 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•7 years ago
|
||
Thanks Francois, verified it works by adding/removing line export MOZILLA_OFFICIAL=1 in mozconfig
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&tochange=9309f280df23bf975c18a016a1fa8d9a035d8238&fromchange=ab26aa132ce72f927505b18f1fa455035a61b4d7&selectedJob=126605247
Assignee | ||
Comment 6•7 years ago
|
||
MozReview-Commit-ID: L8N9vg9RGg
Assignee | ||
Updated•7 years ago
|
Attachment #8902176 -
Attachment description: Should not use SOCIAL_ENGINEERING threat type on non-official builds → For Beta release - Should not use SOCIAL_ENGINEERING threat type on non-official builds
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/97423f06e9c8 Should not use SOCIAL_ENGINEERING threat type on non-official builds r=francois
Keywords: checkin-needed
Reporter | ||
Updated•7 years ago
|
Attachment #8902176 -
Flags: review+
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/97423f06e9c8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 9•7 years ago
|
||
Please nominate this for Beta approval when you get a chance.
status-firefox55:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Flags: needinfo?(tnguyen)
Version: unspecified → 56 Branch
Reporter | ||
Comment 10•7 years ago
|
||
Comment on attachment 8902176 [details] [diff] [review] For Beta release - Should not use SOCIAL_ENGINEERING threat type on non-official builds Approval Request Comment [Feature/Bug causing the regression]: bug 1394056 [User impact if declined]: Safe Browsing is completely broken on non-official builds. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Not yet, it just landed on central. [Needs manual test from QE? If yes, steps to reproduce]: Start with an official build of Firefox (e.g. Nightly). 1. Start the browser in a new profile 2. Wait 5 minutes 3. Go to https://testsafebrowsing.appspot.com/ and test the first 3 links under "Webpage Warnings". Repeat the test with a non-official build. [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: It's a simple pref flip. [String changes made/needed]: None
Flags: needinfo?(tnguyen)
Attachment #8902176 -
Flags: approval-mozilla-beta?
Comment 11•7 years ago
|
||
Hi Florin, could you help find someone to verify if this issue was fixed as expected on the latest Nightly build? Thanks!
Flags: qe-verify+
Flags: needinfo?(florin.mezei)
Comment 12•7 years ago
|
||
Comment on attachment 8902176 [details] [diff] [review] For Beta release - Should not use SOCIAL_ENGINEERING threat type on non-official builds I think we should go ahead and uplift this for beta 8 and verify it there. Will this need a different way to test/verify because of the experiment with v4 safebrowsing? How do we know that we're testing the correct version?
Attachment #8902176 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 13•7 years ago
|
||
Question for you in comment 12, francois. Thanks!
Flags: needinfo?(francois)
Reporter | ||
Comment 14•7 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12) > Comment on attachment 8902176 [details] [diff] [review] > For Beta release - Should not use SOCIAL_ENGINEERING threat type on > non-official builds > > I think we should go ahead and uplift this for beta 8 and verify it there. Agreed. It's a trivial patch, so very low-risk. > Will this need a different way to test/verify because of the experiment with > v4 safebrowsing? How do we know that we're testing the correct version? No, the experiment has not yet started. Beta and Nightly are all 100% V4 at the moment.
Flags: needinfo?(francois)
Comment 15•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/9bcdcd27ed40
Comment 16•7 years ago
|
||
Removing ni? as this will be covered as part of bug verification in Beta 56.
Flags: needinfo?(florin.mezei)
Comment 17•7 years ago
|
||
Verified! It works. :) Step: 1. Download source code from Beta branch 2. Run ./mach build 3. Run ./mach run 4. Enter about:config in Url bar 5. Check the preference: url classifier.phishTable Expected Result: - url classifier.phishTable is "googpub-phish-proto,test-phish-simple" Actual Result: - url classifier.phishTable is "googpub-phish-proto,test-phish-simple" OS: Windows 10 Pro Build info: 56.0b11 Hg log: changeset: 421677:6bd183fc6921
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•