Bug 1337743 - Handle corrupt packets to Marionette; r=whimboo, a=test-only
authorAndreas Tolfsen <ato@mozilla.com>
Thu, 09 Feb 2017 18:29:25 +0000
changeset 395552 a775ef464ad07f7f6566631dfa53a8b825600572
parent 395551 f53f188e2a0885ad8a6d6d3057af9baa1c263613
child 395553 04e6b63a88126a3130ebee1c4dd1b20c734ae066
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswhimboo, test-only
bugs1337743
milestone54.0a2
Bug 1337743 - Handle corrupt packets to Marionette; r=whimboo, a=test-only When Marionette receives packets it does not know how to deal with, handle this gracefully and attempt to respond to the client that we were unable to process them. If it receives a packet that is corrupt, e.g. one it is impossible to determine the message ID of, report an error to the console without failing spectacularly. If it receives a packet that it is unable to unmarshal, attempt to craft a response for it with containing the error signature. MozReview-Commit-ID: BLi8yIkGQfF
testing/marionette/server.js
--- a/testing/marionette/server.js
+++ b/testing/marionette/server.js
@@ -429,24 +429,43 @@ server.TCPConnection = class {
    * a Command, the corresponding is executed.
    *
    * @param {Array.<number, number, ?, ?>} data
    *     A four element array where the elements, in sequence, signifies
    *     message type, message ID, method name or error, and parameters
    *     or result.
    */
   onPacket (data) {
-    let msg = Message.fromMsg(data);
-    msg.origin = MessageOrigin.Client;
-    this.log_(msg);
+    // unable to determine how to respond
+    if (!Array.isArray(data)) {
+      let e = new TypeError(
+          "Unable to unmarshal packet data: " + JSON.stringify(data));
+      error.report(e);
+      return;
+    }
 
+    // return immediately with any error trying to unmarshal message
+    let msg;
+    try {
+      msg = Message.fromMsg(data);
+      msg.origin = MessageOrigin.Client;
+      this.log_(msg);
+    } catch (e) {
+      let resp = this.createResponse(data[1]);
+      resp.sendError(e);
+      return;
+    }
+
+    // look up previous command we received a response for
     if (msg instanceof Response) {
       let cmd = this.commands_.get(msg.id);
       this.commands_.delete(msg.id);
       cmd.onresponse(msg);
+
+    // execute new command
     } else if (msg instanceof Command) {
       this.lastID = msg.id;
       this.execute(msg);
     }
   }
 
   /**
    * Executes a WebDriver command and sends back a response when it has
@@ -464,21 +483,21 @@ server.TCPConnection = class {
    * Errors thrown in commands are marshaled and sent back, and if they
    * are not WebDriverError instances, they are additionally propagated
    * and reported to {@code Components.utils.reportError}.
    *
    * @param {Command} cmd
    *     The requested command to execute.
    */
   execute (cmd) {
-    let resp = new Response(cmd.id, this.send.bind(this));
+    let resp = this.createResponse(cmd.id);
     let sendResponse = () => resp.sendConditionally(resp => !resp.sent);
     let sendError = resp.sendError.bind(resp);
 
-    let req = Task.spawn(function*() {
+    let req = Task.spawn(function* () {
       let fn = this.driver.commands[cmd.name];
       if (typeof fn == "undefined") {
         throw new UnknownCommandError(cmd.name);
       }
 
       if (cmd.name !== "newSession") {
         assert.session(this.driver);
       }
@@ -492,16 +511,32 @@ server.TCPConnection = class {
           resp.body = rv;
         }
       }
     }.bind(this));
 
     req.then(sendResponse, sendError).catch(error.report);
   }
 
+  /**
+   * Fail-safe creation of a new instance of |message.Response|.
+   *
+   * @param {?} msgID
+   *     Message ID to respond to.  If it is not a number, -1 is used.
+   *
+   * @return {message.Response}
+   *     Response to the message with |msgID|.
+   */
+  createResponse (msgID) {
+    if (typeof msgID != "number") {
+      msgID = -1;
+    }
+    return new Response(msgID, this.send.bind(this));
+  }
+
   sendError (err, cmdID) {
     let resp = new Response(cmdID, this.send.bind(this));
     resp.sendError(err);
   }
 
   /**
    * When a client connects we send across a JSON Object defining the
    * protocol level.