legacy storage module should throw on canceled master password entry in getAllLogins

RESOLVED FIXED in mozilla1.9.1a2

Status

()

Toolkit
Password Manager
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: zpao, Assigned: zpao)

Tracking

Trunk
mozilla1.9.1a2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

In order for the new mozStorage module to handle importing correctly, the legacy module needs to throw if the master password was canceled. This allows the mozStorage module to gracefully fail, and prompt later.
Created attachment 332978 [details] [diff] [review]
Patch v0.1

patch to storage-Legacy.js
Attachment #332978 - Flags: review?(dolske)
Comment on attachment 332978 [details] [diff] [review]
Patch v0.1

Yeah, I think there also needs to be a change to LoadSignons() in toolkit/components/passwordmgr/content/passwordManager.js... The call to getAllLogins() should be wrapped in a try/catch, and if an exception is thrown |signons| should end up as an empty array (as would have happened before this change).
Attachment #332978 - Flags: review?(dolske) → review-
Created attachment 332986 [details] [diff] [review]
Patch v0.2

Updated to include the content fixes - the one mentioned above, and the other one we discussed
Attachment #332978 - Attachment is obsolete: true
Attachment #332986 - Flags: review?(dolske)
Comment on attachment 332986 [details] [diff] [review]
Patch v0.2

>+
>+        // We want to throw in this case, so that LoginManagerStorage-mozStorage
>+        // will halt importing.
>+        if (userCanceled)
>+            throw "User canceled Master Password entry";
> 
>         // decrypt entries for caller.
>         [result, userCanceled] = this._decryptLogins(result);

I think you probably want to add that *after* userCanceled is assigned a value. :) Also, no need for the comment.
Attachment #332986 - Flags: review?(dolske) → review-
Created attachment 333001 [details] [diff] [review]
Patch v0.3

I'm not even sure how that happened...
Attachment #332986 - Attachment is obsolete: true
Attachment #333001 - Flags: review?(dolske)
Attachment #333001 - Flags: review?(dolske) → review+
Pushed changeset 29f2c16ef6e2.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a2
You need to log in before you can comment on or make changes to this bug.