From 050db9880ae1bca0d43e879a5ca0febefbddc8a2 Mon Sep 17 00:00:00 2001
From: Dan Graham <dan@saucelabs.com>
Date: Thu, 18 Jan 2018 18:07:56 -0800
Subject: [PATCH] Fall back to MJSONWP if W3C is invalid (#10034)

* If processCapabilities throws error, then log a warning that it was invalid and log the message and fall back to using jsonwpCaps
---
 lib/utils.js             | 40 +++++++++++++++++++++++++---------------
 test/driver-e2e-specs.js | 19 +++++++++++++++++++
 test/utils-specs.js      | 17 ++++++++++++++++-
 3 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/lib/utils.js b/lib/utils.js
index 8ebd7d683..c17585f95 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -75,23 +75,33 @@ function parseCapsForInnerDriver (jsonwpCaps, w3cCapabilities, constraints={}, d
   if (hasW3CCaps) {
     // Call the process capabilities algorithm to find matching caps on the W3C
     // (see: https://github.com/jlipps/simple-wd-spec#processing-capabilities)
-    let matchingW3CCaps = processCapabilities(w3cCapabilities, constraints, true);
-    desiredCaps = matchingW3CCaps;
+    let matchingW3CCaps;
+    try {
+      matchingW3CCaps = processCapabilities(w3cCapabilities, constraints, true);
+    } catch (err) {
+      if (jsonwpCaps) {
+        logger.warn(`Could not parse W3C capabilities: ${err.message}. Falling back to JSONWP protocol.`);
+      } else {
+        throw err;
+      }
+    } finally {
+      desiredCaps = matchingW3CCaps;
 
-    // Create a new w3c capabilities payload that contains only the matching caps in `alwaysMatch`
-    processedW3CCapabilities = {
-      alwaysMatch: {...insertAppiumPrefixes(desiredCaps)},
-      firstMatch: [{}],
-    };
+      // Create a new w3c capabilities payload that contains only the matching caps in `alwaysMatch`
+      processedW3CCapabilities = {
+        alwaysMatch: {...insertAppiumPrefixes(desiredCaps)},
+        firstMatch: [{}],
+      };
 
-    // If we found extraneuous keys in JSONWP caps, fall back to JSONWP
-    if (hasJSONWPCaps) {
-      let differingKeys = _.difference(_.keys(jsonwpCaps), _.keys(matchingW3CCaps));
-      if (!_.isEmpty(differingKeys)) {
-        logger.warn(`The following capabilities were provided in the JSONWP desired capabilities that are missing ` +
-          `in W3C capabilities: ${JSON.stringify(differingKeys)}. Falling back to JSONWP protocol.`);
-        desiredCaps = jsonwpCaps;
-        processedW3CCapabilities = null;
+      // If we found extraneuous keys in JSONWP caps, fall back to JSONWP
+      if (hasJSONWPCaps) {
+        let differingKeys = _.difference(_.keys(jsonwpCaps), _.keys(matchingW3CCaps));
+        if (!_.isEmpty(differingKeys)) {
+          logger.warn(`The following capabilities were provided in the JSONWP desired capabilities that are missing ` +
+            `in W3C capabilities: ${JSON.stringify(differingKeys)}. Falling back to JSONWP protocol.`);
+          desiredCaps = jsonwpCaps;
+          processedW3CCapabilities = null;
+        }
       }
     }
   }
diff --git a/test/driver-e2e-specs.js b/test/driver-e2e-specs.js
index 3b3fe81bd..df55377a4 100644
--- a/test/driver-e2e-specs.js
+++ b/test/driver-e2e-specs.js
@@ -203,6 +203,25 @@ describe('FakeDriver - via HTTP', function () {
       should.not.exist(sessionId);
       value.capabilities.should.deep.equal(caps);
     });
+
+    it('should fall back to MJSONWP if w3c caps are invalid', async function () {
+      const combinedCaps = {
+        "desiredCapabilities": {
+          ...caps,
+        },
+        "capabilities": {
+          "alwaysMatch": {},
+          "firstMatch": [{}, {
+            ...caps,
+            deviceName: null,
+          }],
+        },
+      };
+      const {value, sessionId, status} = await request.post({url: baseUrl, json: combinedCaps});
+      status.should.exist;
+      sessionId.should.exist;
+      value.should.deep.equal(caps);
+    });
   });
 });
 
diff --git a/test/utils-specs.js b/test/utils-specs.js
index f3b01ff55..40ca3a847 100644
--- a/test/utils-specs.js
+++ b/test/utils-specs.js
@@ -34,7 +34,7 @@ describe('utils', function () {
       processedW3CCapabilities.alwaysMatch.should.deep.equal({'appium:foo': 'bar', ...insertAppiumPrefixes(BASE_CAPS)});
     });
     it('should reject if W3C caps are not passing constraints', function () {
-      (() => parseCapsForInnerDriver(BASE_CAPS, W3C_CAPS, {hello: {presence: true}})).should.throw(/'hello' can't be blank/);
+      (() => parseCapsForInnerDriver(undefined, W3C_CAPS, {hello: {presence: true}})).should.throw(/'hello' can't be blank/);
     });
     it('should only accept W3C caps that have passing constraints', function () {
       let w3cCaps = {
@@ -74,6 +74,21 @@ describe('utils', function () {
       desiredCaps.should.eql(jsonwpCaps);
       processedJsonwpCapabilities.should.eql(jsonwpCaps);
     });
+    it('should fall back to MJSONWP caps if W3C capabilities are invalid', function () {
+      let w3cCapabilities = {
+        alwaysMatch: {platformName: 'Fake', propertyName: 'PROP_NAME'},
+      };
+      let constraints = {
+        deviceName: {
+          presence: true,
+        }
+      };
+      const {desiredCaps, processedJsonwpCapabilities, processedW3CCapabilities} = parseCapsForInnerDriver({...BASE_CAPS}, w3cCapabilities, constraints);
+
+      should.not.exist(processedW3CCapabilities);
+      desiredCaps.should.eql(BASE_CAPS);
+      processedJsonwpCapabilities.should.eql(BASE_CAPS);
+    });
   });
 
   describe('insertAppiumPrefixes()', function () {