Closed Bug 549457 Opened 10 years ago Closed 10 years ago

errors doing STARTTLS ignored

Categories

(MailNews Core :: Networking: IMAP, defect)

x86
Windows 7
defect
Not set

Tracking

(blocking-thunderbird3.1 beta2+, thunderbird3.1 beta2-fixed)

RESOLVED FIXED
Tracking Status
blocking-thunderbird3.1 --- beta2+
thunderbird3.1 --- beta2-fixed

People

(Reporter: Bienvenu, Assigned: Bienvenu)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch proposed fix (obsolete) — Splinter Review
We're not telling the user when doing an imap connection with STARTTLS fails. This can happen if the server advertises STARTTLS, but we fail to negotiate a connection, and the server resets the connection. I have a patch for this. Not sure I can write a unit test for this, but I may be able to trick fakeserver into it.

The proposed fix makes sure we pass along errors when we're not retrying a url, and makes sure we retry urls if we fail during the logon process.
Attached patch fix with unit test (obsolete) — Splinter Review
This tweaks the retry url logic so that we get told about the server resetting the connection on the second attempt, and extends the fakeserver to optionally simulate this behavior on starttls, and adds a unit test that verifies that we get an alert.
Attachment #429611 - Attachment is obsolete: true
Attachment #429878 - Flags: superreview?(bugzilla)
Attachment #429878 - Flags: review?(bugzilla)
This is one of several fixes for our handling of connection problems that I'd really like to get into 3.1 beta 2 for more testing.
Status: NEW → ASSIGNED
Whiteboard: [has patch for review standard8]
Comment on attachment 429878 [details] [diff] [review]
fix with unit test

There's a little bit of bitrot, but I fixed that and tried running the tests, when I did, I got:

TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | ReferenceError: run_test is not defined - See following stack:
JS frame :: /Users/moztest/comm/main/src/mozilla/testing/xpcshell/head.js :: _execute_test :: line 151
/Users/moztest/comm/main/src/mozilla/testing/xpcshell/head.js:129: TypeError: parts is null

From the looks of it, it couldn't get the js stack for some reason.
Attachment #429878 - Flags: superreview?(bugzilla)
Attachment #429878 - Flags: review?(bugzilla)
Attachment #429878 - Flags: review-
this test still passes for me, once I fixed the bitrot. I'll try it on a mac build next.
Attached patch fix bit-rot (obsolete) — Splinter Review
Attachment #429878 - Attachment is obsolete: true
oops, wrong patch
Attachment #434634 - Attachment is obsolete: true
Comment on attachment 434648 [details] [diff] [review]
bit-rot fix with fake server changes

sorry to do this to you, but could you retry this? It works fine for me on the mac as well.
Attachment #434648 - Flags: superreview?(bugzilla)
Attachment #434648 - Flags: review?(bugzilla)
Comment on attachment 434648 [details] [diff] [review]
bit-rot fix with fake server changes

Yep, that's working better.
Attachment #434648 - Flags: superreview?(bugzilla)
Attachment #434648 - Flags: superreview+
Attachment #434648 - Flags: review?(bugzilla)
Attachment #434648 - Flags: review+
Flags: in-testsuite+
fixed for trunk.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [has patch for review standard8]
You need to log in before you can comment on or make changes to this bug.