Make HSTS headers not clobber preloaded pins.

chrome/browser/resources/net_internals/hsts_view.js

Index: chrome/browser/resources/net_internals/hsts_view.js
diff --git a/chrome/browser/resources/net_internals/hsts_view.js b/chrome/browser/resources/net_internals/hsts_view.js
index 5ed13607ffc14ccf46f6b451ad14b5f43a86b9c5..4d9c9c7f8348f878fd6677c03e570a4941a89c9f 100644
--- a/chrome/browser/resources/net_internals/hsts_view.js
+++ b/chrome/browser/resources/net_internals/hsts_view.js
@@ -116,54 +116,51 @@ var HSTSView = (function() {
       this.queryOutputDiv_.innerHTML = '';
 
       var s = addNode(this.queryOutputDiv_, 'span');
-      s.innerHTML = '<b>Found</b>: mode: ';
-
-      // 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_, ' sts_include_subdomains:');
-
-      t = addNode(this.queryOutputDiv_, 'tt');
-      t.textContent = result.sts_subdomains;
-
-      addTextNode(this.queryOutputDiv_, ' pkp_include_subdomains:');
-
-      t = addNode(this.queryOutputDiv_, 'tt');
-      t.textContent = result.pkp_subdomains;
-
-      addTextNode(this.queryOutputDiv_, ' sts_observed:');
-
-      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:');
-
+      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;
+
+      addTextNode(this.queryOutputDiv_, ' domain: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt', result.domain);
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      addTextNode(this.queryOutputDiv_, ' static_upgrade_mode: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          modeToString(result.static_upgrade_mode));
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      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_, ' static_pkp_include_subdomains: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.static_pkp_include_subdomains);
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      addTextNode(this.queryOutputDiv_, ' static_sts_observed: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.static_sts_observed);
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      addTextNode(this.queryOutputDiv_, ' static_pkp_observed: ');
+      t = addNodeWithText(this.queryOutputDiv_, 'tt',
+                          result.static_pkp_observed);
+      b = addNode(this.queryOutputDiv_, 'br');
+
+      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)
@@ -172,10 +169,39 @@ var HSTSView = (function() {
         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_);
     }
   };