Closed Bug 996871 Opened 10 years ago Closed 10 years ago

add preferences to talos to prevent several features from making external network connections

Categories

(Testing :: Talos, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: jmaher, Mentored)

References

Details

(Whiteboard: [lang=python])

Attachments

(1 file, 1 obsolete file)

In bug 995995, we identified some external access from the browser, oh no!

here are the prefs:
+
+// Don't connect to Yahoo! for RSS feed tests.
+// en-US only uses .types.0.uri, but set all of them just to be sure.
+user_pref('browser.contentHandlers.types.0.uri', 'http://test1.example.org/rss?url=%%s')
+user_pref('browser.contentHandlers.types.1.uri', 'http://test1.example.org/rss?url=%%s')
+user_pref('browser.contentHandlers.types.2.uri', 'http://test1.example.org/rss?url=%%s')
+user_pref('browser.contentHandlers.types.3.uri', 'http://test1.example.org/rss?url=%%s')
+user_pref('browser.contentHandlers.types.4.uri', 'http://test1.example.org/rss?url=%%s')
+user_pref('browser.contentHandlers.types.5.uri', 'http://test1.example.org/rss?url=%%s')


Here is a way to get started with Talos:
https://wiki.mozilla.org/Buildbot/Talos/Running#Running_locally_-_Source_Code

Here is how to get familiar with Mercurial:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ

the general preferences for talos are located here:
http://hg.mozilla.org/build/talos/file/tip/talos/PerfConfigurator.py#l237

the above prefs need to be converted from the user_pref(name, value) to a data structure:
name: value,
Blocks: 617414
Dropping the good-first label here - this looks a little heavy to be a starter bug, and blockers don't get to be good first bugs.
Whiteboard: [good first bug][mentor=jmaher][lang=python] → [mentor=jmaher][lang=python]
This is a really simple fix- literally just copy/paste the code into a file.
.
I didn't realize this was blocking anything, my impression was this work was nice to have so we can have similar prefs in unit tests and talos.  We don't do any explicit rss in talos
Blocks: 1023483
Mentor: jmaher
Whiteboard: [mentor=jmaher][lang=python] → [lang=python]
ok, here are more preferences we need to add (taken from the last 2 months of history http://hg.mozilla.org/mozilla-central/filelog/f78e532e8a10/testing/profiles/prefs_general.js):
user_pref("network.http.speculative-parallel-limit", 0);

user_pref('toolkit.telemetry.server', 'https://%(server)s/telemetry-dummy/');
or
user_pref("toolkit.telemetry.enabled", false);

possibly this:
user_pref('identity.fxaccounts.auth.uri', 'https://%(server)s/fxa-dummy/');

and most likely this:
user_pref("datareporting.healthreport.about.reportUrl", "http://%(server)s/abouthealthreport/");

and this:
user_pref("general.useragent.updates.enabled", false);

and this:
user_pref("browser.webapps.checkForUpdates", 0);


Nathan, any concerns here with these prefs for talos or overlooked prefs that you can think of?
Flags: needinfo?(nfroyd)
(In reply to Joel Maher (:jmaher) from comment #3)
> user_pref('toolkit.telemetry.server', 'https://%(server)s/telemetry-dummy/');
> or
> user_pref("toolkit.telemetry.enabled", false);

I think we want the latter one, though having the former one for safety would be good.

> possibly this:
> user_pref('identity.fxaccounts.auth.uri', 'https://%(server)s/fxa-dummy/');
> 
> and most likely this:
> user_pref("datareporting.healthreport.about.reportUrl",
> "http://%(server)s/abouthealthreport/");

Yes.

> and this:
> user_pref("general.useragent.updates.enabled", false);

Yes.

> and this:
> user_pref("browser.webapps.checkForUpdates", 0);

Yes.

> Nathan, any concerns here with these prefs for talos or overlooked prefs
> that you can think of?

I think we also want to turn off experiments (experiments.{enabled,supported}, bug 997188), along with snippets (browser.snippets.enabled, browser.snippets.syncPromo.enabled, bug 1022785).  We could turn off safebrowsing (cf. bug 1018400), but it looks like the talos configuration already sets the server preferences for safebrowsing to local servers, so maybe that's not a big deal.

And whatever solution is used for bug 1027125 and bug 1027287, we will want in Talos.
Flags: needinfo?(nfroyd)
Blocks: 1026970
I do not think we should disable telemetry (or experiments) during tests. There are important codepaths there. The telemetry prefs cover all sorts of things that are not just the upload step, and this could potentially hide performance or correctness issues with telemetry enabled.
so telemetry is making outside network access, that is the main reason for disabling it.  We already disable it on unittests.  I am fine with whatever decision we make on it.
(In reply to Joel Maher (:jmaher) from comment #6)
> so telemetry is making outside network access, that is the main reason for
> disabling it.  We already disable it on unittests.  I am fine with whatever
> decision we make on it.

I think what Benjamin is saying is that he'd rather us leave it enabled, but force it to use a dummy URL, so it wouldn't upload anything.

This is what we do already for mochitests:
http://mxr.mozilla.org/mozilla-central/source/testing/profiles/prefs_general.js#194
193 // We want to collect telemetry, but we don't want to send in the results.
194 user_pref('toolkit.telemetry.server', 'https://%(server)s/telemetry-dummy/');

Although for JS tests we do just disable it outright:
http://mxr.mozilla.org/mozilla-central/source/js/src/tests/user.js#25
sold!
(In reply to Ed Morley [:edmorley UTC+0] from comment #7)
> Although for JS tests we do just disable it outright:
> http://mxr.mozilla.org/mozilla-central/source/js/src/tests/user.js#25

Maybe someday we'll have a unified test environment where we know a dummy HTTP server is always available, and then we won't need to disable lots of things during non-mochitests...</pipe-dream>
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Attachment #8443057 - Flags: review?(nfroyd)
and for speculative parsing.
Attachment #8443057 - Attachment is obsolete: true
Attachment #8443057 - Flags: review?(nfroyd)
Attachment #8443058 - Flags: review?(nfroyd)
Comment on attachment 8443058 [details] [diff] [review]
port over recent mochitest prefs to talos (1.1)

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

r=me with the changes below

::: talos/PerfConfigurator.py
@@ +303,5 @@
> +        'browser.contentHandlers.types.1.uri': 'http://127.0.0.1/rss?url=%%s',
> +        'browser.contentHandlers.types.2.uri': 'http://127.0.0.1/rss?url=%%s',
> +        'browser.contentHandlers.types.3.uri': 'http://127.0.0.1/rss?url=%%s',
> +        'browser.contentHandlers.types.4.uri': 'http://127.0.0.1/rss?url=%%s',
> +        'browser.contentHandlers.types.5.uri': 'http://127.0.0.1/rss?url=%%s',

I think all of these should use %s; other uses of the pref have %s and I believe the %%s is only used in prefs_general.js to make things come out right when Python's % operator is applied to the contents.

@@ +305,5 @@
> +        'browser.contentHandlers.types.3.uri': 'http://127.0.0.1/rss?url=%%s',
> +        'browser.contentHandlers.types.4.uri': 'http://127.0.0.1/rss?url=%%s',
> +        'browser.contentHandlers.types.5.uri': 'http://127.0.0.1/rss?url=%%s',
> +        'identity.fxaccounts.auth.uri': 'https://127.0.0.1/fxa-dummy/',
> +        'datareporting.healthreport.about.reportUrl': 'http://127.0.0.1/abouthealthreport/',

prefs_general.js has datareporting.healthreport.documentServerURI, maybe set that too?

@@ +311,5 @@
> +        'browser.webapps.checkForUpdates': 0,
> +        'browser.snippets.enabled': False,
> +        'browser.snippets.syncPromo.enabled': False,
> +        'toolkit.telemetry.server', 'https://127.0.0.1/telemetry-dummy/',
> +        'network.http.speculative-parallel-limit': 0

Nit: maybe include trailing comma here?
Attachment #8443058 - Flags: review?(nfroyd) → review+
Depends on: 1028999
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Summary: add preferences to talos to remove external network access for rss → add preferences to talos to prevent several features from making external network connections
we still have connections going to telemetry-experiment.cdn.mozilla.net.

I have verified in runs that we have:
toolkit.telemetry.server: https://127.0.0.1/telemetry-dummy/

bsmedberg, what should I do to disable these?
Flags: needinfo?(benjamin)
hmm, could it be as simple as adding;
user_pref("experiments.manifest.uri", "http://%(server)s/experiments-dummy/manifest");
thanks!
Fixing in bug 1030093
No longer blocks: 1023483
Blocks: 1030161
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: