commit e1ff5cdec9719853a6b0b8569b95196fe18e1bb5 Author: David Fifield david@bamsoftware.com Date: Tue Feb 19 00:58:13 2019 -0700
Detect errors of proxy.onRequest.
It turns out that if an error occurs in proxy.onRequest, Firefox will ignore it and continue on as if there were no proxy. This can happen, for example, if the "type" of a ProxyInfo isn't one of the recognized types, or if the ProxyInfo is missing a field like "host". https://bugzilla.mozilla.org/show_bug.cgi?id=1528873 To prevent silent failures like this, we register another pair of event listeners. Unlike the headers and proxy event listeners, these remain static for all requests, and are not overwritten with each new request: * proxy.onError detects when an error occurs. * webRequest.onBeforeRequest cancels all requests after an error I made the error condition be persistent because it's not something that should arise during normal operation. proxy.onError only gets called when an error occurs in proxy.onRequest, and that only happens when the proxy specification is bogus. It doesn't get called when there's an network error, for example.
In order to make exceptions in proxy.onRequest be noticed by proxy.onError, I had to make it return a rejection promise rather than throw an exception. An exception just gets logged to the browser console and nothing else. --- webextension/background.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/webextension/background.js b/webextension/background.js index 3b24a37..b43e614 100644 --- a/webextension/background.js +++ b/webextension/background.js @@ -250,6 +250,30 @@ async function roundtrip(request) { } }
+// If an error occurs in a proxy.onRequest listener (for instance if a ProxyInfo +// field is missing or invalid), the browser will ignore the proxy and just +// connect directly. It will, however, call proxy.onError listeners. Register a +// static proxy.onError listener that sets a global flag if an error ever +// occurs; and a static browser.onBeforeRequest listener which checks the flag +// and cancels every request if it is set. We could be less severe here (we +// probably only need to cancel the *next* request that occurs after a proxy +// error), but this setup is meant to be a fail-closed safety net for what is +// essentially a "can't happen" state under correct configuration. Note that +// proxy.onError doesn't get called for transient errors like a failure to +// connect to the proxy, only for nonsensical ProxyInfo configurations. +// https://bugzilla.mozilla.org/show_bug.cgi?id=1528873 +// https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/p... +let proxyError = null; +browser.proxy.onError.addListener(error => { + console.log(`proxy error, disabling: ${error.message}`); + proxyError = error; +}); +browser.webRequest.onBeforeRequest.addListener( + details => ({cancel: proxyError != null}), + {urls: ["http://*/*", "https://*/*%22%5D%7D, + ["blocking"] +); + // Connect to our native process. let port = browser.runtime.connectNative("meek.http.helper");