Closed
Bug 480026
Opened 16 years ago
Closed 16 years ago
auto config verify logon needs to be able to override cert dialog
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b3
People
(Reporter: Bienvenu, Assigned: Bienvenu)
References
Details
Attachments
(1 file)
(deleted),
patch
|
neil
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
The normal imap/pop3/smtp code overrides the invalid cert dialog via the msg window attached to the url. This doesn't work for the verifyLogon call that the auto config code makes, since it doesn't take a msgWindow. Making it take a msgWindow would cause password prompts in the case of bad passwords (which wouldn't be a bad thing IMO) but that's a somewhat separate issue. What I can do is make the verifyLogon calls return the URI that gets run, and the caller can set their own notification callbacks on the channel - in the case of autoconfig, it will set the bad cert handler override.
Flags: blocking-thunderbird3+
Assignee | ||
Comment 1•16 years ago
|
||
My mistake; it's the transport that needs the notification callbacks set, and you can't get from the uri to the transport. The channel might be a substitute, but I can't get there either. So I think I am going to need to pass in a msg window - if we so decide, I can suppress the password prompt an other way.
Assignee | ||
Comment 2•16 years ago
|
||
this patch makes two changes - pass in an nsIMsgWindow to verifyLogon, and have verifyLogon return the nsIURI that is run.
This should allow the autoconfig code to do what it wants, though I need to figure out a way of retrying the url in the case of accepted certificate exceptions.
Attachment #364003 -
Flags: superreview?(neil)
Attachment #364003 -
Flags: review?(neil)
Updated•16 years ago
|
OS: Windows XP → All
Hardware: x86 → All
Comment 3•16 years ago
|
||
Comment on attachment 364003 [details] [diff] [review]
proposed fix
> rv = RunPopUrl(aServer, url);
>+ if (aURL)
>+ url.forget(aURL);
>+ }
You can only return values on success. Does RunPopUrl always succeed? (Why are you returning the URL anyway? Maybe the bad cert handler should retry the detection if the certificate is accepted?)
Assignee | ||
Comment 4•16 years ago
|
||
(In reply to comment #3)
> (From update of attachment 364003 [details] [diff] [review])
> > rv = RunPopUrl(aServer, url);
> >+ if (aURL)
> >+ url.forget(aURL);
> >+ }
> You can only return values on success. Does RunPopUrl always succeed?
I'll fix that.
> (Why are you returning the URL anyway?
This gives us the flexibility to poke at the url. For example, if I add an attribute to suppress password prompts to the url, I could set it after launching the url, since the password prompting doesn't happen until later.
Maybe the bad cert handler should retry the
> detection if the certificate is accepted?)
Yes, that's what's going to happen - I can't make the backend code do it, because the bad cert handler dialog is asynchronous, which it needs to be to get a chance to retrieve the bad cert...
Updated•16 years ago
|
Attachment #364003 -
Flags: superreview?(neil)
Attachment #364003 -
Flags: superreview+
Attachment #364003 -
Flags: review?(neil)
Attachment #364003 -
Flags: review+
Comment 5•16 years ago
|
||
Comment on attachment 364003 [details] [diff] [review]
proposed fix
OK, so r+sr with me with the outparam not written on failure.
Assignee | ||
Comment 6•16 years ago
|
||
fix, fix checked in, with the rv of RunPopUrl checked before doing the forget.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Flags: wanted1.8.1.x?
Flags: blocking1.8.1.next?
Assignee | ||
Comment 7•15 years ago
|
||
It wouldn't be this patch, which is irrelevant for 2.0 since 2.0 doesn't have automatic account discovery/config, but bug 429843 would be very relevant.
Updated•15 years ago
|
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x-
Flags: blocking1.8.1.next?
Flags: blocking1.8.1.next-
You need to log in
before you can comment on or make changes to this bug.
Description
•