Make HSTS headers not clobber preloaded pins.

chrome/browser/resources/net_internals/hsts_view.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * HSTS is HTTPS Strict Transport Security: a way for sites to elect to always
  * use HTTPS. See http://dev.chromium.org/sts
  *
  * This UI allows a user to query and update the browser's list of HSTS domains.
  * It also allows users to query and update the browser's list of public key
@@ skipping 98 matching lines @@
 
       if (result.result == false) {
         this.queryOutputDiv_.innerHTML = '<b>Not found</b>';
         yellowFade(this.queryOutputDiv_);
         return;
       }
 
       this.queryOutputDiv_.innerHTML = '';
 
       var s = addNode(this.queryOutputDiv_, 'span');
-      s.innerHTML = '<b>Found</b>: mode: ';
+      s.innerHTML = '<b>Found:</b><br/>';
+      var t;

[eroman, 2014/05/07 21:17:16]

Is there any point to the variables "t" and "b"? They are assigned however never
read from. Just remove them.

If you do need them, then at a minimum choose better names :)

+      var b;
 
-      // TODO(palmer): Combine these 2-line pairs into 1:
-      // addNodeWithText(this.queryOutputDiv_, 'tt', results.sts_observed);
-      var t = addNode(this.queryOutputDiv_, 'tt');
-      t.textContent = modeToString(result.mode);
+      addTextNode(this.queryOutputDiv_, ' domain: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt', result.domain);
+      b = addNode(this.queryOutputDiv_, 'br');
 
-      addTextNode(this.queryOutputDiv_, ' sts_include_subdomains:');
+      addTextNode(this.queryOutputDiv_, ' static_upgrade_mode: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          modeToString(result.static_upgrade_mode));
+      b = addNode(this.queryOutputDiv_, 'br');
 
-      t = addNode(this.queryOutputDiv_, 'tt');
-      t.textContent = result.sts_subdomains;
+      addTextNode(this.queryOutputDiv_, ' static_sts_include_subdomains: ');

[eroman, 2014/05/07 21:17:16]

I feel like this code has reached a critical mass of values that it should be
refactored to be data driven:

var entries = {
  "domain": result.domain,
  "static_upgrade_mode": modeToString(result.static_upgrade_mode),
  "static_sts_include_subdomains", result.static_sts_include_subdomains
  // So on and so forth...
};

for (var key in entries) {
  var value = entries[key];
  addTextNode(this.queryOutputDiv_, ' ' + key + ': ');
  addNodeWithText(this.queryOutputDiv_, 'tt', value)
  addNode(this.queryOutputDiv_, 'br');
}

Or alternately if the intent is to pretty print everything in the result object
perhaps something like this would work, and save having to update this as much
in the future when the result object changes:

for (var key in result) {
  var value = result[key];
  
  // Pretty print some values differently...
  if (key == 'static_upgrade_mode' || key == 'dynamic_upgrade_mode') {
    value = modeToString(value);
  }

  addTextNode(this.queryOutputDiv_, ' ' + key + ': ');
  addNodeWithText(this.queryOutputDiv_, 'tt', value)
  addNode(this.queryOutputDiv_, 'br');
}

+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.static_sts_include_subdomains);
+      b = addNode(this.queryOutputDiv_, 'br');
 
-      addTextNode(this.queryOutputDiv_, ' pkp_include_subdomains:');
+      addTextNode(this.queryOutputDiv_, ' static_pkp_include_subdomains: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.static_pkp_include_subdomains);
+      b = addNode(this.queryOutputDiv_, 'br');
 
-      t = addNode(this.queryOutputDiv_, 'tt');
-      t.textContent = result.pkp_subdomains;
+      addTextNode(this.queryOutputDiv_, ' static_sts_observed: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.static_sts_observed);
+      b = addNode(this.queryOutputDiv_, 'br');
 
-      addTextNode(this.queryOutputDiv_, ' sts_observed:');
+      addTextNode(this.queryOutputDiv_, ' static_pkp_observed: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.static_pkp_observed);
+      b = addNode(this.queryOutputDiv_, 'br');
 
-      t = addNode(this.queryOutputDiv_, 'tt');
-      t.textContent = result.sts_observed;
-
-      addTextNode(this.queryOutputDiv_, ' pkp_observed:');
-
-      t = addNode(this.queryOutputDiv_, 'tt');
-      t.textContent = result.pkp_observed;
-
-      addTextNode(this.queryOutputDiv_, ' domain:');
-
-      t = addNode(this.queryOutputDiv_, 'tt');
-      t.textContent = result.domain;
-
-      addTextNode(this.queryOutputDiv_, ' pubkey_hashes:');
-
+      addTextNode(this.queryOutputDiv_, ' static_spki_hashes: ');
       t = addNode(this.queryOutputDiv_, 'tt');
 
       // |public_key_hashes| is an old synonym for what is now
       // |preloaded_spki_hashes|, which in turn is a legacy synonym for
-      // |static_spki_hashes|. Look for all three, and also for
-      // |dynamic_spki_hashes|.
+      // |static_spki_hashes|.
       if (typeof result.public_key_hashes === 'undefined')
         result.public_key_hashes = '';
       if (typeof result.preloaded_spki_hashes === 'undefined')
         result.preloaded_spki_hashes = '';
       if (typeof result.static_spki_hashes === 'undefined')
         result.static_spki_hashes = '';
-      if (typeof result.dynamic_spki_hashes === 'undefined')
-        result.dynamic_spki_hashes = '';
 
       var hashes = [];
       if (result.public_key_hashes)
         hashes.push(result.public_key_hashes);
       if (result.preloaded_spki_hashes)
         hashes.push(result.preloaded_spki_hashes);
       if (result.static_spki_hashes)
         hashes.push(result.static_spki_hashes);
-      if (result.dynamic_spki_hashes)
-        hashes.push(result.dynamic_spki_hashes);
 
       t.textContent = hashes.join(',');
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      addTextNode(this.queryOutputDiv_, ' dynamic_upgrade_mode: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          modeToString(result.dynamic_upgrade_mode));
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      addTextNode(this.queryOutputDiv_, ' dynamic_sts_include_subdomains: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.dynamic_sts_include_subdomains || '');
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      addTextNode(this.queryOutputDiv_, ' dynamic_pkp_include_subdomains: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.dynamic_pkp_include_subdomains || '');
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      addTextNode(this.queryOutputDiv_, ' dynamic_sts_observed: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.dynamic_sts_observed || '');
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      addTextNode(this.queryOutputDiv_, ' dynamic_pkp_observed: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.dynamic_pkp_observed || '');
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      addTextNode(this.queryOutputDiv_, ' dynamic_spki_hashes: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.dynamic_spki_hashes || '');
+
       yellowFade(this.queryOutputDiv_);
     }
   };
 
   function modeToString(m) {
     // These numbers must match those in
     // TransportSecurityState::DomainState::UpgradeMode.
     if (m == 0) {
       return 'STRICT';
     } else if (m == 1) {
       return 'OPPORTUNISTIC';
     } else {
       return 'UNKNOWN';
     }
   }
 
   function yellowFade(element) {
     element.style.webkitTransitionProperty = 'background-color';
     element.style.webkitTransitionDuration = '0';
     element.style.backgroundColor = '#fffccf';
     setTimeout(function() {
       element.style.webkitTransitionDuration = '1000ms';
       element.style.backgroundColor = '#fff';
     }, 0);
   }
 
   return HSTSView;
 })();