Note: There are a few cases of duplicates in user autocompletion which are being worked on.

safebrowsing tests shouldn't load internet pages

RESOLVED FIXED in Firefox 6

Status

()

Toolkit
Safe Browsing
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: bhearsum, Assigned: philor)

Tracking

({verified1.9.2})

Trunk
Firefox 7
verified1.9.2
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox6 fixed, status2.0 .x-fixed, status1.9.2 .20-fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Currently, there's two safebrowsing tests that load a www.mozilla.org webpage:
http://mxr.mozilla.org/mozilla-central/source/browser/components/safebrowsing/content/test/browser_bug415846.js#40
http://mxr.mozilla.org/mozilla-central/source/browser/components/safebrowsing/content/test/browser_bug400731.js#26


We've been trying to get rid of tests like this so we can shut off network access to build & test machines (bug 617414). Can these be replaced with something loading from the local webserver?
I think it should be fine to redirect www.mozilla.org to the local webserver like we do with fxfeeds:
http://mxr.mozilla.org/mozilla-central/source/build/pgo/server-locations.txt#126

We might need to add some content so it doesn't 404, though.

Alternately, maybe we can add some test URLs in the default phishing/malware db on example.com? Not sure if that's worth the effort.
Duplicate of this bug: 627292
Created attachment 538706 [details] [diff] [review]
Fix v.1

Yeah, pretty sure this is the patch I was talking about writing in bug 627292, before I confused myself into thinking that it wasn't needed. Shoving more stuff into the db and testing with a db other than what we ship just doesn't sound like that much fun.
Assignee: nobody → philringnalda
Status: NEW → ASSIGNED
Attachment #538706 - Flags: review?(ted.mielczarek)
Comment on attachment 538706 [details] [diff] [review]
Fix v.1

Do these need to be non-404?
Comment on attachment 538706 [details] [diff] [review]
Fix v.1

http://tbpl.mozilla.org/?tree=Try&rev=9b9fa1e53ce8 says they don't need to be anything but a 404, and my vague understanding of how safebrowsing works says it makes sense that it won't care whether a bad URI is a 404 or a picture of a kitten, but let's ask.
Attachment #538706 - Flags: feedback?(mars.martian+bugmail)
Comment on attachment 538706 [details] [diff] [review]
Fix v.1

Okay. r=me on the addition, would be good to get someone who understands safebrowsing to check that testing with 404s is still valid.
Attachment #538706 - Flags: review?(ted.mielczarek) → review+
I tried accessing its-a-trap.html with my internet disconnected and was still able to retrieve the warning.
Do the tests explicitly fail when no [www.]mozilla.com server is provided?
Oh, two steps up the dependency chain, not just one like I thought: no, they don't fail, according to bug 617414 comment 31.
Comment on attachment 538706 [details] [diff] [review]
Fix v.1

This will stifle any outgoing requests as expected and a 404 is OK for safebrowsing.

Ted: Bugs 662046 and 662213 discuss removing its-a-trap and its-an-attack and providing new pages.
Attachment #538706 - Flags: feedback?(mars.martian+bugmail) → feedback+
http://hg.mozilla.org/integration/mozilla-inbound/rev/99fc38a20ef3
Whiteboard: [inbound]
Merged:
http://hg.mozilla.org/mozilla-central/rev/99fc38a20ef3
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
OS: Linux → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 7
Version: unspecified → Trunk
(Reporter)

Comment 12

6 years ago
Thanks all!
(Reporter)

Comment 13

6 years ago
Comment on attachment 538706 [details] [diff] [review]
Fix v.1

Drivers, this patch applies cleanly to 1.9.2 and mozilla-aurora. I'd like to land it in both of those places to stop the safebrowsing tests from attempting to access the internet, which will start to fail if they continue to once bug 646046 is done.
Attachment #538706 - Flags: approval1.9.2.19?
Attachment #538706 - Flags: approval-mozilla-aurora?
Drivers: note that this is a test-harness-only change with zero risk to the actual shipping product.

Updated

6 years ago
Attachment #538706 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Reporter)

Comment 15

6 years ago
Comment on attachment 538706 [details] [diff] [review]
Fix v.1

http://hg.mozilla.org/releases/mozilla-aurora/rev/6c38dd703981

I think setting checkin+ is the right thing to do here?
Attachment #538706 - Flags: checkin+
(Reporter)

Comment 16

6 years ago
Comment on attachment 538706 [details] [diff] [review]
Fix v.1

Looks we're not turning off mozilla-2.0 quite yet either, so we need this there, too.
> Drivers, this patch applies cleanly to 1.9.2 and mozilla-aurora. I'd like to
> land it in both of those places to stop the safebrowsing tests from
> attempting to access the internet, which will start to fail if they continue
> to once bug 646046 is done.
> note that this is a test-harness-only change with zero risk to the
> actual shipping product.
Attachment #538706 - Flags: approval2.0?
Comment on attachment 538706 [details] [diff] [review]
Fix v.1

Approved for 1.9.2.19, a=dveditz for release-drivers
Approved for the mozilla2.0 repository, a=dveditz for release-drivers
Attachment #538706 - Flags: approval2.0?
Attachment #538706 - Flags: approval2.0+
Attachment #538706 - Flags: approval1.9.2.19?
Attachment #538706 - Flags: approval1.9.2.19+
(In reply to comment #15)
> I think setting checkin+ is the right thing to do here?

Nope, that would say "this is checked in on the trunk, but I'm leaving the bug open for something else." What you wanted to avoid the LegNeatoBot nagging me about pushing to Aurora was "status-firefox6: fixed".
status-firefox6: --- → fixed
(Reporter)

Comment 19

6 years ago
Comment on attachment 538706 [details] [diff] [review]
Fix v.1

http://hg.mozilla.org/releases/mozilla-2.0/rev/e1727a01dc94
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a6fe9498f635
Attachment #538706 - Flags: checkin+
(Reporter)

Updated

6 years ago
status1.9.2: --- → .19-fixed
status2.0: --- → .x-fixed
Verified for 1.9.2 (by looking at tests). Whee.
Keywords: verified1.9.2
Hello.

Are there any guidelines anyone could provide in order to verify this issue?
This was purely a test harness fix. You could verify it by running a packet sniffer (like Wireshark), and running the safebrowsing mochitests and verifying that they do not contact mozilla.org, if you wanted.
Component: Phishing Protection → Phishing Protection
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.