413314 - error thrown if nsILoginInfo .usernameField or .passwordField is null

Status: RESOLVED WONTFIX

(Toolkit :: Password Manager, defect)

Description

[Massimiliano Mirra]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2) Gecko/2007121016 Firefox/3.0b2
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2) Gecko/2007121016 Firefox/3.0b2

In 3.0b3pre, creating a nsILoginInfo with NULL usernameField and/or passwordField and passing it to addLogin results in:

!!! [Exception... "'[JavaScript Error: "l.usernameField is null" {file:
 "file:///home/pkg/firefox3.0b3pre/components/storage-Legacy.js" line: 484}]' 
when calling method: [nsILoginManagerStorage::modifyLogin]"  nsresult: 
"0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame 
:: file:///home/pkg/firefox3.0b3pre/components/nsLoginManager.js :: anonymous 
:: line 386"  data: yes]

Problem is in storage-Legacy.js, where badCharacterPresent() does not check whether usernameField and passwordField are set before checking for bad characters:

        function badCharacterPresent(l, c) {
            return ((l.formSubmitURL && l.formSubmitURL.indexOf(c) != -1) ||
                    (l.httpRealm     && l.httpRealm.indexOf(c)     != -1) ||
                                        l.hostname.indexOf(c)      != -1  ||
                                        l.usernameField.indexOf(c) != -1  ||
                                        l.passwordField.indexOf(c) != -1);
        }

http://developer.mozilla.org/en/docs/nsILoginInfo#Parameters says NULL is a valid value for usernameField and passwordField.

Possible workaround: pass empty strings as usernameField and passwordField values.



Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Ryan Hickman]

Reporter, could you please provide a test url?

Comment 2

[Justin Dolske [:Dolske]]

Test URL not needed; it's an API issue.

I think I want to close this as WONTFIX and just require empty strings. The empty-string vs. null (as in formSubmitURL / httpRealm) has been a big pain, so it would seem smart not to continue that pattern with the form field names. Leaving open for now while I mull that over.

Comment 4

[Doron Rosenberg (IBM)]

Also note that if this remains closed, the docs on mdc need updating.

Comment 5

[Justin Dolske [:Dolske]]

</pondering>

...WONTFIX.

The tricky problem here is that since we're currently using a text file (signons3.txt) for storing entries, we can't store a |null| value. For the formSubmitURL and httpRealm fields, we fudge it by storing them as blank lines, and determine which one was supposed to be null when we read the file. We can do this because exactly one of the those two fields is supposed to be null, and that's a useful distinction (and thus worth the extra code) because it helps ensure we don't use a form login in a HTTP authentication, or vice versa.

I've been regressing this empty-string vs null distinction, because while it seemed like a simple solution at the time, it's been a headache since then. :-) 

We *could* do the same sort of thing for the usernameField and passwordField entries... But I'd have add code for that, check other code to make sure it's null-safe, probably update existing tests, write new ones... blah. And since null would be the expected value in the cases resulting in this bug (and dupes), that would mean breaking them again by forcing a switch back from "" to null... double blah.

Probably more of an explanation than this small issue really needs, but there it is. :-)

I'll update the DevMo docs.