diff --git a/services/settings/Utils.jsm b/services/settings/Utils.jsm index d94be26507eb..017b518c2c18 100644 --- a/services/settings/Utils.jsm +++ b/services/settings/Utils.jsm @@ -112,12 +112,25 @@ var Utils = { async fetch(input, init = {}) { return new Promise(function(resolve, reject) { const request = new ServiceRequest(); + function fallbackOrReject(err) { + if ( + // At most one recursive Utils.fetch call (bypassProxy=false to true). + bypassProxy || + !request.isProxied || + !request.bypassProxyEnabled + ) { + reject(err); + return; + } + resolve(Utils.fetch(input, { ...init, bypassProxy: true })); + } request.onerror = () => - reject(new TypeError("NetworkError: Network request failed")); + fallbackOrReject(new TypeError("NetworkError: Network request failed")); request.ontimeout = () => - reject(new TypeError("Timeout: Network request failed")); - request.onabort = () => reject(new DOMException("Aborted", "AbortError")); + fallbackOrReject(new TypeError("Timeout: Network request failed")); + request.onabort = () => + fallbackOrReject(new DOMException("Aborted", "AbortError")); request.onload = () => { // Parse raw response headers into `Headers` object. const headers = new Headers(); @@ -141,9 +154,9 @@ var Utils = { resolve(new Response(request.response, responseAttributes)); }; - const { method = "GET", headers = {} } = init; + const { method = "GET", headers = {}, bypassProxy = false } = init; - request.open(method, input, true); + request.open(method, input, { bypassProxy }); // By default, XMLHttpRequest converts the response based on the // Content-Type header, or UTF-8 otherwise. This may mangle binary // responses. Avoid that by requesting the raw bytes. diff --git a/services/settings/test/unit/test_remote_settings_utils.js b/services/settings/test/unit/test_remote_settings_utils.js index 4e1ee45dea67..8cc5001cf7d8 100644 --- a/services/settings/test/unit/test_remote_settings_utils.js +++ b/services/settings/test/unit/test_remote_settings_utils.js @@ -3,6 +3,7 @@ const { TestUtils } = ChromeUtils.import( "resource://testing-common/TestUtils.jsm" ); +const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); const { Utils } = ChromeUtils.import("resource://services-settings/Utils.jsm"); const BinaryOutputStream = Components.Constructor( @@ -16,6 +17,12 @@ server.start(-1); registerCleanupFunction(() => server.stop(() => {})); const SERVER_BASE_URL = `http://localhost:${server.identity.primaryPort}`; +const proxyServer = new HttpServer(); +proxyServer.identity.add("http", "localhost", server.identity.primaryPort); +proxyServer.start(-1); +registerCleanupFunction(() => proxyServer.stop(() => {})); +const PROXY_PORT = proxyServer.identity.primaryPort; + // A sequence of bytes that would become garbage if it were to be read as UTF-8: // - 0xEF 0xBB 0xBF is a byte order mark. // - 0xC0 on its own is invalid (it's the first byte of a 2-byte encoding). @@ -28,6 +35,17 @@ server.registerPathHandler("/binary.dat", (request, response) => { binaryOut.writeByteArray([0xef, 0xbb, 0xbf, 0xc0]); }); +// HTTPS requests are proxied with CONNECT, but our test server is HTTP, +// which means that the proxy will receive GET http://localhost:port. +var proxiedCount = 0; +proxyServer.registerPrefixHandler("/", (request, response) => { + ++proxiedCount; + Assert.equal(request.path, "/binary.dat", `Proxy request ${proxiedCount}`); + // Close connection without sending any response. + response.seizePower(); + response.finish(); +}); + add_task(async function test_utils_fetch_binary() { let res = await Utils.fetch(`${SERVER_BASE_URL}/binary.dat`); @@ -75,3 +93,73 @@ add_task(async function test_utils_fetch_has_conservative() { let internalChannel = channel.QueryInterface(Ci.nsIHttpChannelInternal); Assert.ok(internalChannel.beConservative, "beConservative flag is set"); }); + +add_task(async function test_utils_fetch_has_conservative() { + let channelPromise = TestUtils.topicObserved("http-on-modify-request"); + await Utils.fetch(`${SERVER_BASE_URL}/binary.dat`); + + let channel = (await channelPromise)[0].QueryInterface(Ci.nsIHttpChannel); + + Assert.equal(channel.URI.spec, `${SERVER_BASE_URL}/binary.dat`, "URL OK"); + + let internalChannel = channel.QueryInterface(Ci.nsIHttpChannelInternal); + Assert.ok(internalChannel.beConservative, "beConservative flag is set"); +}); + +add_task(async function test_utils_fetch_with_bad_proxy() { + Services.prefs.setIntPref("network.proxy.type", 1); + Services.prefs.setStringPref("network.proxy.http", "127.0.0.1"); + Services.prefs.setIntPref("network.proxy.http_port", PROXY_PORT); + Services.prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true); + + // The URL that we're going to request. + const DESTINATION_URL = `${SERVER_BASE_URL}/binary.dat`; + + Assert.equal(proxiedCount, 0, "Proxy not used yet"); + { + info("Bad proxy, default prefs"); + let res = await Utils.fetch(DESTINATION_URL); + Assert.equal(res.status, 201, "Bypassed bad proxy"); + // 10 instead of 1 because of reconnect attempts after a dropped request. + Assert.equal(proxiedCount, 10, "Proxy was used by HttpChannel"); + } + + // Disables the failover logic from HttpChannel. + Services.prefs.setBoolPref("network.proxy.failover_direct", false); + proxiedCount = 0; + { + info("Bad proxy, disabled network.proxy.failover_direct"); + let res = await Utils.fetch(DESTINATION_URL); + Assert.equal(res.status, 201, "Bypassed bad proxy"); + // 10 instead of 1 because of reconnect attempts after a dropped request. + Assert.equal(proxiedCount, 10, "Proxy was used by ServiceRequest"); + } + + proxiedCount = 0; + { + info("Using internal option of Utils.fetch: bypassProxy=true"); + let res = await Utils.fetch(DESTINATION_URL, { bypassProxy: true }); + Assert.equal(res.status, 201, "Bypassed bad proxy"); + Assert.equal(proxiedCount, 0, "Not using proxy when bypassProxy=true"); + } + + // Disables the failover logic from ServiceRequest/Utils.fetch + Services.prefs.setBoolPref("network.proxy.allow_bypass", false); + proxiedCount = 0; + + info("Bad proxy, disabled network.proxy.allow_bypass"); + await Assert.rejects( + Utils.fetch(DESTINATION_URL), + /NetworkError/, + "Bad proxy request should fail without failover" + ); + // 10 instead of 1 because of reconnect attempts after a dropped request. + Assert.equal(proxiedCount, 10, "Attempted to use proxy again"); + + Services.prefs.clearUserPref("network.proxy.type"); + Services.prefs.clearUserPref("network.proxy.http"); + Services.prefs.clearUserPref("network.proxy.http_port"); + Services.prefs.clearUserPref("network.proxy.allow_hijacking_localhost"); + Services.prefs.clearUserPref("network.proxy.failover_direct"); + Services.prefs.clearUserPref("network.proxy.allow_bypass"); +}); diff --git a/toolkit/modules/ServiceRequest.jsm b/toolkit/modules/ServiceRequest.jsm index fa07d60a0dd5..6058fa1feae3 100644 --- a/toolkit/modules/ServiceRequest.jsm +++ b/toolkit/modules/ServiceRequest.jsm @@ -9,6 +9,7 @@ * can be set, Telemetry collected, etc. in a central place. */ +const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); const { Log } = ChromeUtils.import("resource://gre/modules/Log.jsm"); const { XPCOMUtils } = ChromeUtils.import( "resource://gre/modules/XPCOMUtils.jsm" @@ -47,7 +48,23 @@ class ServiceRequest extends XMLHttpRequest { // Disable cutting edge features, like TLS 1.3, where middleboxes might brick us internal.beConservative = true; // Disable use of proxy for this request if necessary. - internal.bypassProxy = options?.bypassProxy; + if (options?.bypassProxy && this.bypassProxyEnabled) { + internal.bypassProxy = true; + } } } + + get bypassProxy() { + let { channel } = this; + return channel.QueryInterface(Ci.nsIHttpChannelInternal).bypassProxy; + } + + get isProxied() { + let { channel } = this; + return !!(channel instanceof Ci.nsIProxiedChannel && channel.proxyInfo); + } + + get bypassProxyEnabled() { + return Services.prefs.getBoolPref("network.proxy.allow_bypass", true); + } } diff --git a/toolkit/modules/tests/xpcshell/test_servicerequest_xhr.js b/toolkit/modules/tests/xpcshell/test_servicerequest_xhr.js index 7775fc9857b2..fc70646d4954 100644 --- a/toolkit/modules/tests/xpcshell/test_servicerequest_xhr.js +++ b/toolkit/modules/tests/xpcshell/test_servicerequest_xhr.js @@ -28,3 +28,35 @@ add_task(async function test_tls_conservative() { "TLS setting in request channel is not set to conservative for XHR" ); }); + +add_task(async function test_bypassProxy_default() { + const request = new ServiceRequest(); + request.open("GET", "http://example.com", true); + const sr_channel = request.channel.QueryInterface(Ci.nsIHttpChannelInternal); + + ok(!sr_channel.bypassProxy, "bypassProxy is false on SR channel"); + ok(!request.bypassProxy, "bypassProxy is false for SR"); +}); + +add_task(async function test_bypassProxy_true() { + const request = new ServiceRequest(); + request.open("GET", "http://example.com", { bypassProxy: true }); + const sr_channel = request.channel.QueryInterface(Ci.nsIHttpChannelInternal); + + ok(sr_channel.bypassProxy, "bypassProxy is true on SR channel"); + ok(request.bypassProxy, "bypassProxy is true for SR"); +}); + +add_task(async function test_bypassProxy_disabled_by_pref() { + const request = new ServiceRequest(); + + ok(request.bypassProxyEnabled, "bypassProxyEnabled is true"); + Services.prefs.setBoolPref("network.proxy.allow_bypass", false); + ok(!request.bypassProxyEnabled, "bypassProxyEnabled is false"); + + request.open("GET", "http://example.com", { bypassProxy: true }); + const sr_channel = request.channel.QueryInterface(Ci.nsIHttpChannelInternal); + + ok(!sr_channel.bypassProxy, "bypassProxy is false on SR channel"); + ok(!request.bypassProxy, "bypassProxy is false for SR"); +});