Clean up config merging, type check certs

 - cleans up config merging now that abr.manager has been removed
   from the config
 - uses thing.constructor == Object instead of
   typeof(thing) == 'object', for better detection of anonymous
   objects
 - adds a default (empty) server certificate for type-checking
 - treats empty certs the same as null (no cert provided)

Fixes #784

Change-Id: Ie833a1b3bf484d5f12f3ebf6d513ed51740bdc44

externs/shaka/player.js

@@ -378,8 +378,10 @@ shakaExtern.DashContentProtectionCallback;
  *   audio.
  *   <i>Defaults to '', i.e., no specific robustness required.</i> <br>
  * @property {Uint8Array} serverCertificate
- *   <i>Defaults to null, i.e., certificate will be requested from the license
- *   server if required.</i> <br>
+ *   <i>Defaults to null.</i> <br>
+ *   <i>An empty certificate (byteLength 0) will be treated as null.</i> <br>
+ *   <i>A certificate will be requested from the license server if
+ *   required.</i> <br>
  *   A key-system-specific server certificate used to encrypt license requests.
  *   Its use is optional and is meant as an optimization to avoid a round-trip
  *   to request a certificate.

lib/media/drm_engine.js

@@ -277,7 +277,8 @@ shaka.media.DrmEngine.prototype.attach = function(video) {
   });
 
   var setServerCertificate = null;
-  if (this.currentDrmInfo_.serverCertificate) {
+  if (this.currentDrmInfo_.serverCertificate &&
+      this.currentDrmInfo_.serverCertificate.length) {
     setServerCertificate = this.mediaKeys_.setServerCertificate(
         this.currentDrmInfo_.serverCertificate).then(function(supported) {
       if (!supported) {

lib/player.js

@@ -1709,7 +1709,7 @@ shaka.Player.prototype.configOverrides_ = function() {
       persistentStateRequired: false,
       videoRobustness: '',
       audioRobustness: '',
-      serverCertificate: null
+      serverCertificate: new Uint8Array(0)
     }
   };
 };

lib/util/config_utils.js

@@ -45,16 +45,6 @@ shaka.util.ConfigUtils.mergeConfigObjects =
     var subPath = path + '.' + k;
     var subTemplate = ignoreKeys ? overrides[path] : template[k];
 
-    /**
-     * @type {boolean}
-     * If true, simply copy the object over and don't verify.
-     */
-    var copyObject = !!({
-      '.abr.manager': true
-    })[subPath] || !!({
-      'serverCertificate': true
-    })[k];
-
     // The order of these checks is important.
     if (!ignoreKeys && !(k in destination)) {
       shaka.log.error('Invalid config, unrecognized key ' + subPath);
@@ -63,17 +53,33 @@ shaka.util.ConfigUtils.mergeConfigObjects =
       // destination config and replaced with a default from the template if
       // possible.
       if (subTemplate === undefined || ignoreKeys) {
+        // There is nothing in the template, so delete.
         delete destination[k];
       } else {
+        // There is something in the template, so go back to that.
         destination[k] = subTemplate;
       }
-    } else if (copyObject) {
-      destination[k] = source[k];
-    } else if (typeof destination[k] == 'object' &&
-               typeof source[k] == 'object') {
+    } else if (subTemplate.constructor == Object &&
+               source[k] &&
+               source[k].constructor == Object) {
+      // These are plain Objects with no other constructor.
+
+      if (!destination[k]) {
+        // The only way we should see a null destination is when ignoreKeys is
+        // true, so assert that it is.
+        goog.asserts.assert(ignoreKeys, 'Null destination without ignoreKeys!');
+        // Initialize the destination with the template so that normal merging
+        // and type-checking can happen.
+        destination[k] = subTemplate;
+      }
+
       shaka.util.ConfigUtils.mergeConfigObjects(
           destination[k], source[k], subTemplate, overrides, subPath);
-    } else if (typeof source[k] != typeof subTemplate) {
+    } else if (typeof source[k] != typeof subTemplate ||
+               source[k] == null ||
+               source[k].constructor != subTemplate.constructor) {
+      // The source is the wrong type.  This check allows objects to be nulled,
+      // but does not allow null for any non-object fields.
       shaka.log.error('Invalid config, wrong type for ' + subPath);
     } else if (typeof destination[k] == 'function' &&
                destination[k].length != source[k].length) {

test/media/drm_engine_unit.js

@@ -498,7 +498,7 @@ describe('DrmEngine', function() {
     });
 
     it('sets server certificate if present in config', function(done) {
-      var cert = new Uint8Array(0);
+      var cert = new Uint8Array(1);
       config.advanced['drm.abc'] = { serverCertificate: cert };
       drmEngine.configure(config);
 
@@ -509,7 +509,7 @@ describe('DrmEngine', function() {
 
     it('prefers server certificate from DrmInfo', function(done) {
       var cert1 = new Uint8Array(5);
-      var cert2 = new Uint8Array(0);
+      var cert2 = new Uint8Array(1);
       manifest.periods[0].variants[0].drmInfos[0].serverCertificate = cert1;
 
       config.advanced['drm.abc'] = { serverCertificate: cert2 };
@@ -628,7 +628,7 @@ describe('DrmEngine', function() {
     });
 
     it('fails with an error if setServerCertificate fails', function(done) {
-      var cert = new Uint8Array(0);
+      var cert = new Uint8Array(1);
       config.advanced['drm.abc'] = { serverCertificate: cert };
       drmEngine.configure(config);
 
@@ -1268,7 +1268,7 @@ describe('DrmEngine', function() {
     });
 
     it('interrupts failed calls to setServerCertificate', function(done) {
-      var cert = new Uint8Array(0);
+      var cert = new Uint8Array(1);
       config.advanced['drm.abc'] = { serverCertificate: cert };
       drmEngine.configure(config);
 
@@ -1290,7 +1290,7 @@ describe('DrmEngine', function() {
     });
 
     it('interrupts successful calls to setServerCertificate', function(done) {
-      var cert = new Uint8Array(0);
+      var cert = new Uint8Array(1);
       config.advanced['drm.abc'] = { serverCertificate: cert };
       drmEngine.configure(config);
 

test/player_unit.js

@@ -548,9 +548,9 @@ describe('Player', function() {
       });
 
       newConfig = player.getConfiguration();
-      expect(newConfig.drm.advanced).toEqual(jasmine.objectContaining({
-        'ks1': { distinctiveIdentifierRequired: true }
-      }));
+      expect(newConfig.drm.advanced).toEqual({
+        'ks1': jasmine.objectContaining({ distinctiveIdentifierRequired: true })
+      });
       expect(logErrorSpy).not.toHaveBeenCalled();
       var lastGoodConfig = newConfig;
 
@@ -668,6 +668,48 @@ describe('Player', function() {
       });
     });
 
+    it('checks the type of serverCertificate', function() {
+      logErrorSpy.and.stub();
+
+      player.configure({
+        drm: {
+          advanced: {
+            'com.widevine.alpha': {
+              serverCertificate: null
+            }
+          }
+        }
+      });
+
+      expect(logErrorSpy).toHaveBeenCalledWith(
+          stringContaining('.serverCertificate'));
+
+      logErrorSpy.calls.reset();
+      player.configure({
+        drm: {
+          advanced: {
+            'com.widevine.alpha': {
+              serverCertificate: 'foobar'
+            }
+          }
+        }
+      });
+
+      expect(logErrorSpy).toHaveBeenCalledWith(
+          stringContaining('.serverCertificate'));
+    });
+
+    it('does not throw when null appears instead of an object', function() {
+      logErrorSpy.and.stub();
+
+      player.configure({
+        drm: { advanced: null }
+      });
+
+      expect(logErrorSpy).toHaveBeenCalledWith(
+          stringContaining('.drm.advanced'));
+    });
+
     it('configures play and seek range for VOD', function(done) {
       player.configure({playRangeStart: 5, playRangeEnd: 10});
       var timeline = new shaka.media.PresentationTimeline(300, 0);