Closed Bug 462045 Opened 13 years ago Closed 13 years ago

make server.isSecure work for POP/IMAP

Categories

(MailNews Core :: Backend, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b1

People

(Reporter: mkmelin, Assigned: mkmelin)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch proposed fix (obsolete) — Splinter Review
Following up on bug 316245 comment 20. 
server.isSecure doesn't really work for POP/IMAP. I think this the best of the suggested options is to make it work - that way it's handy to use from js, and the TLS is a bit of a special case anyways...
Attachment #345154 - Flags: superreview?(neil)
Attachment #345154 - Flags: review?(neil)
Comment on attachment 345154 [details] [diff] [review]
proposed fix

This code duplication worries me... maybe it would be better to move the existing code to nsNntpIncomingServer?

I'm also tempted to suggest a followup bug to make NNTP use the socket type throughout (making isSecure readonly), but I imagine that might prove a headache for existing profiles.
Attachment #345154 - Flags: superreview?(neil) → superreview?(bienvenu)
Well, that depends on how you look at it. 
Sure, there's a ~10 row code duplication, but at least to me is seem all one ever want to know if it's secure or not. alwaysUseTLS is the odd one out, only applicable for pop and imap so therefore I'd argue checking for it belong in those classes.
Attachment #345154 - Flags: superreview?(bienvenu)
Attachment #345154 - Flags: review?(neil)
Comment on attachment 345154 [details] [diff] [review]
proposed fix

Though making isSecure readonly should certainly be a goal, and we even have bug 420262 about nntp starttls...
Attached patch proposed fix, v2Splinter Review
Attachment #345154 - Attachment is obsolete: true
Attachment #345793 - Flags: superreview?(bienvenu)
Attachment #345793 - Flags: review?(neil)
Ignore the one space off in nsMsgFolderDataSource...
Attachment #345793 - Flags: review?(neil) → review+
sr ping
Comment on attachment 345793 [details] [diff] [review]
proposed fix, v2

sr=bienvenu, except for a nit - should leave arg name as aIsSecure here:

 NS_IMETHODIMP
-nsMsgIncomingServer::GetIsSecure(PRBool* aIsSecure)
+nsMsgIncomingServer::GetIsSecure(PRBool *isSecure)
 {
-  return GetBoolValue("isSecure", aIsSecure);
+  NS_ENSURE_ARG_POINTER(isSecure);
+  PRInt32 socketType;
+  nsresult rv = GetSocketType(&socketType);
+  NS_ENSURE_SUCCESS(rv,rv);
+  *isSecure = (socketType == nsIMsgIncomingServer::alwaysUseTLS ||
+               socketType == nsIMsgIncomingServer::useSSL);
 }
Attachment #345793 - Flags: superreview?(bienvenu) → superreview+
changeset:   812:e689f497950d
http://hg.mozilla.org/comm-central/rev/e689f497950d

->FIXED
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b1
Comment 1 filed as bug 463096.
Depends on: 463390
Depends on: 470067
You need to log in before you can comment on or make changes to this bug.