500822 - Importing passwords to mozstorage can fail when signons3.txt is corrupted.

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Description

[RazorBoy341]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.11) Gecko/2009060215 Firefox/3.0.11 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)

when I copy both key3.db and signons3.txt files into a new Firefox 3.5 profile, the passwords don't show up at all, even after deleting signons.sqlite to force an import.

Reproducible: Always

Steps to Reproduce:
1.Create new profile for Firefox 3.5

2.Open it, then close it.

3.Copy both key3.db and signons3.txt files from old profile into new Firefox 3.5 profile.

4.Open Firefox 3.5 and go to Tools/Options/Security/Saved Passwords and see whether any passwords are there (They won't be!)

5.Shut down Firefox 3.5, get into the profile and delete the signons.sqlite file to try to force a password import.

6.Open up, again, Firefox 3.5, go to Tools/Options/Security/Saved Passwords.
Actual Results:  
No passwords at all appear there.

Expected Results:  
The passwords should be there.

Comment 1

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

WFM here. My guess is that there are corrupted passwords in the file, though it still seems odd because we're using the old storage to get the passwords. Another possibility is that it gets stuck somewhere else along the line 

I agree with what people said in the thread on mozillazine & I'm surprised we haven't heard more issues if it's a big problem. So if you don't mind helping out a bit, and attaching some debug output, that would be really helpful. Just enable debugging (as per https://wiki.mozilla.org/Firefox:Password_Manager_Debugging) at the end of step 1. When you open Firefox 3.5 again in step 4, you should see output in the error console. You can copy all of that & paste it into a text file to attach here.

Comment 2

[Dave Buco]

This is a very sporadic, intermittent little bugger. Having tried 3 times (installing 3.5 RC3 over 3.0.11), it worked once and failed twice. Seems to have a mind of it's own. 

My wife tried the same way (installing 3.5 RC3 over 3.0.11) and it did not migrate her passwords. 

I also had a friend do the same. Had him back up his profile and attempt to install over 3.0.11. He has only 9 passwords saved in his 3.0.11 profile. His first attempt failed, second worked.

Don't know what else to say except that this is real and not corrupt passwords. My friend

Comment 3

[Dave Buco]

I did the installing of 3.5 RC3 over 3.0.11 as an initial test because that's what the vast majority of average end users will do.

Now, I also used the steps shown above. Passwords did not appear on first attempt. Tried a second time, and it worked. Again, this seems to have a mind of it's own.

Comment 4

[Justin Dolske [:Dolske]]

I'd also be curious to know if, when an update appears to fail, there's a signons.sqlite being created in the profile. And, if there is, what's in it. [Note that it's only imported on-demand, try to view passwords from Preferences to ensure that it has tried to import them.]

You can examine the DB with the SQLite Manager extension (https://addons.mozilla.org/en-US/firefox/addon/5817), or with the command-line sqlite commandline utility if you have it ("sqlite3 signons.sqlite", then type ".dump").

Comment 5

[RazorBoy341]

FYI: After further testing, I've discovered that the problem started after Firefox 3.1b2. I've also found out that the signons.sqlite file from its profile, when transfered over to the profile for Firefox 3.5rc3, works just fine when its alongside a copy of the key3.db file from your official Firefox 3 profile.

What I did:

1.Installed Firefox 3.1b2

2.Opened the browser, closed it, getting signons.sqlite file into its profile folder.

3.Copied both key3.db and signons3.txt file into profile folder for Firefox 3.1b2.

4.Opened Firefox 3.1b2, then went to Tools/Options/Security/Saved Passwords to confirm that no passwords had transfered over.

5.Closed browser, then went into profile folder to delete signons.sqlite, and then did a browser restart to successfully force a password transfer in Firefox 3.1b2.

6.Closed browser, again, then copied both key3.db (from official Firefox 3 profile) and signons.sqlite files into new Firefox 3.5rc3 profile.

7.Opened Firefox 3.5rc3, then went to Tools/Options/Security/Saved Passwords to confirm successful password transfer.

Can anybody confirm?

Comment 6

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

RazorBoy or Dave:
Can you please attach debug output as I described in comment #1. I have a suspicion as to what is happening but I can't confirm if I can't see the output.

Comment 7

[Justin Dolske [:Dolske]]

(In reply to comment #5)

> Can anybody confirm?

Confirm what? Those steps look normal to me. [I assume you're using the profile manager or -P / -profile to explicitly select a specific profile in those steps?]

Passwords are not reimported from signons*.txt if there's already a signons.sqlite present.

Comment 8

[makondo]

Attachment: passwords debug log

Comment 9

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 385833 [details]
passwords debug log

This looks like a log from a profile that already has signons.sqlite. Output that imports will look like this:

PwMgr mozStorage: Initializing Database
PwMgr mozStorage: Creating Database
PwMgr mozStorage: Creating Tables
PwMgr mozStorage: Creating Indices
PwMgr mozStorage: Importing legacy storage
PwMgr Storage: Checking file signons3.txt (SignonFileName3)
PwMgr Storage: Reading passwords from /Users/pao/Library/Application Support/Firefox/Profiles/ghj974gk.tmp.PWupgrade3.5/signons3.txt

Comment 10

[RazorBoy341]

Attachment: Shows error console output from Firefox 3.5rc3 regarding the password manager problem

Comment 11

[RazorBoy341]

Okay, created a new FF 3.5rc3 profile, turned on Password Manager Debugging, closed browser, copied both key3.db and signons3.txt files into profile, deleted the signons.sqlite file and opened the browser again to try to force a password import. Results from the error eonsole have been posted to this bug in the form of a text file (as explained in Comment #10)

Comment 12

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #10)
> Created an attachment (id=385858) [details]
> Shows error console output from Firefox 3.5rc3 regarding the password manager
> problem

Thanks for posting this. We're pretty sure this is caused by a corrupt or super old entry which is somehow getting through. This may be asking a lot, but it would be super helpful for us if we can look at your signons3.txt file. We can't decrypt it without key3.db, but we don't need to (nor do we have any intention to do so)

If you don't feel comfortable doing that, you can obfuscate your logins by looking for lines that have a bunch of random numbers & letters (those are encrypted data). Just ensure that if a line begins with a ~, you leave that there. For example "~dGVzdHVzZXIz" would become "~XXX"

There's no need for everybody to see the file, so feel free to email the file to me directly as opposed to posting it here.

Comment 13

[RazorBoy341]

(In reply to comment #12)
> (In reply to comment #10)
> > Created an attachment (id=385858) [details] [details]
> > Shows error console output from Firefox 3.5rc3 regarding the password manager
> > problem
> 
> Thanks for posting this. We're pretty sure this is caused by a corrupt or super
> old entry which is somehow getting through. This may be asking a lot, but it
> would be super helpful for us if we can look at your signons3.txt file. We
> can't decrypt it without key3.db, but we don't need to (nor do we have any
> intention to do so)
> 
> If you don't feel comfortable doing that, you can obfuscate your logins by
> looking for lines that have a bunch of random numbers & letters (those are
> encrypted data). Just ensure that if a line begins with a ~, you leave that
> there. For example "~dGVzdHVzZXIz" would become "~XXX"
> 
> There's no need for everybody to see the file, so feel free to email the file
> to me directly as opposed to posting it here.

Sent the requested file to you via e-mail.

Comment 14

[Justin Dolske [:Dolske]]

Paul and I took a look at the file.

The nutshell version is that the signons3.txt you have is corrupted, and doesn't match the expected structure. There are lines missing and unexpected extra lines. Have you ever attempted to edit the file by hand? It's easy to mess up the format.

There is a bug in the password manager, though, in that it should be better at ignoring failures when trying to import corrupted entries. Specifically, we should try/catch the addLogin call, and just ignore any failures.

I think this is unlikely to be a common problem. The code that's not catching addLogin calls has been this way since the signons.sqlite support landed in bug 288040 back in August 2008 (so, since 3.1 Alpha 2). The fix for bug 474846 should have made us a bit better at reading corrupted files, but that wasn't picked up until Beta 4. So, since this is the first report of passwords failing to import, it seems uncommon.

Paul is working on a fix, we should take it in 3.5.1 just to be safe.

Comment 15

[Justin Dolske [:Dolske]]

Just for future reference: the specific failure here was that due to the corrupted signons3.txt, the storage-Legacy parser was essentially out of sync, and was emiting a login with the formSubmitURL set to "." (a period). addLogin's call to checkLoginValues() was throwing as a result.

Comment 16

[RazorBoy341]

(In reply to comment #14)
> Paul and I took a look at the file.
> 
> The nutshell version is that the signons3.txt you have is corrupted, and
> doesn't match the expected structure. There are lines missing and unexpected
> extra lines. Have you ever attempted to edit the file by hand? It's easy to
> mess up the format.
> 
> There is a bug in the password manager, though, in that it should be better at
> ignoring failures when trying to import corrupted entries. Specifically, we
> should try/catch the addLogin call, and just ignore any failures.
> 
> I think this is unlikely to be a common problem. The code that's not catching
> addLogin calls has been this way since the signons.sqlite support landed in bug
> 288040 back in August 2008 (so, since 3.1 Alpha 2). The fix for bug 474846
> should have made us a bit better at reading corrupted files, but that wasn't
> picked up until Beta 4. So, since this is the first report of passwords failing
> to import, it seems uncommon.
> 
> Paul is working on a fix, we should take it in 3.5.1 just to be safe.

Actually, I've never touched the file. But my research shows that the same exact file worked totally fine in Firefox 3.1b2, but stopped working in versions that followed. 

In any case, I followed the steps described in Comment #5 to get a successful password import and an signons.sqlite file that works fine in Firefox 3.5 (official release version), sol the people who are having this problem might want to try that method to get around the problem until the FF 3.5.1 fix comes down the pike.

Comment 17

[Justin Dolske [:Dolske]]

(In reply to comment #16)

> Actually, I've never touched the file. But my research shows that the same
> exact file worked totally fine in Firefox 3.1b2, but stopped working in
> versions that followed. 

Probably just luck. Beta 2 doesn't have the fix for bug 474846, so the corrupted entries happen to be parsed in a different way. In this particular case, that might be just enough to let thing stumble their way to success.

Comment 18

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

Fix as discussed.

Comment 19

[Justin Dolske [:Dolske]]

Comment on attachment 386143 [details] [diff] [review]
Patch v0.1

>+            for each (let login in logins){
>+                try {
>+                    this._addLogin(login, true);
>+                } catch (e) {}
>+            }

Nit: space before "{" (first list).

Probably worth having the catch log that the addLogin call failed.


>+let matchData = Cc["@mozilla.org/hash-property-bag;1"].createInstance(Ci.nsIWritablePropertyBag2);
>+matchData.setPropertyAsAString("hostname", "http://dummyhost.mozilla.org");
>+matchData.setPropertyAsAString("usernameField", "put_user_here");
>+logins = storage.searchLogins({}, matchData);

Could have just used findLogins() here. ;-)

I assume you verified that the test fails if the patch to storage-mozStorage.js is removed?

Comment 20

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #19)
> Probably worth having the catch log that the addLogin call failed.

Should that include info about the login itself or just that we failed to import a login? Or the actual text of the error.

> Could have just used findLogins() here. ;-)

Well, yea. But due to how the corruption was manifesting I wanted an exact match on the usernameField too.

> I assume you verified that the test fails if the patch to storage-mozStorage.js
> is removed?

I actually remembered as I was walking to the train that I didn't. And it turns it doesn't. Adding the next block of text from our corrupted sample makes it fail without the patch, but pass with.

I'll upload a new patch when we finalize what goes in the log message.

Comment 21

[Justin Dolske [:Dolske]]

(In reply to comment #20)

> Should that include info about the login itself or just that we failed to
> import a login? Or the actual text of the error.

I'd just say something like log("import addLogin failed: " + e), the point just being to know that something went wrong, lest someone be investigating why something didn't seem to import.

Comment 22

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.2 (for check-in)

With fixes

Comment 23

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

pushed http://hg.mozilla.org/mozilla-central/rev/7c3deed4e33d

Comment 24

[Mark Sapiro]

I note the status of this bug is FIXED and Comment #14 implies the fix is in 3.5.1, but I have just tried upgrading from 3.0.11 to 3.5.1 and I still have the same issue. After upgrade there is no signons.sqlite. I go to Tools -> Options -> Security -> Saved Passwords and signons.sqlite and signons.sqlite-journal are created with no saved passwords.

Yes, my signons3.txt is not completely clean, but it works with 3.0.11. I have tried cleaning it by hand with no success. I thought I understood that a signons3.txt that works with 3.0 would be successfully imported into 3.5.1. Did this fix not make it into 3.5.1, or is my problem different?

Comment 25

[Justin Dolske [:Dolske]]

This fix hasn't landed on the Firefox 3.5.x branch yet.

Comment 26

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 386184 [details] [diff] [review]
Patch v0.2 (for check-in)

a1912=beltzner

Comment 27

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/feb24d49e980

Comment 28

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

I think I got this status1.9.1 flag right...

Comment 29

[u279076]

Without a "corrupt" profile in this bug, it's going to be hard for verify this for 3.5.2.  Can someone please advise QA on how to verify this for 3.5.2?

Comment 30

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

The easiest thing would be to use the test case that's checked into the tree.
1. Delete key3.db & signons* from your profile.
2. From http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/test/unit/data/ copy key3.db & signons-500822-1.txt to your profile. 3. Rename signons-500822-1.txt to signons3.txt
4. Open Firefox & show passwords from the preferences.

Firefox 3.5 (and 3.5.1) should fail to import passwords. You should see passwords in 3.5.2.

Comment 31

[u279076]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.2)
Gecko/20090729 Firefox/3.5.2
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729
Firefox/3.5.2
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.2) Gecko/20090729
Firefox/3.5.2

I see no passwords using comment 30 in any of these builds.

Comment 32

[Mark Sapiro]

(In reply to comment #31)
> Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.2)
> Gecko/20090729 Firefox/3.5.2
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729
> Firefox/3.5.2
> Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.2) Gecko/20090729
> Firefox/3.5.2
> 
> I see no passwords using comment 30 in any of these builds.

Comment 30 is incomplete. Between steps 3 an 4 you must also remove any signons.sqlite from the profile.

Comment 33

[Mark Sapiro]

(In reply to comment #32)
> 
> Comment 30 is incomplete. Between steps 3 an 4 you must also remove any
> signons.sqlite from the profile.

Ooops. That's not true. You have to do it, but it's covered in step 1.

Comment 34

[u279076]

Yes...as you said in step 1 "signons*".  I took this to mean any and all files beginning with "signons".  This would include signons.sqlite, which I removed.

Comment 31 still stands.

Comment 35

[u279076]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.2)
Gecko/20090729 Firefox/3.5.2
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729
Firefox/3.5.2
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.2) Gecko/20090729
Firefox/3.5.2

Nevermind comment 30.  I wasn't saving the RAW files.  Using RAW files this WFM.  Marking verified1.9.1

Comment 36

[Mark Sapiro]

I have just downloaded and installed Firefox 3.5.2 and I can confirm that it properly imported my signons3.txt passwords.