Bug 1455705 fix how browserSettings.proxyConfig sets network prefs, r?rpl
proxyConfig set urls onto the pref rather than host names. This adds a round-trip test
with a test that uses the proxy after setting the config. Also fixes setting prefs
when httpProxyAll is true.
MozReview-Commit-ID: FpXKjnOXEkl
--- a/toolkit/components/extensions/parent/ext-browserSettings.js
+++ b/toolkit/components/extensions/parent/ext-browserSettings.js
@@ -206,22 +206,18 @@ ExtensionPreferencesManager.addSetting("
"network.proxy.autoconfig_url": value.autoConfigUrl,
"network.proxy.share_proxy_settings": value.httpProxyAll,
"network.proxy.socks_version": value.socksVersion,
"network.proxy.no_proxies_on": value.passthrough,
};
for (let prop of ["http", "ftp", "ssl", "socks"]) {
if (value[prop]) {
- let url = new URL(prop === "socks" ?
- `http://${value[prop]}` :
- value[prop]);
- prefs[`network.proxy.${prop}`] = prop === "socks" ?
- url.hostname :
- `${url.protocol}//${url.hostname}`;
+ let url = new URL(`http://${value[prop]}`);
+ prefs[`network.proxy.${prop}`] = url.hostname;
let port = parseInt(url.port, 10);
prefs[`network.proxy.${prop}_port`] = isNaN(port) ? 0 : port;
} else {
prefs[`network.proxy.${prop}`] = undefined;
prefs[`network.proxy.${prop}_port`] = undefined;
}
}
@@ -363,19 +359,19 @@ this.browserSettings = class extends Ext
autoLogin: Services.prefs.getBoolPref("signon.autologin.proxy"),
proxyDNS: Services.prefs.getBoolPref("network.proxy.socks_remote_dns"),
httpProxyAll: Services.prefs.getBoolPref("network.proxy.share_proxy_settings"),
socksVersion: Services.prefs.getIntPref("network.proxy.socks_version"),
passthrough: Services.prefs.getCharPref("network.proxy.no_proxies_on"),
};
for (let prop of ["http", "ftp", "ssl", "socks"]) {
- let url = Services.prefs.getCharPref(`network.proxy.${prop}`);
+ let host = Services.prefs.getCharPref(`network.proxy.${prop}`);
let port = Services.prefs.getIntPref(`network.proxy.${prop}_port`);
- proxyConfig[prop] = port ? `${url}:${port}` : url;
+ proxyConfig[prop] = port ? `${host}:${port}` : host;
}
return proxyConfig;
},
// proxyConfig is unsupported on android.
undefined, false, ["android"]
),
{
@@ -392,24 +388,36 @@ this.browserSettings = class extends Ext
let value = details.value;
if (!PROXY_TYPES_MAP.has(value.proxyType)) {
throw new ExtensionError(
`${value.proxyType} is not a valid value for proxyType.`);
}
+ if (value.httpProxyAll) {
+ // Match what about:preferences does with proxy settings
+ // since the proxy service does not check the value
+ // of share_proxy_settings.
+ for (let prop of ["ftp", "ssl", "socks"]) {
+ value[prop] = value.http;
+ }
+ }
+
for (let prop of ["http", "ftp", "ssl", "socks"]) {
- let url = value[prop];
- if (url) {
- if (prop === "socks") {
- url = `http://${url}`;
- }
+ let host = value[prop];
+ if (host) {
try {
- new URL(url);
+ // Fixup in case a full url is passed.
+ if (host.includes("://")) {
+ value[prop] = new URL(host).host;
+ } else {
+ // Validate the host value.
+ new URL(`http://${host}`);
+ }
} catch (e) {
throw new ExtensionError(
`${value[prop]} is not a valid value for ${prop}.`);
}
}
}
if (value.proxyType === "autoConfig" || value.autoConfigUrl) {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_browserSettings.js
@@ -230,37 +230,37 @@ add_task(async function test_browser_set
await testSetting(
"useDocumentFonts", false,
{"browser.display.use_document_fonts": 0});
await testSetting(
"useDocumentFonts", true,
{"browser.display.use_document_fonts": 1});
- async function testProxy(config, expectedPrefs) {
+ async function testProxy(config, expectedPrefs, expectedConfig = config) {
// proxyConfig is not supported on Android.
if (AppConstants.platform === "android") {
return Promise.resolve();
}
let proxyConfig = {
- proxyType: "system",
+ proxyType: "none",
autoConfigUrl: "",
autoLogin: false,
proxyDNS: false,
httpProxyAll: false,
socksVersion: 5,
passthrough: "localhost, 127.0.0.1",
http: "",
ftp: "",
ssl: "",
socks: "",
};
return testSetting(
- "proxyConfig", config, expectedPrefs, Object.assign(proxyConfig, config)
+ "proxyConfig", config, expectedPrefs, Object.assign(proxyConfig, expectedConfig)
);
}
await testProxy(
{proxyType: "none"},
{"network.proxy.type": proxySvc.PROXYCONFIG_DIRECT},
);
@@ -304,55 +304,77 @@ add_task(async function test_browser_set
await testProxy(
{
proxyType: "manual",
http: "http://www.mozilla.org",
autoConfigUrl: "",
},
{
"network.proxy.type": proxySvc.PROXYCONFIG_MANUAL,
- "network.proxy.http": "http://www.mozilla.org",
+ "network.proxy.http": "www.mozilla.org",
"network.proxy.http_port": 0,
"network.proxy.autoconfig_url": "",
+ },
+ {
+ proxyType: "manual",
+ http: "www.mozilla.org",
+ autoConfigUrl: "",
+ }
+ );
+
+ // When using proxyAll, we expect all proxies to be set to
+ // be the same as http.
+ await testProxy(
+ {
+ proxyType: "manual",
+ http: "http://www.mozilla.org:8080",
+ ftp: "http://www.mozilla.org:1234",
+ httpProxyAll: true,
+ },
+ {
+ "network.proxy.type": proxySvc.PROXYCONFIG_MANUAL,
+ "network.proxy.http": "www.mozilla.org",
+ "network.proxy.http_port": 8080,
+ "network.proxy.ftp": "www.mozilla.org",
+ "network.proxy.ftp_port": 8080,
+ "network.proxy.ssl": "www.mozilla.org",
+ "network.proxy.ssl_port": 8080,
+ "network.proxy.socks": "www.mozilla.org",
+ "network.proxy.socks_port": 8080,
+ "network.proxy.share_proxy_settings": true,
+ },
+ {
+ proxyType: "manual",
+ http: "www.mozilla.org:8080",
+ ftp: "www.mozilla.org:8080",
+ ssl: "www.mozilla.org:8080",
+ socks: "www.mozilla.org:8080",
+ httpProxyAll: true,
}
);
await testProxy(
{
proxyType: "manual",
- http: "http://www.mozilla.org:8080",
- httpProxyAll: true,
- },
- {
- "network.proxy.type": proxySvc.PROXYCONFIG_MANUAL,
- "network.proxy.http": "http://www.mozilla.org",
- "network.proxy.http_port": 8080,
- "network.proxy.share_proxy_settings": true,
- }
- );
-
- await testProxy(
- {
- proxyType: "manual",
- http: "http://www.mozilla.org:8080",
+ http: "www.mozilla.org:8080",
httpProxyAll: false,
- ftp: "http://www.mozilla.org:8081",
- ssl: "http://www.mozilla.org:8082",
+ ftp: "www.mozilla.org:8081",
+ ssl: "www.mozilla.org:8082",
socks: "mozilla.org:8083",
socksVersion: 4,
passthrough: ".mozilla.org",
},
{
"network.proxy.type": proxySvc.PROXYCONFIG_MANUAL,
- "network.proxy.http": "http://www.mozilla.org",
+ "network.proxy.http": "www.mozilla.org",
"network.proxy.http_port": 8080,
"network.proxy.share_proxy_settings": false,
- "network.proxy.ftp": "http://www.mozilla.org",
+ "network.proxy.ftp": "www.mozilla.org",
"network.proxy.ftp_port": 8081,
- "network.proxy.ssl": "http://www.mozilla.org",
+ "network.proxy.ssl": "www.mozilla.org",
"network.proxy.ssl_port": 8082,
"network.proxy.socks": "mozilla.org",
"network.proxy.socks_port": 8083,
"network.proxy.socks_version": 4,
"network.proxy.no_proxies_on": ".mozilla.org",
}
);
@@ -479,25 +501,16 @@ add_task(async function test_bad_value_p
async () => {
await browser.test.assertRejects(
browser.browserSettings.proxyConfig.set({value: {
proxyType: "abc",
}}),
/abc is not a valid value for proxyType/,
"proxyConfig.set rejects with an invalid proxyType value.");
- for (let protocol of ["http", "ftp", "ssl"]) {
- let value = {proxyType: "manual"};
- value[protocol] = "abc";
- await browser.test.assertRejects(
- browser.browserSettings.proxyConfig.set({value}),
- `abc is not a valid value for ${protocol}.`,
- `proxyConfig.set rejects with an invalid ${protocol} value.`);
- }
-
await browser.test.assertRejects(
browser.browserSettings.proxyConfig.set({value: {
proxyType: "autoConfig",
}}),
/undefined is not a valid value for autoConfigUrl/,
"proxyConfig.set for type autoConfig rejects with an empty autoConfigUrl value.");
await browser.test.assertRejects(
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_proxy_settings.js
@@ -0,0 +1,79 @@
+"use strict";
+
+ChromeUtils.import("resource://gre/modules/ExtensionUtils.jsm");
+ChromeUtils.defineModuleGetter(this, "HttpServer",
+ "resource://testing-common/httpd.js");
+
+const {
+ createAppInfo,
+ promiseShutdownManager,
+ promiseStartupManager,
+} = AddonTestUtils;
+
+AddonTestUtils.init(this);
+
+createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "42");
+
+// We cannot use createHttpServer because it also messes with proxies. We want
+// httpChannel to pick up the prefs we set and use those to proxy to our server.
+// If this were to fail, we would get an error about making a request out to
+// the network.
+const proxy = new HttpServer();
+proxy.start(-1);
+proxy.registerPathHandler("/fubar", (request, response) => {
+ response.setStatusLine(request.httpVersion, 200, "OK");
+ response.write("ok");
+});
+registerCleanupFunction(() => {
+ return new Promise(resolve => {
+ proxy.stop(resolve);
+ });
+});
+
+add_task(async function test_proxy_settings() {
+ async function background(host, port) {
+ browser.webRequest.onBeforeRequest.addListener(details => {
+ browser.test.assertEq(host, details.proxyInfo.host, "proxy host matched");
+ browser.test.assertEq(port, details.proxyInfo.port, "proxy port matched");
+ }, {urls: ["http://example.com/*"]});
+ browser.webRequest.onCompleted.addListener(details => {
+ browser.test.notifyPass("proxytest");
+ }, {urls: ["http://example.com/*"]});
+ browser.webRequest.onErrorOccurred.addListener(details => {
+ browser.test.notifyFail("proxytest");
+ }, {urls: ["http://example.com/*"]});
+
+ // Wait for the settings before testing a request.
+ await browser.browserSettings.proxyConfig.set({value: {
+ proxyType: "manual",
+ http: `${host}:${port}`,
+ }});
+ browser.test.sendMessage("ready");
+ }
+
+ let extension = ExtensionTestUtils.loadExtension({
+ manifest: {
+ permissions: [
+ "proxy",
+ "webRequest",
+ "browserSettings",
+ "<all_urls>",
+ ],
+ },
+ useAddonManager: "temporary",
+ background: `(${background})("${proxy.identity.primaryHost}", ${proxy.identity.primaryPort})`,
+ });
+
+ await promiseStartupManager();
+ await extension.startup();
+ await extension.awaitMessage("ready");
+ equal(Services.prefs.getStringPref("network.proxy.http"), proxy.identity.primaryHost, "proxy address is set");
+ equal(Services.prefs.getIntPref("network.proxy.http_port"), proxy.identity.primaryPort, "proxy port is set");
+ let ok = extension.awaitFinish("proxytest");
+ let contentPage = await ExtensionTestUtils.loadContentPage("http://example.com/fubar");
+ await ok;
+
+ await contentPage.close();
+ await extension.unload();
+ await promiseShutdownManager();
+});
--- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
@@ -62,16 +62,18 @@ skip-if = (os == "win" && !debug) #Bug 1
skip-if = true # This test no longer tests what it is meant to test.
[test_ext_permission_xhr.js]
[test_ext_persistent_events.js]
[test_ext_privacy.js]
[test_ext_privacy_disable.js]
[test_ext_privacy_update.js]
[test_ext_proxy_auth.js]
[test_ext_proxy_onauthrequired.js]
+[test_ext_proxy_settings.js]
+skip-if = os == "android" # proxy settings are not supported on android
[test_ext_proxy_socks.js]
[test_ext_proxy_speculative.js]
[test_ext_redirects.js]
[test_ext_runtime_connect_no_receiver.js]
[test_ext_runtime_getBrowserInfo.js]
[test_ext_runtime_getPlatformInfo.js]
[test_ext_runtime_id.js]
[test_ext_runtime_onInstalled_and_onStartup.js]