- tor-commits - lists.torproject.org

[meek/webextension] Minor Chrome compatibility.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit b950f1dd8b754457a17ecfdaae8a8683f7d3602f Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 02:02:09 2019 -0600 Minor Chrome compatibility. This doesn't hurt, but we're still reliant on Firefox for using the global "browser" object (instead of "chrome"), and especially for the proxy code, which has a totally different API in Chrome. --- webextension/browser/background.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/webextension/browser/background.js b/webextension/browser/background.js index c47d06b..0b896e9 100644 --- a/webextension/browser/background.js +++ b/webextension/browser/background.js @@ -336,7 +336,8 @@ port.onDisconnect.addListener(p => { // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/… // "Note that in Google Chrome port.error is not supported: instead, use // runtime.lastError to get the error message." - if (p.error) { - console.log(`${browser.runtime.id}: disconnected because of error: ${p.error.message}`); + let error = p.error || browser.runtime.lastError; + if (error) { + console.log(`${browser.runtime.id}: disconnected because of error: ${error.message}`); } });

1 0

[meek/webextension] Throw an exception if headersFn is called more than once.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit a684d5a7da79481ac1841b4e66b2b41a5fe0c88b Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 02:09:06 2019 -0600 Throw an exception if headersFn is called more than once. Re-use the catch handler. --- webextension/browser/background.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/webextension/browser/background.js b/webextension/browser/background.js index b8c5197..737c97c 100644 --- a/webextension/browser/background.js +++ b/webextension/browser/background.js @@ -168,8 +168,7 @@ async function roundtrip(params) { try { // Sanity assertion: per-request listeners are called at most once. if (headersCalled) { - console.log("headersFn called more than once"); - return {cancel: true}; + throw new Error("headersFn called more than once"); } headersCalled = true;

1 0

[meek/webextension] Refactoring, use a Request object.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit 4fb5ca496d2fb6b3f96114be3a85c9fcde7f0d3d Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 02:08:33 2019 -0600 Refactoring, use a Request object. --- webextension/browser/background.js | 84 +++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/webextension/browser/background.js b/webextension/browser/background.js index 0b896e9..b8c5197 100644 --- a/webextension/browser/background.js +++ b/webextension/browser/background.js @@ -124,42 +124,38 @@ function Mutex() { const headersMutex = new Mutex(); const proxyMutex = new Mutex(); -async function roundtrip(request) { - // Process the incoming request spec and convert it into parameters to the - // fetch API. Also enforce some restrictions on what kinds of requests we - // are willing to make. - // https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/… - let url; - let init = {}; +async function roundtrip(params) { + // Process the incoming request parameters and convert them into a Request. + // https://developer.mozilla.org/en-US/docs/Web/API/Request/Request#Parameters + let input = params.url; + let init = { + method: params.method, + body: params.body != null ? base64_decode(params.body) : undefined, + // headers will get further treatment below in headersFn. + headers: params.header != null ? params.header : {}, + // Do not read nor write from the browser's HTTP cache. + cache: "no-store", + // Don't send cookies. + credentials: "omit", + // Don't follow redirects (we'll get resp.status:0 if there is one). + redirect: "manual", + }; - if (request.url == null) { + // Also enforce restrictions on what kinds of requests we are willing to + // make. + if (input == null) { throw new Error("request spec failed validation: missing \"url\""); } - if (!(request.url.startsWith("http://") || request.url.startsWith("https://"))) { + if (!(input.startsWith("http://") || input.startsWith("https://"))) { throw new Error("request spec failed validation: only http and https URLs are allowed"); } - url = request.url; - - if (request.method !== "POST") { + if (init.method !== "POST") { throw new Error("request spec failed validation: only POST is allowed"); } - init.method = request.method; - - // Don't set init.headers; that is handled in the onBeforeSendHeaders listener. - - if (request.body != null && request.body !== "") { - init.body = base64_decode(request.body); - } - - // Do not read nor write from the browser's HTTP cache. - init.cache = "no-store"; - // Don't send cookies. - init.credentials = "omit"; - // Don't follow redirects (we'll get resp.status:0 if there is one). - init.redirect = "manual"; + let request = new Request(input, init); // We need to use a webRequest.onBeforeSendHeaders listener to override - // certain header fields, including Host (passing them to fetch in + // certain header fields, including Host (creating a Request with them in // init.headers does not work). But onBeforeSendHeaders is a global setting // (applies to all requests) and we need to be able to set different headers // per request. We make it so that any onBeforeSendHeaders listener is only @@ -177,21 +173,21 @@ async function roundtrip(request) { } headersCalled = true; - // Convert request.header from object to array form. - // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/… - let headers = Object.entries(request.header != null ? request.header : {}) - .map(x => ({name: x[0], value: x[1]})); - // Remove all browser headers that conflict with requested headers. - let overrides = {}; - for (let name of Object.keys(headers)) { - overrides[name.toLowerCase()] = true; + let removals = new Map(); + for (let name of Object.keys(init.headers)) { + removals.set(name.toLowerCase()); } // Also remove some unnecessary or potentially tracking-enabling headers. for (let name of ["Accept", "Accept-Language", "Cookie", "Origin", "User-Agent"]) { - overrides[name.toLowerCase()] = true; + removals.set(name.toLowerCase()); + } + let requestHeaders = details.requestHeaders.filter(header => !removals.has(header.name.toLowerCase())); + // Append the requested headers in array form. + // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/… + for (let [name, value] of Object.entries(init.headers)) { + requestHeaders.push({name, value}); } - let browserHeaders = details.requestHeaders.filter(x => !(x.name.toLowerCase() in overrides)); - return {requestHeaders: browserHeaders.concat(headers)}; + return {requestHeaders}; } catch (error) { // In case of any error in the code above, play it safe and cancel // the request. @@ -228,14 +224,14 @@ async function roundtrip(request) { } try { - // Set our listener that overrides the headers for this request. + // Set a listener that overrides the headers for this request. // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/… browser.webRequest.onBeforeSendHeaders.addListener( headersFn, {urls: ["http://*/*", "https://*/*"]}, ["blocking", "requestHeaders"] ); - // Set our listener that overrides the proxy for this request. + // Set a listener that overrides the proxy for this request. // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/… browser.proxy.onRequest.addListener( proxyFn, @@ -243,9 +239,11 @@ async function roundtrip(request) { ); // Now actually do the request and build a response object. - let resp = await fetch(url, init); - let body = await resp.arrayBuffer(); - return {status: resp.status, body: base64_encode(body)}; + let response = await fetch(request); + return { + status: response.status, + body: base64_encode(await response.arrayBuffer()), + }; } finally { // With certain errors (e.g. an invalid URL), our onBeforeSendHeaders // and onRequest listeners may never get called, and therefore never

1 0

[meek/webextension] Use const in a few more places.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit 09460ec8994fec67d4e4c1d5cf07e21161fb912b Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 11:05:49 2019 -0600 Use const in a few more places. --- webextension/browser/background.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/webextension/browser/background.js b/webextension/browser/background.js index 737c97c..487ea4e 100644 --- a/webextension/browser/background.js +++ b/webextension/browser/background.js @@ -127,8 +127,8 @@ const proxyMutex = new Mutex(); async function roundtrip(params) { // Process the incoming request parameters and convert them into a Request. // https://developer.mozilla.org/en-US/docs/Web/API/Request/Request#Parameters - let input = params.url; - let init = { + const input = params.url; + const init = { method: params.method, body: params.body != null ? base64_decode(params.body) : undefined, // headers will get further treatment below in headersFn. @@ -152,7 +152,7 @@ async function roundtrip(params) { if (init.method !== "POST") { throw new Error("request spec failed validation: only POST is allowed"); } - let request = new Request(input, init); + const request = new Request(input, init); // We need to use a webRequest.onBeforeSendHeaders listener to override // certain header fields, including Host (creating a Request with them in @@ -162,7 +162,7 @@ async function roundtrip(params) { // used for a single request, by acquiring a lock here and releasing it // within the listener itself. The lock is acquired and released before any // network communication happens; i.e., it's fast. - let headersUnlock = await headersMutex.lock(); + const headersUnlock = await headersMutex.lock(); let headersCalled = false; function headersFn(details) { try { @@ -202,7 +202,7 @@ async function roundtrip(params) { // Similarly, for controlling the proxy for each request, we set a // proxy.onRequest listener, use it for one request, then remove it. - let proxyUnlock = await proxyMutex.lock(); + const proxyUnlock = await proxyMutex.lock(); let proxyCalled = false; // async to make exceptions visible to proxy.onError. // https://bugzilla.mozilla.org/show_bug.cgi?id=1528873#c1 @@ -303,7 +303,7 @@ browser.webRequest.onErrorOccurred.addListener( browser.proxy.settings.set({value: {proxyType: "system", proxyDNS: false}}); // Connect to our native process. -let port = browser.runtime.connectNative("meek.http.helper"); +const port = browser.runtime.connectNative("meek.http.helper"); port.onMessage.addListener(message => { switch (message.command) {

1 0

[meek/webextension] Refactoring WebExtension native component.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit 8aad9f71207cd878a55f3e4046b9716386858025 Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 13:29:22 2019 -0600 Refactoring WebExtension native component. The native component actually doesn't need to introspect the objects it passes to the browser, except for the "error" property of a response when it wants to report its own error. --- webextension/native/main.go | 58 +++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 36 deletions(-) diff --git a/webextension/native/main.go b/webextension/native/main.go index 3f4ec3a..d889e9c 100644 --- a/webextension/native/main.go +++ b/webextension/native/main.go @@ -36,7 +36,7 @@ const ( roundTripTimeout = 120 * time.Second // Self-defense against a malfunctioning meek-client. We'll refuse to - // read encoded responses that are longer than this. + // read encoded requests that are longer than this. maxRequestSpecLength = 1000000 // Self-defense against a malfunctioning browser. We'll refuse to @@ -52,32 +52,16 @@ const ( // inFromBrowserLoop receives a webExtensionRoundTripResponse from the browser, // it is tagged with the same ID as the corresponding request. inFromBrowserLoop // looks up the matching channel and sends the response over it. -var requestResponseMap = make(map[string]chan<- *responseSpec) +var requestResponseMap = make(map[string]chan<- responseSpec) var requestResponseMapLock sync.Mutex // A specification of an HTTP request, as received via the socket from // "meek-client --helper". -type requestSpec struct { - Method string `json:"method,omitempty"` - URL string `json:"url,omitempty"` - Header map[string]string `json:"header,omitempty"` - Body []byte `json:"body,omitempty"` - Proxy *proxySpec `json:"proxy,omitempty"` -} - -type proxySpec struct { - Type string `json:"type"` - Host string `json:"host"` - Port int `json:"port"` -} +type requestSpec interface{} // A specification of an HTTP request or an error, as sent via the socket to // "meek-client --helper". -type responseSpec struct { - Error string `json:"error,omitempty"` - Status int `json:"status,omitempty"` - Body []byte `json:"body,omitempty"` -} +type responseSpec interface{} // A "roundtrip" command sent out to the browser over the stdout stream. It // encapsulates a requestSpec as received from the socket, plus @@ -87,17 +71,17 @@ type responseSpec struct { // command:"roundtrip" is to disambiguate with the other command we may send, // "report-address". type webExtensionRoundTripRequest struct { - Command string `json:"command"` // "roundtrip" - ID string `json:"id"` - Request *requestSpec `json:"request"` + Command string `json:"command"` // "roundtrip" + ID string `json:"id"` + Request requestSpec `json:"request"` } // A message received from the the browser over the stdin stream. It // encapsulates a responseSpec along with the ID of the webExtensionResponse // that resulted in this response. type webExtensionRoundTripResponse struct { - ID string `json:"id"` - Response *responseSpec `json:"response"` + ID string `json:"id"` + Response responseSpec `json:"response"` } // Read a requestSpec (receive from "meek-client --helper"). @@ -106,7 +90,7 @@ type webExtensionRoundTripResponse struct { // protocol: a 4-byte length, followed by a JSON object of that length. The only // difference is the byte order of the length: meek-client's is big-endian, // while WebExtension's is native-endian. -func readRequestSpec(r io.Reader) (*requestSpec, error) { +func readRequestSpec(r io.Reader) (requestSpec, error) { var length uint32 err := binary.Read(r, binary.BigEndian, &length) if err != nil { @@ -132,7 +116,7 @@ func readRequestSpec(r io.Reader) (*requestSpec, error) { } // Write a responseSpec (send to "meek-client --helper"). -func writeResponseSpec(w io.Writer, spec *responseSpec) error { +func writeResponseSpec(w io.Writer, spec responseSpec) error { encodedSpec, err := json.Marshal(spec) if err != nil { panic(err) @@ -148,11 +132,7 @@ func writeResponseSpec(w io.Writer, spec *responseSpec) error { } _, err = w.Write(encodedSpec) - if err != nil { - return err - } - - return nil + return err } // Receive a WebExtension message. @@ -195,7 +175,7 @@ func sendWebExtensionMessage(w io.Writer, message []byte) error { // browser. Wait for the browser to send back a webExtensionRoundTripResponse // (which actually happens in inFromBrowserLoop--that function uses the ID to // find this goroutine again). Return a responseSpec object or an error. -func roundTrip(conn net.Conn, outToBrowserChan chan<- []byte) (*responseSpec, error) { +func roundTrip(conn net.Conn, outToBrowserChan chan<- []byte) (responseSpec, error) { err := conn.SetReadDeadline(time.Now().Add(localReadTimeout)) if err != nil { return nil, err @@ -216,7 +196,7 @@ func roundTrip(conn net.Conn, outToBrowserChan chan<- []byte) (*responseSpec, er // This is the channel over which inFromBrowserLoop will send the // response. Register it in requestResponseMap to enable // inFromBrowserLoop to match the corresponding response to it. - responseSpecChan := make(chan *responseSpec) + responseSpecChan := make(chan responseSpec) requestResponseMapLock.Lock() requestResponseMap[id] = responseSpecChan requestResponseMapLock.Unlock() @@ -235,7 +215,7 @@ func roundTrip(conn net.Conn, outToBrowserChan chan<- []byte) (*responseSpec, er // Now wait for the browser to send the response back to us. // inFromBrowserLoop will find the proper channel by looking up the ID // in requestResponseMap. - var resp *responseSpec + var resp responseSpec timeout := time.NewTimer(roundTripTimeout) select { case resp = <-responseSpecChan: @@ -251,6 +231,12 @@ func roundTrip(conn net.Conn, outToBrowserChan chan<- []byte) (*responseSpec, er return resp, err } +// This is a responseSpec for errors that originate inside this program, as +// opposed to being relayed from the browser. +type errorResponseSpec struct { + Error string `json:"error"` +} + // Handle a socket connection, which is used for one request–response roundtrip // through the browser. Delegates the real work to roundTrip, which reads the // requestSpec from the socket and sends it through the browser. Here, we wrap @@ -261,7 +247,7 @@ func handleConn(conn net.Conn, outToBrowserChan chan<- []byte) error { resp, err := roundTrip(conn, outToBrowserChan) if err != nil { - resp = &responseSpec{Error: err.Error()} + resp = &errorResponseSpec{Error: err.Error()} } // Encode the response send it back out over the socket.

1 0

[meek/webextension] Better error message.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit 1a8e2c3b9bf72c618f9ab3af0a905060795a5dc0 Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 12:05:02 2019 -0600 Better error message. --- webextension/native/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webextension/native/main.go b/webextension/native/main.go index 01053a9..2a62e04 100644 --- a/webextension/native/main.go +++ b/webextension/native/main.go @@ -223,7 +223,7 @@ func roundTrip(req requestSpec, outToBrowserChan chan<- []byte) (responseSpec, e requestResponseMapLock.Lock() delete(requestResponseMap, id) requestResponseMapLock.Unlock() - err = fmt.Errorf("timed out") + err = fmt.Errorf("timed out waiting for browser to reply") } return resp, err }

1 0

[meek/webextension] Refactoring WebExtension native component.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit f8d33f6b19afdda89d9829b59688dc60f8264aa6 Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 12:11:55 2019 -0600 Refactoring WebExtension native component. --- webextension/native/endian_386.go | 2 +- webextension/native/endian_amd64.go | 2 +- webextension/native/main.go | 77 +++++++++++++++++++++---------------- 3 files changed, 45 insertions(+), 36 deletions(-) diff --git a/webextension/native/endian_386.go b/webextension/native/endian_386.go index 6a1e44e..af343d7 100644 --- a/webextension/native/endian_386.go +++ b/webextension/native/endian_386.go @@ -5,4 +5,4 @@ package main import "encoding/binary" -var NativeEndian = binary.LittleEndian +var nativeEndian = binary.LittleEndian diff --git a/webextension/native/endian_amd64.go b/webextension/native/endian_amd64.go index 6a1e44e..af343d7 100644 --- a/webextension/native/endian_amd64.go +++ b/webextension/native/endian_amd64.go @@ -5,4 +5,4 @@ package main import "encoding/binary" -var NativeEndian = binary.LittleEndian +var nativeEndian = binary.LittleEndian diff --git a/webextension/native/main.go b/webextension/native/main.go index c2d9873..01053a9 100644 --- a/webextension/native/main.go +++ b/webextension/native/main.go @@ -2,7 +2,13 @@ // purpose is to open a localhost TCP socket for communication with meek-client // in its --helper mode (the WebExtension cannot open a socket on its own). This // program is also in charge of multiplexing the many incoming socket -// connections over the single shared stdio stream to/from the WebExtension. +// connections over the single shared stdio stream to/from the browser. +// +// This program does minimal syntax checking. Apart from ensuring that the +// messages are well-formed JSON with the correct top-level properties, and +// inspecting the ID, it doesn't care about the contents of messages. It's the +// responsibility of the browser half of the WebExtension to check everything +// else. package main @@ -44,14 +50,15 @@ const ( maxWebExtensionMessageLength = 1000000 ) -// We receive multiple (possibly concurrent) connections over our listening -// socket, and we must multiplex all their requests/responses to/from the -// browser over the single shared stdio stream. When roundTrip sends a -// webExtensionRoundTripRequest to the browser, creates a channel to receive the -// response, and stores the ID–channel mapping in requestResponseMap. When -// inFromBrowserLoop receives a webExtensionRoundTripResponse from the browser, -// it is tagged with the same ID as the corresponding request. inFromBrowserLoop -// looks up the matching channel and sends the response over it. +// We receive multiple (possibly concurrent) connections over the listening +// socket, and we must multiplex all their requests/results over the single +// shared stdio stream to the browser. When roundTrip sends a request to the +// browser, it tags the request with a random ID, creates a channel on which to +// receive the result, and stores the ID–channel mapping in requestResponseMap. +// When inFromBrowserLoop receives a webExtensionRoundTripResponse from the +// browser, it will be tagged with an ID. inFromBrowserLoop looks up the channel +// in requestResponseMap using the ID, and sends the result back on the channel +// it finds. var requestResponseMap = make(map[string]chan<- responseSpec) var requestResponseMapLock sync.Mutex @@ -84,13 +91,14 @@ type webExtensionRoundTripResponse struct { Response responseSpec `json:"response"` } -// Read a requestSpec (receive from "meek-client --helper"). +// Receive a requestSpec (from "meek-client --helper"). // // The meek-client protocol is coincidentally similar to the WebExtension stdio -// protocol: a 4-byte length, followed by a JSON object of that length. The only -// difference is the byte order of the length: meek-client's is big-endian, -// while WebExtension's is native-endian. -func readRequestSpec(r io.Reader) (requestSpec, error) { +// protocol: a 4-byte length, followed by a JSON object of that length. The main +// differences are the byte order of the length prefix—meek-client's is +// big-endian, while WebExtension's is native-endian—and that meek-client only +// sends/receives one object per connection. +func recvRequestSpec(r io.Reader) (requestSpec, error) { var length uint32 err := binary.Read(r, binary.BigEndian, &length) if err != nil { @@ -106,8 +114,8 @@ func readRequestSpec(r io.Reader) (requestSpec, error) { return nil, err } - spec := new(requestSpec) - err = json.Unmarshal(encodedSpec, spec) + var spec requestSpec + err = json.Unmarshal(encodedSpec, &spec) if err != nil { return nil, err } @@ -115,13 +123,12 @@ func readRequestSpec(r io.Reader) (requestSpec, error) { return spec, nil } -// Write a responseSpec (send to "meek-client --helper"). -func writeResponseSpec(w io.Writer, spec responseSpec) error { +// Send a responseSpec (to "meek-client --helper"). +func sendResponseSpec(w io.Writer, spec responseSpec) error { encodedSpec, err := json.Marshal(spec) if err != nil { panic(err) } - length := len(encodedSpec) if uint64(length) > math.MaxUint32 { return fmt.Errorf("response spec is too long to represent: %d", length) @@ -130,7 +137,6 @@ func writeResponseSpec(w io.Writer, spec responseSpec) error { if err != nil { return err } - _, err = w.Write(encodedSpec) return err } @@ -139,7 +145,7 @@ func writeResponseSpec(w io.Writer, spec responseSpec) error { // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Nati… func recvWebExtensionMessage(r io.Reader) ([]byte, error) { var length uint32 - err := binary.Read(r, NativeEndian, &length) + err := binary.Read(r, nativeEndian, &length) if err != nil { return nil, err } @@ -161,7 +167,7 @@ func sendWebExtensionMessage(w io.Writer, message []byte) error { if uint64(length) > math.MaxUint32 { return fmt.Errorf("WebExtension message is too long to represent: %d", length) } - err := binary.Write(w, NativeEndian, uint32(length)) + err := binary.Write(w, nativeEndian, uint32(length)) if err != nil { return err } @@ -236,17 +242,20 @@ type errorResponseSpec struct { func handleConn(conn net.Conn, outToBrowserChan chan<- []byte) error { defer conn.Close() + // Read and decode the request from the socket. err := conn.SetReadDeadline(time.Now().Add(localReadTimeout)) if err != nil { return err } - req, err := readRequestSpec(conn) + req, err := recvRequestSpec(conn) if err != nil { return err } + // Pass the parameters to the browser and get the result. resp, err := roundTrip(req, outToBrowserChan) if err != nil { + // In case of error in roundTrip, cook up an error response. resp = &errorResponseSpec{Error: err.Error()} } @@ -255,7 +264,7 @@ func handleConn(conn net.Conn, outToBrowserChan chan<- []byte) error { if err != nil { return err } - return writeResponseSpec(conn, resp) + return sendResponseSpec(conn, resp) } // Receive socket connections and dispatch them to handleConn. @@ -300,15 +309,15 @@ func inFromBrowserLoop() error { delete(requestResponseMap, resp.ID) requestResponseMapLock.Unlock() - if !ok { - // Either the browser made up an ID that we never sent - // it, or (more likely) it took too long and roundTrip - // stopped waiting. Just drop the response on the floor. - continue + if ok { + responseSpecChan <- resp.Response + // Each socket Conn is good for one request–response exchange only. + close(responseSpecChan) } - responseSpecChan <- resp.Response - // Each socket Conn is good for one request–response exchange only. - close(responseSpecChan) + // If !ok, it means that either the browser made up an ID that + // we never sent it, or (more likely) it took too long and + // roundTrip stopped waiting. In this case there's nothing to + // do. } } @@ -350,7 +359,7 @@ func main() { go func() { err := inFromBrowserLoop() if err == io.EOF { - // EOF is not an error. + // EOF is not an error to display. err = nil } errChan <- err @@ -370,7 +379,7 @@ func main() { outToBrowserChan <- message // We quit when we receive a SIGTERM, or when our stdin is closed, or - // some irrecoverable error happens. + // some unrecoverable error happens. // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Nati… signal.Notify(signalChan, syscall.SIGTERM) select {

1 0

[meek/webextension] Read the requestSpec outside of roundTrip.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit 25ecd3e29b6699dc43970d0f4e4813651c64cf15 Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 12:10:19 2019 -0600 Read the requestSpec outside of roundTrip. The difference here is that we don't send a JSON error response as a result of errors at this stage. --- webextension/native/main.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/webextension/native/main.go b/webextension/native/main.go index d889e9c..c2d9873 100644 --- a/webextension/native/main.go +++ b/webextension/native/main.go @@ -175,19 +175,10 @@ func sendWebExtensionMessage(w io.Writer, message []byte) error { // browser. Wait for the browser to send back a webExtensionRoundTripResponse // (which actually happens in inFromBrowserLoop--that function uses the ID to // find this goroutine again). Return a responseSpec object or an error. -func roundTrip(conn net.Conn, outToBrowserChan chan<- []byte) (responseSpec, error) { - err := conn.SetReadDeadline(time.Now().Add(localReadTimeout)) - if err != nil { - return nil, err - } - req, err := readRequestSpec(conn) - if err != nil { - return nil, err - } - +func roundTrip(req requestSpec, outToBrowserChan chan<- []byte) (responseSpec, error) { // Generate an ID that will allow us to match a response to this request. idRaw := make([]byte, 8) - _, err = rand.Read(idRaw) + _, err := rand.Read(idRaw) if err != nil { return nil, err } @@ -245,7 +236,16 @@ type errorResponseSpec struct { func handleConn(conn net.Conn, outToBrowserChan chan<- []byte) error { defer conn.Close() - resp, err := roundTrip(conn, outToBrowserChan) + err := conn.SetReadDeadline(time.Now().Add(localReadTimeout)) + if err != nil { + return err + } + req, err := readRequestSpec(conn) + if err != nil { + return err + } + + resp, err := roundTrip(req, outToBrowserChan) if err != nil { resp = &errorResponseSpec{Error: err.Error()} }

1 0

[meek/webextension] These can be ordinary error returns, not panics.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit 5b96265e01cf5e82b9e82f9e9e05441ac5311245 Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 13:10:33 2019 -0600 These can be ordinary error returns, not panics. --- webextension/native/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webextension/native/main.go b/webextension/native/main.go index 2a62e04..c58fd52 100644 --- a/webextension/native/main.go +++ b/webextension/native/main.go @@ -127,7 +127,7 @@ func recvRequestSpec(r io.Reader) (requestSpec, error) { func sendResponseSpec(w io.Writer, spec responseSpec) error { encodedSpec, err := json.Marshal(spec) if err != nil { - panic(err) + return err } length := len(encodedSpec) if uint64(length) > math.MaxUint32 { @@ -205,7 +205,7 @@ func roundTrip(req requestSpec, outToBrowserChan chan<- []byte) (responseSpec, e Request: req, }) if err != nil { - panic(err) + return nil, err } outToBrowserChan <- message

1 0

[meek/webextension] Cancel onBeforeSendHeaders in case of an exception.
by dcf＠torproject.org 30 Mar '19

30 Mar '19

commit b055968d6b845e20b2b8185354695856375d635b Author: David Fifield <david(a)bamsoftware.com> Date: Sat Mar 30 01:29:59 2019 -0600 Cancel onBeforeSendHeaders in case of an exception. A typo during debugging cause the callback to crash and have no effect. --- webextension/browser/background.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/webextension/browser/background.js b/webextension/browser/background.js index d26b744..c47d06b 100644 --- a/webextension/browser/background.js +++ b/webextension/browser/background.js @@ -192,6 +192,11 @@ async function roundtrip(request) { } let browserHeaders = details.requestHeaders.filter(x => !(x.name.toLowerCase() in overrides)); return {requestHeaders: browserHeaders.concat(headers)}; + } catch (error) { + // In case of any error in the code above, play it safe and cancel + // the request. + console.log(`${browser.runtime.id}: error in onBeforeSendHeaders: ${error.message}`); + return {cancel: true}; } finally { // Now that the listener has been called, remove it and release the // lock to allow the next request to set a different listener.

1 0