Bug 1337743 - Handle corrupt packets to Marionette; r?whimboo draft
authorAndreas Tolfsen <ato@mozilla.com>
Thu, 09 Feb 2017 18:29:25 +0000
changeset 551796 c955a1221f7ffe6bcec52928b38dec5031fc1f6c
parent 551795 29f3ebd212baa90eb75e0fb6b1af27711c32c99c
child 551797 7da5498a7bc846b51d24fa729b583dd5a611b2dc
push id51152
push userbmo:ato@mozilla.com
push dateMon, 27 Mar 2017 12:36:48 +0000
reviewerswhimboo
bugs1337743
milestone55.0a1
Bug 1337743 - Handle corrupt packets to Marionette; r?whimboo 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.