Bug 1312152 - Stop using nsIDialogParamBlock in the client auth UI. r?keeler
nsIDialogParamBlock isn't a great API, and is best avoided.
MozReview-Commit-ID: 2B0HkKNJizo
--- a/security/manager/pki/nsNSSDialogs.cpp
+++ b/security/manager/pki/nsNSSDialogs.cpp
@@ -6,31 +6,31 @@
/*
* Dialog services for PIP.
*/
#include "nsNSSDialogs.h"
#include "mozIDOMWindow.h"
-#include "mozilla/Assertions.h"
-#include "mozilla/Casting.h"
#include "nsArray.h"
#include "nsEmbedCID.h"
+#include "nsHashPropertyBag.h"
#include "nsIDialogParamBlock.h"
#include "nsIInterfaceRequestor.h"
#include "nsIInterfaceRequestorUtils.h"
#include "nsIKeygenThread.h"
#include "nsIPromptService.h"
#include "nsIProtectedAuthThread.h"
#include "nsIWindowWatcher.h"
#include "nsIX509CertDB.h"
#include "nsIX509Cert.h"
#include "nsNSSDialogHelper.h"
#include "nsString.h"
+#include "nsVariant.h"
#define PIPSTRING_BUNDLE_URL "chrome://pippki/locale/pippki.properties"
nsNSSDialogs::nsNSSDialogs()
{
}
nsNSSDialogs::~nsNSSDialogs()
@@ -160,96 +160,100 @@ nsNSSDialogs::ChooseCertificate(nsIInter
{
NS_ENSURE_ARG_POINTER(ctx);
NS_ENSURE_ARG_POINTER(certList);
NS_ENSURE_ARG_POINTER(selectedIndex);
NS_ENSURE_ARG_POINTER(certificateChosen);
*certificateChosen = false;
- nsCOMPtr<nsIDialogParamBlock> block =
- do_CreateInstance(NS_DIALOGPARAMBLOCK_CONTRACTID);
- if (!block) {
+ nsCOMPtr<nsIMutableArray> argArray = nsArrayBase::Create();
+ if (!argArray) {
return NS_ERROR_FAILURE;
}
- // SetObjects() expects an nsIMutableArray, which is why we can't directly use
- // |certList| and have to add an extra layer of indirection.
- nsCOMPtr<nsIMutableArray> paramBlockArray = nsArrayBase::Create();
- if (!paramBlockArray) {
- return NS_ERROR_FAILURE;
- }
- nsresult rv = paramBlockArray->AppendElement(certList, false);
+ nsCOMPtr<nsIWritableVariant> hostnameVariant = new nsVariant();
+ nsresult rv = hostnameVariant->SetAsAUTF8String(hostname);
if (NS_FAILED(rv)) {
return rv;
}
- rv = block->SetObjects(paramBlockArray);
+ rv = argArray->AppendElement(hostnameVariant, false);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+
+ nsCOMPtr<nsIWritableVariant> organizationVariant = new nsVariant();
+ rv = organizationVariant->SetAsAUTF8String(organization);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+ rv = argArray->AppendElement(organizationVariant, false);
if (NS_FAILED(rv)) {
return rv;
}
- rv = block->SetNumberStrings(3);
+ nsCOMPtr<nsIWritableVariant> issuerOrgVariant = new nsVariant();
+ rv = issuerOrgVariant->SetAsAUTF8String(issuerOrg);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+ rv = argArray->AppendElement(issuerOrgVariant, false);
if (NS_FAILED(rv)) {
return rv;
}
- rv = block->SetString(0, NS_ConvertUTF8toUTF16(hostname).get());
+ nsCOMPtr<nsIWritableVariant> portVariant = new nsVariant();
+ rv = portVariant->SetAsInt32(port);
if (NS_FAILED(rv)) {
return rv;
}
- rv = block->SetString(1, NS_ConvertUTF8toUTF16(organization).get());
- if (NS_FAILED(rv)) {
- return rv;
- }
- rv = block->SetString(2, NS_ConvertUTF8toUTF16(issuerOrg).get());
+ rv = argArray->AppendElement(portVariant, false);
if (NS_FAILED(rv)) {
return rv;
}
- rv = block->SetInt(0, port);
+ rv = argArray->AppendElement(certList, false);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+
+ nsCOMPtr<nsIWritablePropertyBag2> retVals = new nsHashPropertyBag();
+ rv = argArray->AppendElement(retVals, false);
if (NS_FAILED(rv)) {
return rv;
}
rv = nsNSSDialogHelper::openDialog(nullptr,
"chrome://pippki/content/clientauthask.xul",
- block);
- if (NS_FAILED(rv)) {
- return rv;
- }
-
- int32_t status;
- rv = block->GetInt(0, &status);
+ argArray);
if (NS_FAILED(rv)) {
return rv;
}
nsCOMPtr<nsIClientAuthUserDecision> extraResult = do_QueryInterface(ctx);
if (extraResult) {
- int32_t rememberSelection;
- rv = block->GetInt(2, &rememberSelection);
+ bool rememberSelection = false;
+ rv = retVals->GetPropertyAsBool(NS_LITERAL_STRING("rememberSelection"),
+ &rememberSelection);
if (NS_SUCCEEDED(rv)) {
- extraResult->SetRememberClientAuthCertificate(rememberSelection!=0);
+ extraResult->SetRememberClientAuthCertificate(rememberSelection);
}
}
- *certificateChosen = (status != 0);
+ rv = retVals->GetPropertyAsBool(NS_LITERAL_STRING("certChosen"),
+ certificateChosen);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
if (*certificateChosen) {
- int32_t index = 0;
- rv = block->GetInt(1, &index);
+ rv = retVals->GetPropertyAsUint32(NS_LITERAL_STRING("selectedIndex"),
+ selectedIndex);
if (NS_FAILED(rv)) {
return rv;
}
-
- if (index < 0) {
- MOZ_ASSERT_UNREACHABLE("Selected index should never be negative");
- return NS_ERROR_FAILURE;
- }
-
- *selectedIndex = mozilla::AssertedCast<uint32_t>(index);
}
return NS_OK;
}
NS_IMETHODIMP
nsNSSDialogs::SetPKCS12FilePassword(nsIInterfaceRequestor *ctx,
nsAString &_password,
--- a/security/manager/pki/resources/content/clientauthask.js
+++ b/security/manager/pki/resources/content/clientauthask.js
@@ -1,68 +1,94 @@
/* -*- tab-width: 2; indent-tabs-mode: nil; js-indent-level: 2 -*-
*
* 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/. */
/* import-globals-from pippki.js */
"use strict";
+/**
+ * @file Implements the functionality of clientauthask.xul: a dialog that allows
+ * a user pick a client certificate for TLS client authentication.
+ * @argument {String} window.arguments[0]
+ * The hostname of the server requesting client authentication.
+ * @argument {String} window.arguments[1]
+ * The Organization of the server cert.
+ * @argument {String} window.arguments[2]
+ * The Organization of the issuer of the server cert.
+ * @argument {Number} window.arguments[3]
+ * The port of the server.
+ * @argument {nsISupports} window.arguments[4]
+ * List of certificates the user can choose from, queryable to
+ * nsIArray<nsIX509Cert>.
+ * @argument {nsISupports} window.arguments[5]
+ * Object to set the return values of calling the dialog on, queryable
+ * to the underlying type of ClientAuthAskReturnValues.
+ */
+
+/**
+ * @typedef ClientAuthAskReturnValues
+ * @type nsIWritablePropertyBag2
+ * @property {Boolean} certChosen
+ * Set to true if the user chose a cert and accepted the dialog, false
+ * otherwise.
+ * @property {Boolean} rememberSelection
+ * Set to true if the user wanted their cert selection to be
+ * remembered, false otherwise.
+ * @property {Number} selectedIndex
+ * The index the chosen cert is at for the given cert list. Undefined
+ * value if |certChosen| is not true.
+ */
+
const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
const { Services } = Cu.import("resource://gre/modules/Services.jsm", {});
/**
* The pippki <stringbundle> element.
* @type <stringbundle>
*/
var bundle;
/**
* The array of certs the user can choose from.
* @type nsIArray<nsIX509Cert>
*/
var certArray;
/**
- * The param block to get params from and set results on.
- * @type nsIDialogParamBlock
- */
-var dialogParams;
-/**
* The checkbox storing whether the user wants to remember the selected cert.
* @type nsIDOMXULCheckboxElement
*/
var rememberBox;
function onLoad() {
- dialogParams = window.arguments[0].QueryInterface(Ci.nsIDialogParamBlock);
-
bundle = document.getElementById("pippki_bundle");
let rememberSetting =
Services.prefs.getBoolPref("security.remember_cert_checkbox_default_setting");
rememberBox = document.getElementById("rememberBox");
rememberBox.label = bundle.getString("clientAuthRemember");
rememberBox.checked = rememberSetting;
- let hostname = dialogParams.GetString(0);
- let org = dialogParams.GetString(1);
- let issuerOrg = dialogParams.GetString(2);
- let port = dialogParams.GetInt(0);
+ let hostname = window.arguments[0];
+ let org = window.arguments[1];
+ let issuerOrg = window.arguments[2];
+ let port = window.arguments[3];
let formattedOrg = bundle.getFormattedString("clientAuthMessage1", [org]);
let formattedIssuerOrg = bundle.getFormattedString("clientAuthMessage2",
[issuerOrg]);
let formattedHostnameAndPort =
bundle.getFormattedString("clientAuthHostnameAndPort",
[hostname, port.toString()]);
setText("hostname", formattedHostnameAndPort);
setText("organization", formattedOrg);
setText("issuer", formattedIssuerOrg);
let selectElement = document.getElementById("nicknames");
- certArray = dialogParams.objects.queryElementAt(0, Ci.nsIArray);
+ certArray = window.arguments[4].QueryInterface(Ci.nsIArray);
for (let i = 0; i < certArray.length; i++) {
let menuItemNode = document.createElement("menuitem");
let cert = certArray.queryElementAt(i, Ci.nsIX509Cert);
let nickAndSerial =
bundle.getFormattedString("clientAuthNickAndSerial",
[cert.nickname, cert.serialNumber]);
menuItemNode.setAttribute("value", i);
menuItemNode.setAttribute("label", nickAndSerial); // This is displayed.
@@ -108,26 +134,22 @@ function setDetails() {
document.getElementById("details").value = detailLines.join("\n");
}
function onCertSelected() {
setDetails();
}
function doOK() {
- // Signal that the user accepted.
- dialogParams.SetInt(0, 1);
+ let retVals = window.arguments[5].QueryInterface(Ci.nsIWritablePropertyBag2);
+ retVals.setPropertyAsBool("certChosen", true);
let index = parseInt(document.getElementById("nicknames").value);
- // Signal the index of the selected cert in the list of cert nicknames
- // provided.
- dialogParams.SetInt(1, index);
- // Signal whether the user wanted to remember the selection.
- dialogParams.SetInt(2, rememberBox.checked);
+ retVals.setPropertyAsUint32("selectedIndex", index);
+ retVals.setPropertyAsBool("rememberSelection", rememberBox.checked);
return true;
}
function doCancel() {
- // Signal that the user cancelled.
- dialogParams.SetInt(0, 0);
- // Signal whether the user wanted to remember the "selection".
- dialogParams.SetInt(2, rememberBox.checked);
+ let retVals = window.arguments[5].QueryInterface(Ci.nsIWritablePropertyBag2);
+ retVals.setPropertyAsBool("certChosen", false);
+ retVals.setPropertyAsBool("rememberSelection", rememberBox.checked);
return true;
}
--- a/security/manager/ssl/tests/mochitest/browser/browser_clientAuth_ui.js
+++ b/security/manager/ssl/tests/mochitest/browser/browser_clientAuth_ui.js
@@ -22,39 +22,31 @@ var cert;
/**
* Opens the client auth cert chooser dialog.
*
* @param {nsIX509Cert} cert The cert to pass to the dialog for display.
* @returns {Promise}
* A promise that resolves when the dialog has finished loading, with
* an array consisting of:
* 1. The window of the opened dialog.
- * 2. The nsIDialogParamBlock passed to the dialog.
+ * 2. The return value nsIWritablePropertyBag2 passed to the dialog.
*/
function openClientAuthDialog(cert) {
- let params = Cc["@mozilla.org/embedcomp/dialogparam;1"]
- .createInstance(Ci.nsIDialogParamBlock);
-
let certList = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);
certList.appendElement(cert, false);
- let array = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);
- array.appendElement(certList, false);
- params.objects = array;
- params.SetString(0, TEST_HOSTNAME);
- params.SetString(1, TEST_ORG);
- params.SetString(2, TEST_ISSUER_ORG);
- params.SetInt(0, TEST_PORT);
-
+ let returnVals = Cc["@mozilla.org/hash-property-bag;1"]
+ .createInstance(Ci.nsIWritablePropertyBag2);
let win = window.openDialog("chrome://pippki/content/clientauthask.xul", "",
- "", params);
+ "", TEST_HOSTNAME, TEST_ORG, TEST_ISSUER_ORG,
+ TEST_PORT, certList, returnVals);
return new Promise((resolve, reject) => {
win.addEventListener("load", function onLoad() {
win.removeEventListener("load", onLoad);
- resolve([win, params]);
+ resolve([win, returnVals]);
});
});
}
/**
* Checks that the contents of the given cert chooser dialog match the details
* of build/pgo/certs/mochitest.client.
*
@@ -101,45 +93,45 @@ function checkDialogContents(win, notBef
add_task(function* setup() {
cert = certDB.findCertByNickname("test client certificate");
Assert.notEqual(cert, null, "Should be able to find the test client cert");
});
// Test that the contents of the dialog correspond to the details of the
// provided cert.
add_task(function* testContents() {
- let [win, params] = yield openClientAuthDialog(cert);
+ let [win, retVals] = yield openClientAuthDialog(cert);
checkDialogContents(win, cert.validity.notBeforeLocalTime,
cert.validity.notAfterLocalTime);
yield BrowserTestUtils.closeWindow(win);
});
// Test that the right values are returned when the dialog is accepted.
add_task(function* testAcceptDialogReturnValues() {
- let [win, params] = yield openClientAuthDialog(cert);
+ let [win, retVals] = yield openClientAuthDialog(cert);
win.document.getElementById("rememberBox").checked = true;
info("Accepting dialog");
win.document.getElementById("certAuthAsk").acceptDialog();
yield BrowserTestUtils.windowClosed(win);
- Assert.equal(params.GetInt(0), 1,
- "1 should be returned to signal user accepted");
- Assert.equal(params.GetInt(1), 0,
+ Assert.ok(retVals.get("certChosen"),
+ "Return value should signal user chose a certificate");
+ Assert.equal(retVals.get("selectedIndex"), 0,
"0 should be returned as the selected index");
- Assert.equal(params.GetInt(2), 1,
- "1 should be returned as the state of the 'Remember this " +
- "decision' checkbox");
+ Assert.ok(retVals.get("rememberSelection"),
+ "Return value should signal 'Remember this decision' checkbox was" +
+ "checked");
});
// Test that the right values are returned when the dialog is canceled.
add_task(function* testCancelDialogReturnValues() {
- let [win, params] = yield openClientAuthDialog(cert);
+ let [win, retVals] = yield openClientAuthDialog(cert);
win.document.getElementById("rememberBox").checked = false;
info("Canceling dialog");
win.document.getElementById("certAuthAsk").cancelDialog();
yield BrowserTestUtils.windowClosed(win);
- Assert.equal(params.GetInt(0), 0,
- "0 should be returned to signal user canceled");
- Assert.equal(params.GetInt(2), 0,
- "0 should be returned as the state of the 'Remember this " +
- "decision' checkbox");
+ Assert.ok(!retVals.get("certChosen"),
+ "Return value should signal user did not choose a certificate");
+ Assert.ok(!retVals.get("rememberSelection"),
+ "Return value should signal 'Remember this decision' checkbox was" +
+ "unchecked");
});