Closed Bug 449810 Opened 16 years ago Closed 16 years ago

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

Categories

(Toolkit :: Password Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.1a2

People

(Reporter: zpao, Assigned: zpao)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Patch v0.1 (obsolete) — Splinter Review
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-
Attached patch Patch v0.2 (obsolete) — Splinter Review
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-
Attached patch Patch v0.3Splinter Review
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
Closed: 16 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.

Attachment

General

Created:
Updated:
Size: