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)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jmaher, Assigned: jmaher, Mentored)
References
Details
(Whiteboard: [lang=python])
Attachments
(1 file, 1 obsolete file)
2.22 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
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,
Comment 1•10 years ago
|
||
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]
Assignee | ||
Comment 2•10 years ago
|
||
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
Updated•10 years ago
|
Mentor: jmaher
Whiteboard: [mentor=jmaher][lang=python] → [lang=python]
Assignee | ||
Comment 3•10 years ago
|
||
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)
Comment 4•10 years ago
|
||
(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)
Comment 5•10 years ago
|
||
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.
Assignee | ||
Comment 6•10 years ago
|
||
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.
Comment 7•10 years ago
|
||
(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
Assignee | ||
Comment 8•10 years ago
|
||
sold!
Comment 9•10 years ago
|
||
(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 | ||
Comment 10•10 years ago
|
||
Assignee | ||
Comment 11•10 years ago
|
||
and for speculative parsing.
Attachment #8443057 -
Attachment is obsolete: true
Attachment #8443057 -
Flags: review?(nfroyd)
Attachment #8443058 -
Flags: review?(nfroyd)
Comment 12•10 years ago
|
||
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+
Assignee | ||
Comment 13•10 years ago
|
||
http://hg.mozilla.org/build/talos/rev/3a6a28b96228
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
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
Assignee | ||
Comment 15•10 years ago
|
||
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)
Assignee | ||
Comment 16•10 years ago
|
||
hmm, could it be as simple as adding; user_pref("experiments.manifest.uri", "http://%(server)s/experiments-dummy/manifest");
Comment 17•10 years ago
|
||
http://hg.mozilla.org/mozilla-central/annotate/a19e0434ea52/testing/profiles/prefs_general.js#l61
Flags: needinfo?(benjamin)
Assignee | ||
Comment 18•10 years ago
|
||
thanks!
Comment 19•10 years ago
|
||
Fixing in bug 1030093
You need to log in
before you can comment on or make changes to this bug.
Description
•