Closed Bug 60280 Opened 25 years ago Closed 16 years ago

Failure to log into AOL mail can lock up app

Categories

(SeaMonkey :: Passwords & Permissions, defect)

Product:
SeaMonkey
Component:
Passwords & Permissions
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: mikepinkerton, Unassigned)

References

Details

(Keywords: hang, top100)

Attachments

(1 file)

Band-aid patch in nsWallet.cpp
764 bytes, patch
Details | Diff | Splinter Review
- try to log into aol mail from www.aol.com, but enter a bad password so it fails - it will ask if you want to save the password, say no. - try again, and fail. - try again... at some point, the password manager dialog will get stuck on the screen and lock up the app. You have to force quit it.
Severity: normal → critical
Keywords: crash, mozilla0.9, top100
What's happening is that the onsubmit handler in nsWalletService.cpp is being called twice when the form is submitted. The second call is occuring before the first call has completed and, in fact, happens while the password-manager dialog is on the screen. I'm trying to zero in on why this is happening by generating a simplified test case. Stand by.
Here's the simplified test case. Note that there is an onchange handler for the password field that causes the form to be submitted. So if you click on the submit button as the first thing after entering the password, indeed the form is submitted twice. Very strange content! Never-the-less, password manager's onsubmit handler should be able to handle this without being perturbed. Perhaps it should set a flag indicating that it has been entered and refuse future entries until the original handler is finished. <html> <body> <form name="loginform"> <table> <tr> <td> Enter Screen Name: <input type=text> Enter Password: <input type=password onchange=document.loginform.submit();> <input type=submit value="Enter AOL Mail"> </td> </tr> </table> </form> </body> </html>
Status: NEW → ASSIGNED
Summary: Failure to log into AOL mail can lock up app → [w]Failure to log into AOL mail can lock up app
Just for the record, here's an even simpler test case that demonstatest the problem. <html> <body> <form name="loginform"> Enter Screen Name: <input type=text> Enter Password: <input type=password onchange=document.loginform.submit();> <input type=submit value="Enter AOL Mail"> </form> </body> </html>
On second thought, the problem is not tht the onsubmit handler in nsWalletService was re-entered but rather that it re-entered the dialog. That is, the DoDialog routine in nsCommonDialogs.cpp is not reentrant. As a band-aid, I could add the flag in nsWalletService (that I spoke about above) to make sure that it never gets re-entered. But the proper solution should be to make nsCommonDialogs be re-entrant. danm, is that possible or should I go for the band-aid fix?
If we do want to go with the band-aid fix, I have a patch for it which I'll attach. I tested it and it works.
Attached patch Band-aid patch in nsWallet.cpp — — Splinter Review
First, let me say that calling the submit function in the password field onchange handler is pointless, weird, and just wrong. But of course we shouldn't crash on it. Second, as Steve says, such HTML makes us stack up two modal password manager dialogs, one each as the password blur and button click events come in. We have similar problems elsewhere: this same situation causes bug 60150, for example. Thanks for analyzing this one, Steve. But since it's a general problem, I'll take the bug and try to think of a general solution. I'll keep Steve's band-aid in mind, should it come to that. But first I need to experiment with blocking processing of queued OS events on a window that's spawned a modal window.
Assignee: morse → danm
Status: ASSIGNED → NEW
Keywords: crash → freeze
Status: NEW → ASSIGNED
Keywords: freeze → hang
Target Milestone: --- → mozilla0.9
Netscape Nav triage team: this is a Netscape beta stopper.
Keywords: nsbeta1
Priority: P3 → P2
Since I'm slow and we're wanting to fix at least this symptom of the problem, r= danm on morse's patch for the upcoming branch build.
a=ben@netscape.com for this bandaid fix for the branch.
Summary: [w]Failure to log into AOL mail can lock up app → Failure to log into AOL mail can lock up app
I'm the developer for Mail on the Web. Having an "onchange" handler is not weird or wrong as has been alleged in previous comments. The point of it is to submit the form when someone is done typing his password and presses the "Enter" key.
Your intentions are admirable but your implementation is flawed because it does result in a double submission. And that is independent of whether or not we have a browser bug. You are no doubt receiving two copies of the submitted form at the server. Is that really what you want?
Taking this one step further, it also means that you will be receiving a partially-completed form if the user is unconventional and decides to type in his password before typing in his username.
Here's a suggestion of what might solve everyone's problem. Add some more code to the onChange handler so it: 1. Checks to make sure that the username field is empty and exits if it is 2. Checks to see if the submit button just got focus and exits if it did 3. If both checks 1 and 2 pass, do the submit
I am not sure how IE or previous Navigator releases handled this scenario, but we never had this problem prior to now. You're right, we don't want a double submission of the form, and we don't get it from either Netscape 4.7x or IE 5.x. Although I appreciate your situation, I don't think that changing my code will fix your bug, since others may do the same thing. I think that CompuServe domestic Webmail, for instance, does the same thing, and that's not my code.
I think that's what Steve Elmer wanted to know. As you can see, we do have a simple band-aid fix (as described above) that we can check in if need be.
Jack, you've looked at your server logs or something to determine that you're not getting double submits from IE or Communicator? If that's truly proven, then they must be doing something like the general fix danm mentioned. If this bug gets closed due to the bandaid checkin, we should open a new bug for the general fix.
fwiw, i've noticed that secureID access to aol webmail has been turned off as of last monday. Was this on purpose? Makes it kinda hard to check my email remotely, and makes it impossible for me to test/verify this bug.
There was a problem with the production SecurID server where, under load, it would sometimes send back incorrect "OK" responses, allowing a user in without the correct SecurID code. The SecurID team has been working on a fix, they think they have it, it's being tested in production today. Best case scenario, SecurID support will be reenabled tomorrow.
Jack, selmer: I watched network traffic using Macintosh Navigator 4.x and 6 (it's a handy platform for testing and I hope behaves the same as Windows). Changing the password field and clicking the Submit button does submit the form twice in Nav 4, though it didn't seem to in Mozilla (despite the two wallet dialogs Mozilla gave me). I don't quite get that, but at least it confirms double submissions can be a problem. I understand this code is a workaround for Navigator which unfortunately doesn't submit even a simple form like this upon hitting Enter. It's a shame it's necessary. (By the way, even this doesn't work on a Mac Navigator 4.x build. Though it does on Windows and Mac Mozilla. Sigh.) Regardless, the hang remains our problem. We should find something larger than this simple thing to choke on. Still, on the server side, I think morse's suggestions for making the workaround a little more watertight are worth considering.
Adding branch accept to status whiteboard. Please check in on the branch A.S.A.P. Thanks! .Angela...
Whiteboard: [branch accept]
Well I got the approvals for checking the band-aid patch onto the branch and decided to test it one last time before doing so. But the aol website has changed and the problem no longer occurs when logging in on their site. There are at least two changes (and probably more) that I noticed: 1. The "Enter AOL mail" button is gone and has been replaced by a "GO" button 2. The onchange attribute has been removed from the <password> tag. Of course #1 is insignificant but it did clue me in to the fact that things have definitely changed. And, as a result of item #2, we no longer get the double submit and therefore there is no problem with the password manager. That's not to see that this bug has been fixed. The simplified content presented in this bug report demonstrates it. But it does mean that the bug is no longer demonstrable on the aol site and so the importance of this bug has suddenly been diminished. For this reason I did not check the band-aid patch presented here is much less important. If someone thinks that's the wrong decision, please speak up. But there's some bad news. See my next posting.
The good news is that the form on the AOL page no longer gets submitted twice. The bad news is that it doesn't get submitted at all. And I have not yet been able to figure out why. Here's a simplified test case that I obtained by extracting the relevent parts of the AOL website. If I run this on 4.x by entering an invalid password and pressing the GO button, I get to a webpage saying that I entered an invalid password. If I do the exact same thing on seamonkey, absolutely nothing happens. <html> <BODY> <FORM name="loginform" action="https://screenname.aol.com/_cqr/mc/dosignin.adp" method="POST"> <INPUT TYPE="TEXT" NAME="screenname"> <INPUT TYPE="PASSWORD" NAME="password"> <INPUT TYPE="SUBMIT"" VALUE="Go"> </FORM> </body> </html> This is a completely new bug and I'll open a new bug report on it.
Ignore the bad news mentioned above -- it's a psm problem on my machine (I don't yet know what's wrong) and not a general bug. I just opened bug 65157 on that problem and then when I realized that this was not a general problem I closed that report out as invalid.
Removed branch accept from status whiteboard. No longer needed on the branch. .Angela...
Whiteboard: [branch accept]
*** Bug 66454 has been marked as a duplicate of this bug. ***
->moz1.0
Priority: P2 → P3
Target Milestone: mozilla0.9 → mozilla1.0
See bug 85286 for a nearly identical problem -- in fact it could be considered as a dup.
Steve, the double submit problem might be caused by bug 90932 which was just fixed. If you build with that patch, does this problem go away?
selmer, I think you meant bug 90392. I'll text that patch out right now.
selmer, I don't even have to try out that patch in bug 90392 to realize that this is a completely different problem. The problem in bug 90392 is that something in the event system is causing the submit to occur twice. The problem here (and in bug 85286) is that the html content is written in such a way as to force the double submission. Specifically, in this report the html password field contains an onchange handler that explicitly submits the form. And in bug 85286 the form tag contains an onsubmit handler that explicitly forces an additional submit.
Of course, you're right about the bug number. This bug comes back to me now. We were expecting a server side fix. Didn't we have that fix for some window of time? Did they revert or what?
Yes, they did fix things on the server side and that's why this bug never got fixed. But there are other servers out there which we have no control over. And now bug 85286 came along (www.msn.com) which is nearly identical to this one (the second submit is triggered slightly differently as described in my last comment) and again we get two password-manager dialogs. Yes, we had a band-aid fix which is posted above in this report. (That fix never got checked in because the bad content on the aol server was apparently fixed.) The band-aid worked because the two submits were recursive and the band-aid checked for recursion. In the case of bug 82586 the two submits are sequential and not recursive so the band-aid fix presented here will not help in that case.
And of course I meant 85286 and not 82586 in the last line of my comment above. I guess all Steve's are dyslexic. ;-)
Blocks: 104166
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 (you can query for this string to delete spam or retrieve the list of bugs I've moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
Target Milestone: mozilla1.0.1 → ---
Depends on: 60150
Target Milestone: --- → Future
Keywords: nsbeta1
attempted to log into aol.com. Entered screename, password, and attempt to log on failed. Returned to ScreenName and password screen.
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-
This bug is targeted at a Mac classic platform/OS, which is no longer supported by mozilla.org. Please re-target it to another platform/OS if this bug applies there as well or resolve this bug. I will resolve this bug as WONTFIX in four weeks if no action has been taken. To filter this and similar messages out, please filter for "mac_cla_reorg".
I'm a bad owner for this bug since I no longer have access to a Mac. So I'm dumping it on sfraser. Simon, this bug is scheduled to be resolved. It could stand to be saved if it's still a problem, which it could be even on OS X.
Assignee: danm-moz → sfraser
Status: ASSIGNED → NEW
have same problems! installed: Mozilla 1.5, MS ME 1. Problem occurs when trying to access internet via own proxy-server password manager asks for user/pswrd, asks again and again....cancelling this gives error message:Cache Access Denied [...] pls check your psswrd etc... 2. since today similar problem when trying to download emails with direct connection (no proxy!). password manager asks for user/pswrd, asks again and again.... ???
OS: Mac System 9.x → All
Hardware: Macintosh → All
Product: Browser → Seamonkey
Assignee: sfraser_bugs → dveditz
QA Contact: tpreston
Assignee: dveditz → nobody
Priority: P3 → --
Target Milestone: Future → ---
Pink, Do you still see this problem with the latest SeaMonkey 2.0Alpha3? We are now using the toolkit login manager and the wallet code is dead so this should not be a problem.
Whiteboard: CLOSEME 2009-03-27
No response from the original reporter. Closing => INCOMPLETE
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → INCOMPLETE
Whiteboard: CLOSEME 2009-03-27
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: