Bug 1308888 - Simplify passing handle to the cert to view in the cert viewer. r=keeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Fri, 21 Oct 2016 00:33:36 +0800
changeset 318962 7d608304562af28f6a824ab1e59a7f973b2760dd
parent 318961 2626096d67040cd7a49fb5347147157c0965d5ed
child 318963 a738ff48cc9ebf6ef010913ab238d30195c5e189
push id83047
push userryanvm@gmail.com
push dateFri, 21 Oct 2016 21:15:09 +0000
treeherdermozilla-inbound@32f298a71ce9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1308888
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1308888 - Simplify passing handle to the cert to view in the cert viewer. r=keeler The cert viewer currently supports two ways to pass a handle to the cert: 1. Passing the nickname of the cert via window.name. 2. Via an nsIDialogParamBlock, which is itself accessed through window.arguments. Method 1 is unused and unnecessary. Method 2 is overly complex: the relevant nsIX509Cert can just be passed directly. This patch does the following: 1. Makes it so that there is only a single, straightforward way to pass a handle to the cert. 2. Makes the cert viewer title localisable while we're nearby. 3. Renames viewCertDetails.js to better reflect the current use of the file. MozReview-Commit-ID: pqtfNgvImT
security/manager/locales/en-US/chrome/pippki/pippki.properties
security/manager/pki/nsNSSDialogHelper.h
security/manager/pki/nsNSSDialogs.cpp
security/manager/pki/resources/content/certViewer.js
security/manager/pki/resources/content/certViewer.xul
security/manager/pki/resources/content/viewCertDetails.js
security/manager/pki/resources/jar.mn
security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
--- a/security/manager/locales/en-US/chrome/pippki/pippki.properties
+++ b/security/manager/locales/en-US/chrome/pippki/pippki.properties
@@ -106,17 +106,19 @@ pageInfo_EncryptionWithBitsAndProtocol=C
 pageInfo_BrokenEncryption=Broken Encryption (%1$S, %2$S bit keys, %3$S)
 pageInfo_Privacy_Encrypted1=The page you are viewing was encrypted before being transmitted over the Internet.
 pageInfo_Privacy_Encrypted2=Encryption makes it difficult for unauthorized people to view information traveling between computers. It is therefore unlikely that anyone read this page as it traveled across the network.
 pageInfo_MixedContent=Connection Partially Encrypted
 pageInfo_MixedContent2=Parts of the page you are viewing were not encrypted before being transmitted over the Internet.
 pageInfo_WeakCipher=Your connection to this website uses weak encryption and is not private. Other people can view your information or modify the website’s behavior.
 
 # Cert Viewer
-certDetails=Certificate Viewer:
+# LOCALIZATION NOTE(certViewerTitle): Title used for the Certificate Viewer.
+# %1$S is a string representative of the certificate being viewed.
+certViewerTitle=Certificate Viewer: “%1$S”
 notPresent=<Not Part Of Certificate>
 
 # Token Manager
 password_not_set=(not set)
 failed_pw_change=Unable to change Master Password.
 incorrect_pw=You did not enter the correct current Master Password. Please try again.
 pw_change_ok=Master Password successfully changed.
 pw_erased_ok=Warning! You have deleted your Master Password. 
--- a/security/manager/pki/nsNSSDialogHelper.h
+++ b/security/manager/pki/nsNSSDialogHelper.h
@@ -6,22 +6,33 @@
 
 #ifndef nsNSSDialogHelper_h
 #define nsNSSDialogHelper_h
 
 class mozIDOMWindowProxy;
 class nsISupports;
 
 /**
- * Common class that uses the window watcher service to open a
- * standard dialog, with or without a parent context. The params
- * parameter can be an nsISupportsArray so any number of additional
- * arguments can be used.
+ * Helper class that uses the window watcher service to open a standard dialog,
+ * with or without a parent context.
  */
 class nsNSSDialogHelper
 {
 public:
-  // params is a nsIDialogParamBlock or a nsIKeygenThread
+  /**
+   * Opens a XUL dialog.
+   *
+   * @param window
+   *        Parent window of the dialog, or nullptr to signal no parent.
+   * @param url
+   *        URL to the XUL dialog.
+   * @param params
+   *        Parameters to pass to the dialog. Same semantics as the
+   *        nsIWindowWatcher.openWindow() |aArguments| parameter.
+   * @param modal
+   *        true if the dialog should be modal, false otherwise.
+   * @return The result of opening the dialog.
+   */
   static nsresult openDialog(mozIDOMWindowProxy* window, const char* url,
                              nsISupports* params, bool modal = true);
 };
 
-#endif
+#endif // nsNSSDialogHelper_h
--- a/security/manager/pki/nsNSSDialogs.cpp
+++ b/security/manager/pki/nsNSSDialogs.cpp
@@ -2,45 +2,38 @@
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /*
  * Dialog services for PIP.
  */
+
+#include "nsNSSDialogs.h"
+
 #include "mozIDOMWindow.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Casting.h"
 #include "nsArray.h"
-#include "nsDateTimeFormatCID.h"
 #include "nsEmbedCID.h"
-#include "nsIComponentManager.h"
-#include "nsIDateTimeFormat.h"
 #include "nsIDialogParamBlock.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIKeygenThread.h"
 #include "nsIPromptService.h"
 #include "nsIProtectedAuthThread.h"
-#include "nsIServiceManager.h"
 #include "nsIWindowWatcher.h"
 #include "nsIX509CertDB.h"
 #include "nsIX509Cert.h"
-#include "nsIX509CertValidity.h"
 #include "nsNSSDialogHelper.h"
-#include "nsNSSDialogs.h"
-#include "nsPromiseFlatString.h"
-#include "nsReadableUtils.h"
 #include "nsString.h"
 
 #define PIPSTRING_BUNDLE_URL "chrome://pippki/locale/pippki.properties"
 
-/* ==== */
-
 nsNSSDialogs::nsNSSDialogs()
 {
 }
 
 nsNSSDialogs::~nsNSSDialogs()
 {
 }
 
@@ -324,43 +317,28 @@ nsNSSDialogs::GetPKCS12FilePassword(nsII
   if (*_retval) {
     _password.Assign(pwTemp);
     free(pwTemp);
   }
 
   return NS_OK;
 }
 
-NS_IMETHODIMP 
+NS_IMETHODIMP
 nsNSSDialogs::ViewCert(nsIInterfaceRequestor* ctx, nsIX509Cert* cert)
 {
-  nsCOMPtr<nsIMutableArray> dlgArray = nsArrayBase::Create();
-  if (!dlgArray) {
-    return NS_ERROR_FAILURE;
-  }
-  nsresult rv = dlgArray->AppendElement(cert, false);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  nsCOMPtr<nsIDialogParamBlock> dlgParamBlock(
-    do_CreateInstance(NS_DIALOGPARAMBLOCK_CONTRACTID));
-  if (!dlgParamBlock) {
-    return NS_ERROR_FAILURE;
-  }
-  rv = dlgParamBlock->SetObjects(dlgArray);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
+  // |ctx| is allowed to be null.
+  NS_ENSURE_ARG(cert);
 
   // Get the parent window for the dialog
   nsCOMPtr<mozIDOMWindowProxy> parent = do_GetInterface(ctx);
   return nsNSSDialogHelper::openDialog(parent,
                                        "chrome://pippki/content/certViewer.xul",
-                                       dlgParamBlock,
-                                       false);
+                                       cert,
+                                       false /*modal*/);
 }
 
 NS_IMETHODIMP
 nsNSSDialogs::DisplayGeneratingKeypairInfo(nsIInterfaceRequestor *aCtx, nsIKeygenThread *runnable) 
 {
   nsresult rv;
 
   // Get the parent window for the dialog
rename from security/manager/pki/resources/content/viewCertDetails.js
rename to security/manager/pki/resources/content/certViewer.js
--- a/security/manager/pki/resources/content/viewCertDetails.js
+++ b/security/manager/pki/resources/content/certViewer.js
@@ -1,27 +1,34 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
+/**
+ * @file Implements functionality for certViewer.xul and its tabs certDump.xul
+ *       and viewCertDetails.xul: a dialog that allows various attributes of a
+ *       certificate to be viewed.
+ * @argument {nsISupports} window.arguments[0]
+ *           The cert to view, queryable to nsIX509Cert.
+ */
+
 const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
 const { Services } = Cu.import("resource://gre/modules/Services.jsm", {});
 
 const nsIX509Cert = Ci.nsIX509Cert;
 const nsX509CertDB = "@mozilla.org/security/x509certdb;1";
 const nsIX509CertDB = Ci.nsIX509CertDB;
 const nsPK11TokenDB = "@mozilla.org/security/pk11tokendb;1";
 const nsIPK11TokenDB = Ci.nsIPK11TokenDB;
 const nsIASN1Object = Ci.nsIASN1Object;
 const nsIASN1Sequence = Ci.nsIASN1Sequence;
 const nsIASN1PrintableItem = Ci.nsIASN1PrintableItem;
 const nsIASN1Tree = Ci.nsIASN1Tree;
 const nsASN1Tree = "@mozilla.org/security/nsASN1Tree;1";
-const nsIDialogParamBlock = Ci.nsIDialogParamBlock;
 
 var bundle;
 
 function doPrompt(msg)
 {
   let prompts = Components.classes["@mozilla.org/embedcomp/prompt-service;1"].
     getService(Components.interfaces.nsIPromptService);
   prompts.alert(window, null, msg);
@@ -66,31 +73,21 @@ function AddUsage(usage)
   text.setAttribute("style", "margin: 2px 5px");
   text.setAttribute("readonly", "true");
   text.setAttribute("class", "scrollfield");
   verifyInfoBox.appendChild(text);
 }
 
 function setWindowName()
 {
-  //  Get the cert from the cert database
-  var certdb = Components.classes[nsX509CertDB].getService(nsIX509CertDB);
-  var myName = self.name;
   bundle = document.getElementById("pippki_bundle");
-  var cert;
 
-  var certDetails = bundle.getString('certDetails');
-  if (myName != "") {
-    document.title = certDetails + '"' + myName + '"'; // XXX l10n?
-    cert = certdb.findCertByNickname(myName);
-  } else {
-    var params = window.arguments[0].QueryInterface(nsIDialogParamBlock);
-    cert = params.objects.queryElementAt(0, nsIX509Cert);
-    document.title = certDetails + '"' + cert.windowTitle + '"'; // XXX l10n?
-  }
+  let cert = window.arguments[0].QueryInterface(Ci.nsIX509Cert);
+  document.title = bundle.getFormattedString("certViewerTitle",
+                                             [cert.windowTitle]);
 
   //
   //  Set the cert attributes for viewing
   //
 
   //  The chain of trust
   AddCertChain("treesetDump", cert.getChain());
   DisplayGeneralDataFromCert(cert);
--- a/security/manager/pki/resources/content/certViewer.xul
+++ b/security/manager/pki/resources/content/certViewer.xul
@@ -15,17 +15,18 @@
   xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
   buttons="accept"
   buttonlabelaccept="&certmgr.close.label;"
   buttonaccesskeyaccept="&certmgr.close.accesskey;"
   onload="setWindowName();">
 
 <stringbundle id="pippki_bundle" src="chrome://pippki/locale/pippki.properties"/>
 
-<script type="application/javascript" src="chrome://pippki/content/viewCertDetails.js"/>
+<script type="application/javascript"
+        src="chrome://pippki/content/certViewer.js"/>
 <script type="application/javascript" src="chrome://pippki/content/pippki.js"/>
 
   <tabbox flex="1">
     <tabs>
       <tab id="general_tab" label="&certmgr.detail.general_tab.title;"
            accesskey="&certmgr.detail.general_tab.accesskey;"/>
       <tab id="prettyprint_tab" label="&certmgr.detail.prettyprint_tab.title;"
            accesskey="&certmgr.detail.prettyprint_tab.accesskey;"/>
--- a/security/manager/pki/resources/jar.mn
+++ b/security/manager/pki/resources/jar.mn
@@ -19,22 +19,22 @@ pippki.jar:
     content/pippki/OrphanOverlay.xul         (content/OrphanOverlay.xul)
     content/pippki/viewCertDetails.xul       (content/viewCertDetails.xul)
     content/pippki/editcacert.xul            (content/editcacert.xul)
     content/pippki/editcacert.js             (content/editcacert.js)
 *   content/pippki/exceptionDialog.xul       (content/exceptionDialog.xul)
     content/pippki/exceptionDialog.js        (content/exceptionDialog.js)
     content/pippki/deletecert.xul            (content/deletecert.xul)
     content/pippki/deletecert.js             (content/deletecert.js)
-    content/pippki/viewCertDetails.js        (content/viewCertDetails.js)
     content/pippki/setp12password.xul        (content/setp12password.xul)
     content/pippki/pippki.js                 (content/pippki.js)
     content/pippki/clientauthask.xul	     (content/clientauthask.xul)
     content/pippki/clientauthask.js          (content/clientauthask.js)
     content/pippki/certViewer.xul            (content/certViewer.xul)
+    content/pippki/certViewer.js             (content/certViewer.js)
     content/pippki/certDump.xul              (content/certDump.xul)
     content/pippki/device_manager.xul        (content/device_manager.xul)
     content/pippki/device_manager.js         (content/device_manager.js)
     content/pippki/load_device.xul           (content/load_device.xul)
     content/pippki/choosetoken.xul           (content/choosetoken.xul)
     content/pippki/choosetoken.js            (content/choosetoken.js)
     content/pippki/createCertInfo.xul        (content/createCertInfo.xul)
     content/pippki/createCertInfo.js         (content/createCertInfo.js)
--- a/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
+++ b/security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
@@ -5,113 +5,118 @@
 
 // Repeatedly opens the certificate viewer dialog with various certificates and
 // determines that the viewer correctly identifies either what usages those
 // certificates are valid for or what errors prevented the certificates from
 // being verified.
 
 var { OS } = Cu.import("resource://gre/modules/osfile.jsm", {});
 
-add_task(function* () {
+add_task(function* testCAandTitle() {
   let cert = yield readCertificate("ca.pem", "CTu,CTu,CTu");
   let win = yield displayCertificate(cert);
   checkUsages(win, ["SSL Certificate Authority"]);
+
+  // There's no real need to test the title for every cert, so we just test it
+  // once here.
+  Assert.equal(win.document.title, "Certificate Viewer: \u201Cca\u201D",
+               "Actual and expected title should match");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testSSLEndEntity() {
   let cert = yield readCertificate("ssl-ee.pem", ",,");
   let win = yield displayCertificate(cert);
   checkUsages(win, ["SSL Server Certificate", "SSL Client Certificate"]);
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testEmailEndEntity() {
   let cert = yield readCertificate("email-ee.pem", ",,");
   let win = yield displayCertificate(cert);
   checkUsages(win, ["Email Recipient Certificate", "Email Signer Certificate"]);
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testCodeSignEndEntity() {
   let cert = yield readCertificate("code-ee.pem", ",,");
   let win = yield displayCertificate(cert);
   checkUsages(win, ["Object Signer"]);
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testExpired() {
   let cert = yield readCertificate("expired-ca.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win, "Could not verify this certificate because it has expired.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testIssuerExpired() {
   let cert = yield readCertificate("ee-from-expired-ca.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because the CA certificate " +
              "is invalid.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testUnknownIssuer() {
   let cert = yield readCertificate("unknown-issuer.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because the issuer is " +
              "unknown.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testInsecureAlgo() {
   let cert = yield readCertificate("md5-ee.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because it was signed using " +
              "a signature algorithm that was disabled because that algorithm " +
              "is not secure.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testUntrusted() {
   let cert = yield readCertificate("untrusted-ca.pem", "p,p,p");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because it is not trusted.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testUntrustedIssuer() {
   let cert = yield readCertificate("ee-from-untrusted-ca.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because the issuer is not " +
              "trusted.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testRevoked() {
   // Note that there's currently no way to un-do this. This should only be a
   // problem if another test re-uses a certificate with this same key (perhaps
   // likely) and subject (less likely).
   let certBlocklist = Cc["@mozilla.org/security/certblocklist;1"]
                         .getService(Ci.nsICertBlocklist);
   certBlocklist.revokeCertBySubjectAndPubKey(
     "MBIxEDAOBgNVBAMMB3Jldm9rZWQ=", // CN=revoked
     "VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8="); // hash of the shared key
   let cert = yield readCertificate("revoked.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win,
              "Could not verify this certificate because it has been revoked.");
   yield BrowserTestUtils.closeWindow(win);
 });
 
-add_task(function* () {
+add_task(function* testInvalid() {
   // This certificate has a keyUsage extension asserting cRLSign and
   // keyCertSign, but it doesn't have a basicConstraints extension. This
   // shouldn't be valid for any usage. Sadly, we give a pretty lame error
   // message in this case.
   let cert = yield readCertificate("invalid.pem", ",,");
   let win = yield displayCertificate(cert);
   checkError(win, "Could not verify this certificate for unknown reasons.");
   yield BrowserTestUtils.closeWindow(win);
@@ -124,23 +129,18 @@ add_task(function* () {
  *
  * @param {nsIX509Cert} certificate
  *        The certificate to view and determine usages for.
  * @return {Promise}
  *         A promise that will resolve with a handle on the opened certificate
  *         viewer window when the usages have been determined.
  */
 function displayCertificate(certificate) {
-  let array = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);
-  array.appendElement(certificate, false);
-  let params = Cc["@mozilla.org/embedcomp/dialogparam;1"]
-                 .createInstance(Ci.nsIDialogParamBlock);
-  params.objects = array;
   let win = window.openDialog("chrome://pippki/content/certViewer.xul", "",
-                              "", params);
+                              "", certificate);
   return TestUtils.topicObserved("ViewCertDetails:CertUsagesDone",
                                  (subject, data) => subject == win)
   .then(([subject, data]) => subject, error => { throw error; });
 }
 
 /**
  * Given a certificate viewer window, finds the usages the certificate is valid
  * for.