1404049 - Thunderbird does not honor mail.imap.use_literal_plus=false

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Raimo Niskanen]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170926190823

Steps to reproduce:

I am trying to work around a bug (I think) in a IMAP4 server that claims to support capability LITERAL+ but responds with "invalid data syntax" to the APPEND command.

I do this by setting mail.imap.use_literal_plus=false, but the setting seems to be inefective.

Thunderbird version is 56.0b3 (32-bit).  I tried to go up to the beta track from the stable to see if it was fixed, but the problem remains.


Actual results:

8408[1ec273a0]: 1eb87800:imap.bredband.net:NA:CreateNewLineFromSocket: * OK [CAPABILITY IMAP4REV1 LITERAL+ UTF8=ALL SASL-IR LOGIN-REFERRALS AUTH=LOGIN ID] mail65c50 IMAP4rev1 Bigfoot

8408[1ec273a0]: 1eb87800:imap.bredband.net:NA:SendData: 1 authenticate LOGIN

8408[1ec273a0]: 1eb87800:imap.bredband.net:NA:CreateNewLineFromSocket: 1 OK [IMAP4rev1 UIDPLUS IDLE LOGIN-REFERRALS NAMESPACE QUOTA CHILDREN] Remote user rainis authenticated

5976[bce37d0]: 1729000:imap.bredband.net:A:SendData: 9 append "Sent" (\Seen) {480+}

5976[bce37d0]: 1729000:imap.bredband.net:A:SendData: BCC: Raimo Niskanen <[e-mail]>

To: [e-mail]

From: Raimo Niskanen <[e-mail]>

Subject: Test 5

Message-ID: <[e-mail]>

Date: Thu, 28 Sep 2017 19:05:43 +0200

User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101

 Thunderbird/52.3.0

MIME-Version: 1.0

5976[bce37d0]: Content-Type: text/plain; charset=windows-1252; format=flowed

Content-Language: sv

Content-Transfer-Encoding: 7bit



Test 5

-- 

Test

5976[bce37d0]: 1729000:imap.bredband.net:A:SendData: 

5976[bce37d0]: ReadNextLine [stream=16bad1f0 nb=58 needmore=0]
5976[bce37d0]: 1729000:imap.bredband.net:A:CreateNewLineFromSocket: 9 BAD invalid APPEND command syntax, invalid data syntax




Expected results:

I expected Thunderbird to not use {480+} at the end of the "9 append" command, since I had told it to not use LITERAL+, and hoped that the server then would understand the APPEND command and manage to save the message in the "Sent" folder.

Comment 2

[gene smith]

Attachment: 1404049-review-changes0.patch

I tested this with yahoo imap that supports LITERAL+ and indeed, even with  mail.imap.use_literal_plus set false it still did the literal+ style append. Problem is because it only used mail.imap.use_literal_plus after an imap capability is requested and the response is received. But capability requests don't always occur since often TB just uses capability responses that occur *without* a request to set flags. So I moved the usage of mail.imap.use_literal_plus to the point where the LITERAL+ capability is actually used (where it decides whether to use {123} or {123+}). This fixes the problem.

Wayne, Jorg said he doesn't have time for feedback or reviews now. So since I don't know who else to ask I have set the feedback flag for you.

Comment 3

[Wayne Mery (:wsmwk)]

Comment on attachment 8913549 [details] [diff] [review]
1404049-review-changes0.patch

I'm not a code jockey, but I've heard of such issues in the past, and the rearrangement looks reasonable.

Comment 4

[Jorg K (CEST = GMT+2)]

Comment on attachment 8913549 [details] [diff] [review]
1404049-review-changes0.patch

Review of attachment 8913549 [details] [diff] [review]:
-----------------------------------------------------------------

This looks right, but I need to run a try over it because we have some crazy tests testing wrong things.

Right now, IMAP in TB is pretty much broken due to bug 1404003. Once I sort this out, I'll return here.

In general, there are five people doing mailnews reviews: Kent, Magnus, Joshua, Aceman and myself. Kent, Joshua and myself are most familiar with IMAP. Aceman is our account manager specialist, so I deferred the review of bug 1402558 to him.

::: mailnews/imap/src/nsImapProtocol.cpp
@@ -5471,5 @@
> -      if (capabilityFlag & kLiteralPlusCapability)
> -      {
> -        GetServerStateParser().SetCapabilityFlag(capabilityFlag & ~kLiteralPlusCapability);
> -      }
> -    }

Why are you removing this code? The code switches the internal flag off under certain circumstances. That's not a bad thing to do.

Or maybe it is. It seems fishy to negate a real capability, so that the internal state is actually a lie. Your approach seems better. Leave the internal state intact, but check for usage in the right spot.

Besides, the code here is 200% clumsy. Why check the flag and only turn it off when it's set. You can always turn it off.

Comment 5

[Jorg K (CEST = GMT+2)]

Gene, can you please comment on the part of my comment #4 below the quoted code.

Do you also feel that negating the capability flag is the wrong approach and instead the preference should be checked in the appropriate location as you do in your patch. Maybe a rhetorical question since you've sent a patch that does exactly that.

I just wanted to confirm.

Comment 6

[gene smith]

Yes I agree. I need to clean up the patch later today. Remove some printf and gds in comments.

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 8913549 [details] [diff] [review]
1404049-review-changes0.patch

There were not prints or gds in comments ;-) Let's take it as it is.

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: 1404049-review-changes0.patch - rebased to latest trunk

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ac845a2682a7
fix that mail.imap.use_literal_plus set to false may have no effect. r=jorgk

Comment 10

[Raimo Niskanen]

I have a question regarding the capabilities list sent by this server in the log I supplied.
The first is: * OK [CAPABILITY IMAP4REV1 LITERAL+ UTF8=ALL SASL-IR LOGIN-REFERRALS AUTH=LOGIN ID] mail65c50 IMAP4rev1 Bigfoot
And then as a response to: 1 authenticate LOGIN
the server says: 1 OK [IMAP4rev1 UIDPLUS IDLE LOGIN-REFERRALS NAMESPACE QUOTA CHILDREN] Remote user rainis authenticated

That sure looks like a capability list without ID and LITERAL+, but as I try to read RFC 3501 an OK response may have a capability response code, and that should look like 1 OK [CAPABILITY ...].

Is this server sending an incorrect response or is TB logging hiding the CAPABILITY response code?

Because if this is a valid capability list then TB tries to use both the ID command and the LITERAL+ feature against the server's wish, but if it is an incorrect response it is a server bug...

I hope someone can clarify...

Comment 11

[gene smith]

Can you tell me imap server you are connecting to? Also maybe full imap log attached might help.
I will test locally and review code. Probably should move this issue to a new bug for future discussion.

Comment 12

[Raimo Niskanen]

Attachment: TBird log of IMAP

TBird log of IMAP traffic.

It is the server imap.bredband.net that is interesting  and that login occurs about 90%..95% down in the log.

Watch the two capabilities lists - one in the server greeting, I think, and the other as a reply to the 1 AUTHENTICATE LOGIN command.

Then comes a failure in the 9 APPEND "Sent" command where TBird uses literal+ and the server barfs.

Comment 13

[Raimo Niskanen]

My ISP (bredbandsbolaget.se) will not tell me which IMAP server they are using.  In fact I do not think neither 1:st or 2:nd line support knows this...

Comment 14

[gene smith]

I see MPDaemon mentioned in some responses so that must be your server. I have a guest account on that type of server but I don't see an auth response like
"1 OK [IMAP4rev1 UIDPLUS IDLE LOGIN-REFERRALS NAMESPACE QUOTA CHILDREN] Remote user rainis authenticated"
on MPDaemon or any of my other several account imap servers. My reading of the rfc implies the correct response should be
"1 OK [CAPABILITY IMAP4rev1 UIDPLUS IDLE LOGIN-REFERRALS NAMESPACE QUOTA CHILDREN] Remote user rainis authenticated"; however, the rfc really doesn't provide an example.

Also, looking at your full log I see that tb does send capability commands to your server, but I assumed it didn't since that was the case with the yahoo server. I also have some accounts, other than yahoo, that do the same and also support LITERAL+. But testing them with mail.imap.use_literal_plus=false they still did the literal+ style accept! (This is without the patch/fix from above applied.) So this indicates that the patch/fix is still effective and OK since it no longer relies on a capability command occurring to block the LITERAL+ style accept.

Anyhow, back to your latest issue. From what I see in the tb code, the internal capability flags are only changed after a leading "CAPABILITY" token is detected in a response. Since your server (MPDaemon?) leaves off the CAPABILITY token, the other capability tokens/flags are ignored by tb. However, earlier capability requests did return leading "CAPABILITY" followed by LITERAL+ capability which remains in effect as you see (but it is rejected by your imap server when tb tries to do it).

Looking closely at the tb imap log for my many test accounts, I do see one server (Zimbra) that does this:
C: 2 authenticate PLAIN
:
Logging suppressed for this command (it probably contained authentication information)
:
S: 2 OK [CAPABILITY IMAP4rev1 ACL BINARY CATENATE CHILDREN CONDSTORE ENABLE ESEARCH ESORT I18NLEVEL=1 ID IDLE LIST-EXTENDED LIST-STATUS LITERAL+ LOGIN-REFERRALS MULTIAPPEND NAMESPACE QRESYNC QUOTA RIGHTS=ektx SASL-IR SEARCHRES SORT THREAD=ORDEREDSUBJECT UIDPLUS UNSELECT WITHIN XLIST] AUTHENTICATE completed

So this is good evidence that the leading "CAPABILITY" string is needed and required by the rfc (although this does not reduce the previously determined capabilities like you are needing).

P/S: My isp also knows nothing about their imap server (called openwave) that also has a bad bug that they and openwave refuse to fix or even discuss!

Comment 15

[Raimo Niskanen]

As I read the log I can not see that TB uses the CAPABILITY command towards imap.bredband.net, and the MDaemon name is also not from that server.  Both are in the session with the other server in the log (epost.tba.net).

The only clue about the server I find from imap.bredband.net is "Bigfoot".  And the only match my Internet Search Skills gives is http://webmail.bigfoot.com/, which may be a provider for bredbandsbolaget.se, who knows!

I agree that one has to read RFC 3501 "as the devil reads the Bible" (a swedish saying) to think that since the the server MAY return a CAPABILITY response code in the tagged response to the LOGIN command means that a capability list without the CAPABILITY response code itself is enough...

They (Bredbandsbolaget) have claimed for a month now that they are working on a solution.  And when I finally sit down and debug their problem this is what turns up.  It is a sad state of affairs when there is better to hope for a workaround in the client that for an ISP looking into a bug of their own. :-(

Is it not also strange to change the capabilities after logon.  Makes me wonder if they have some kind of proxy first and then the actual server.  Did you say that the server may not reduce its capabilities from the greeting after e.g logon?

Comment 16

[Raimo Niskanen]

RFC 5301 says that the server MAY send an updated CAPABILITY response code in the tagged OK response as part of a successful authentication.

Comment 17

[gene smith]

Actually rfc 3501. Yes updating ok but not sure about missing CAPABILITY token. I will parse the rfc and maybe ask on the imap mailing list.
Sorry didn't see the 2nd account. Tks for pointing it out.

Comment 18

[gene smith]

Another thought. Can you get me a temporary guest account on bredband so I can test possible changes?

Comment 19

[Raimo Niskanen]

Yes, 3501, slipped on the keys.  I agree on that CAPABILITY token should be present.

I had one last account that I could create.  You can borrow it, and I want it back.
[e-mail]
User Name: manis
Password: K.Vrf6Hy
Incoming Server: imap.bredband.net:993 SSL/TLS, Normal password
Outgoing Server: smtp.bredband.net:465 SSL/TLS, Normal password

Comment 20

[gene smith]

Yes section 7.1 of rfc says capability atom must be first in square bracket enclosed "response code". So tb correct to ignore the attempt to set capabilities.
Tks for account I will do some testing.

Comment 21

[gene smith]

First off, the username manis didn't work but [e-mail] did. So I was able to send email from my isp account to [e-mail] and it worked.
But when I look at the log for bredband entries, all the capability responses, even for authentication, return the "correct" stings, including the "CAPABILITY" atom and the full list including LITERAL+ and ID !!!
I then copy a email from Local Folders to materjoni account Sent folder and it works. Looking at the log I see it uses the LITERAL+ method and succeeds.
So the imap account you loaned me seems to have no problems at all, at least with imap.

Not that it matters but I was unable to send email via the smtp account. Tried both usernames but it responds with something about a "misbehaving sending server". 

Also, since the the capabilites I see aren't getting reduced, tb also send the ID command and the imap server responds with this:

 * ID ("name", "Bigfoot", "version", "1.0", "os", "Linux", "os-version", "2.6", "vendor", "Megamailservers.com", "author", "Derek Snider")

So apparently the server bredband uses is from Megamailservers.com running on linux. The website is not very helpful.

Can you try this account you loaned me from your site and see if it works for you?

Comment 22

[gene smith]

From my location imap.bredband.net resolves to 91.136.8.208. You might try that ip address for the server name in your normal account and see if that fixes the problem (it works for me but using this causes certificate warnings you have to accept). We have seen another issue where imap.yahoo.com resolves to various different addresses and server behavior differed depending on which one the user randomly gets. You may have hit on it when you mentioned proxy above, which might be sort of what's causing the problem depends on which imap "proxy" server at various ip addresses you connnect to.

Comment 23

[Raimo Niskanen]

Sorry about the login name. I should have tested the test account myself first...

It has fluctuated in the past if the user name (manis) should be used or the e-mail address should be used as user name, for Bredbandsbolaget...

I confirm what you see.  With the new account the capabilities after logon looks correct.  Some changes but ID and LITERAL+ remains, and there is a CAPABILITIES atom first in the bracketed list.  The APPEND command works.

On the same TB with my old account it still looks as in the log I have attached.  After logon the OK reply has a list without the CAPABILITES atom and without ID and LITERAL+, and the APPEND command fails.
Here I used TB 52.3.0 on my work laptop.

So my newly created test account behaves differently from my problem account.

I am hunting the support at Bredbandsbolaget to figure out why SMTP authentication does not work, but since this is not webmail it has to go to 2:nd line support.  Still waiting...

The DNS name imap.bredband.net resolves to the same IP for me.  So it seems they are using different backend IMAP servers for different accounts.

Comment 24

[Jorg K (CEST = GMT+2)]

I've been watching this RESOLVED (in comment #9) bug from the distance. Anything wrong with the patch we landed?

Comment 25

[gene smith]

(In reply to Jorg K (GMT+2) from comment #24)
> I've been watching this RESOLVED (in comment #9) bug from the distance.
> Anything wrong with the patch we landed?

No, it's fine.

Comment 26

[Raimo Niskanen]

The patch mends a setting that can be used as a workaround for a problem I have encountered.

We are discussing if the actual problem is due to a TB bug or a server bug, or if TB should change behaviour to interoperate with servers behaving this way, and if so maybe a new report should be created.

So far we seem to be reaching the conclusion that this is a server bug and TB should not change.

Except for applying this patch, which is already done, and then I/we have a workaround for servers misbehaving this way.

Comment 27

[gene smith]

Raimo,have you tried the patch with a daily build?

Comment 28

[Raimo Niskanen]

Nope.  I have not stumbled on how to find a daily build of upcoming 58.0...

Comment 29

[Raimo Niskanen]

... for Windows...

Comment 30

[Raimo Niskanen]

This must be it:
  https://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central/thunderbird-58.0a1.en-US.win64.installer.exe

Comment 31

[gene smith]

I think that's it. No warranty!

Comment 32

[Raimo Niskanen]

Works like a charm!  Problem solved!  Have not seen any others yet ;-)

Thank you guys for excellent support.  You really deserve a donation!
Should I make it through donate.mozilla.org, or have you separated from Mozilla now and want it some other way?

Comment 33

[Wayne Mery (:wsmwk)]

(In reply to Raimo Niskanen from comment #32)
> Works like a charm!  Problem solved!  Have not seen any others yet ;-)
> 
> Thank you guys for excellent support.  You really deserve a donation!
> Should I make it through donate.mozilla.org, or have you separated from
> Mozilla now and want it some other way?

https://donate.mozilla.org/en-US/thunderbird/about/ gets  to thunderbird

thanks

Comment 34

[Raimo Niskanen]

Where can I find information about when 58.0 is due to become official beta?  The current official beta is 56.0b3.

Comment 35

[gene smith]

I don't know but I'm sure Wayne or Jorg can tell you.

Comment 36

[Wayne Mery (:wsmwk)]

see the beta column at https://wiki.mozilla.org/RapidRelease/Calendar - it will be after the date shown, with an undetermined  delay - emphasis on the last two words.  We cannot predict a date.

Comment 37

[Jorg K (CEST = GMT+2)]

(In reply to Raimo Niskanen from comment #26)
> The patch mends a setting that can be used as a workaround for a problem I
> have encountered. ...
Thanks for the summary. I'm the guy who looks after the code in general and releases, so I can't watch everything in detail.

You got answers for all your questions:
Dailies:
https://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central/
Sadly currently Daily is a little broken in various regards, for example compaction not working (bug 1403771).

Donations:
https://donate.mozilla.org/en-US/thunderbird/about/

Releases:
https://wiki.mozilla.org/RapidRelease/Calendar

TB 58 beta should be coming out in late November 2017. If the fix here is highly desirable, I can include it in TB 57 beta due out in a couple of weeks.

Comment 38

[gene smith]

(In reply to Jorg K (GMT+2) from comment #37)
> 
> TB 58 beta should be coming out in late November 2017. If the fix here is
> highly desirable, I can include it in TB 57 beta due out in a couple of
> weeks.

Jorg, Probably very low usage for this but at least it was tested by a real user and it solves his problem and the fix is pretty simple. So I would vote to include it in 57b.

Raimo, If your ISP/imap server provider convinces you that "1 OK [IMAP4REV4 ....] ..." is a valid capability response then please open a new bug report for this and we can continue our discussion anew. You can also take back the account I borrowed if/when you need it.

Comment 39

[Raimo Niskanen]

Jorg.  It would be awsome to get the fix in TB 57 beta!

gene. I will open a new bug report, if I ever get to someone at Bredbandsbolaget that knows enough about IMAP to dare to claim that. This is very unlikely!
Great to know you do not need the account anymore.

Comment 40

[Jorg K (CEST = GMT+2)]

Comment on attachment 8914514 [details] [diff] [review]
1404049-review-changes0.patch - rebased to latest trunk

Small fix, low risk, tested on live case.

Comment 41

[Jorg K (CEST = GMT+2)]

Beta (TB 57):
https://hg.mozilla.org/releases/comm-beta/rev/c2ea76e8472e7a539d63e23cb408d8b47dd6123b