Opened 3 years ago

Closed 3 years ago

#1486712 closed Bugs (fixed)

Unable to save mail in cyrus imapd

Reported by: felfert Owned by:
Priority: 5 Milestone: 0.4.1
Component: Core functionality Version: 0.4-stable
Severity: normal Keywords:
Cc: fritz@…

Description

I'm using cyrus (2.3.7) and am trying to get RCMail 0.4-beta running.
During my tests, i noticed, that RCM always reports an error, when it is appending a mail to some imap folder (e.g.: saving in Sent after sending the mail). Fortunately, being a developer myself, I was able to dig into the code: It turns out, that RCM does not adhere to RFC when it is using the function appendFromFile() in program/includes/rcube_imap_generic.php. Instead of using CRLF as line-delimiter, it is using LF which in turn triggers an error in cyrus. As you might know, cyrus is very picky about newlines as it adheres *very* close to RFCs.

In order to show CR and LF in the logs, i instrumented the existing logging slightly (now shown as \r and \n respectively). This change is attached in the first attachment (imap-debug.patch).

Will attach more stuff after submission ...

Attachments (5)

imap-debug.patch (966 bytes) - added by felfert 3 years ago.
Patch to improve imap debug logging: CR and LF are show as \r and \n in the log
default-setup.log (4.0 KB) - added by felfert 3 years ago.
imap debug log of saving mail to drafts (default setup)
manual-delimiter.log (4.2 KB) - added by felfert 3 years ago.
imap debug log of saving mail to drafts (mail_header_delimiter changed to "\r\n" in config)
manual-delimiter-textonly.log (2.5 KB) - added by felfert 3 years ago.
imap debug log of saving mail to drafts (mail_header_delimiter changed to "\r\n" in config and having switched to "text-only" in composer)
htmlmail-save-delimiterfix.patch (877 bytes) - added by felfert 3 years ago.
Patch to fix delimiters of the plain-text alternative part when composing html mail.

Download all attachments as: .zip

Change History (21)

Changed 3 years ago by felfert

Patch to improve imap debug logging: CR and LF are show as \r and \n in the log

Changed 3 years ago by felfert

imap debug log of saving mail to drafts (default setup)

Changed 3 years ago by felfert

imap debug log of saving mail to drafts (mail_header_delimiter changed to "\r\n" in config)

Changed 3 years ago by felfert

imap debug log of saving mail to drafts (mail_header_delimiter changed to "\r\n" in config and having switched to "text-only" in composer)

comment:1 Changed 3 years ago by felfert

Description of the attached logs:

All Logging was done via syslog with my patch imap-debug.patch applied.

default-setup.log was created by composing a mail with a text attachment in HTML mode and hitting the "save" button. It can be clearly seen, that the default delimiter (automatically assigned to "\n") is utterly wrong and results in cyrus's response "NO Message contains bare newlines".

manual-delimiter.log was created by the same action after changing

$rcmail_config['mail_header_delimiter'] = "\r\n";

In this case, almost all content is sent correctly, except for two linefeeds in the plain-text part.

manual-delimiter-textonly.log finally shows success.

Conclusion:

The problem is twofold:

  • IMHO, the concept of using mail_header_delimiter is wrong. IMAP should not use the mail_header_delimiter. Instead, it should always use CRLF. If i guess why it was introduced in the first place, i can imagine, that on Unix - when piping messages into external commands - it might be necessary to us linefeeds.
  • When composing html messages, the creation of the plain-text part should consider that and always create CRLF line endings.

Will report more findings when I have debugged the issue further ...

Cheers

-Fritz

comment:2 Changed 3 years ago by felfert

  • Cc fritz@… added

Changed 3 years ago by felfert

Patch to fix delimiters of the plain-text alternative part when composing html mail.

comment:3 Changed 3 years ago by felfert

Hi again,

I just attached htmlmail-save-delimiterfix.patch which fixes the error seen in the testcase shown in manual-delimiter.log.

Verbose description:
When composing a html mail, an alternative text/plain part is generated in program/steps/mail/sendmail.inc, around line 430. There, the class html2text is used to parse the html part and produce the plain text. This class always returns single linefeeds in the return value of its method get_text(). Therefore, that value has to be converted to use CRLF before being processed further.

comment:4 follow-up: Changed 3 years ago by alec

Could you provide an imap account on your server for testing?

comment:5 in reply to: ↑ 4 Changed 3 years ago by felfert

Replying to alec:

Could you provide an imap account on your server for testing?

Unfortunately not (it's a customer installation). However i can offer to setup an identical server on VMWare or VirtualBox? (your choice) and give you that VM-Image so you can test at your location.
Please let me know if that's ok for you and - if yes - your personal preference (VBox or VMWare)

Cheers

-Fritz

comment:6 follow-up: Changed 3 years ago by rgalps

Hi There

I'm having a pretty much identical issue with 0.4-beta and cyrus imap, but instead of when saving a message, it occurs when sending a message that has an attachment:

May 12 18:08:06 mta1 roundcube: [12-May-2010 18:08:06 +0100]: User russ@… [xxx.xxx.xxx.xxx]; Message for <xxx@xxx>; 250: 2.0.0 Ok: queued as D058416D7
May 12 18:08:06 mta1 roundcube: [12-May-2010 18:08:06 +0100]: IMAP Error: Could not save message in Sent in /home/webmail2/public_html/program/steps/mail/sendmail.inc on line 621 (POST /?_task=mail&_action=send)

The error returned from cyrus is: "a NO Message contains bare newlines".

I've applied the patch suggested here (even thought it only seems valid for HTML emails) - it's had no affect, same problem. Doesn't matter what the attachment is (I've tried a tiny text file with 'hello' in it, and a 12MB file) always the same error. Save case with HTML / Plain Text emails.

I can provide a test IMAP account to replicate the issue if required.

Please let me know if any further is needed.

Thanks

Russ

comment:7 in reply to: ↑ 6 ; follow-up: Changed 3 years ago by felfert

Replying to rgalps:

[...]
I've applied the patch suggested here (even thought it only seems valid for HTML emails) - it's had no affect, same problem. Doesn't matter what the attachment is (I've tried a tiny text file with 'hello' in it, and a 12MB file) always the same error. Save case with HTML / Plain Text emails.
[...]

The patch fixes only the conversion routine. In order to make it work completely, you also have to set the mail_header_delimiter config variable to "\r\n" (See my description of logfiles above).
Apart from that there's no difference between saving a mail during (actually after) sending and saving a mail as draft. The code is the same, just the destination folder is (usually) a different one.

Cheers

-Fritz

comment:8 in reply to: ↑ 7 ; follow-up: Changed 3 years ago by rgalps

Replying to felfert:

The patch fixes only the conversion routine. In order to make it work completely, you also have to set the mail_header_delimiter config variable to "\r\n" (See my description of logfiles above).
Apart from that there's no difference between saving a mail during (actually after) sending and saving a mail as draft. The code is the same, just the destination folder is (usually) a different one.

Thanks Fritz

I can confirm that the patch does fix the issue as long as you have the mail_header_delimiter set - that was the step I was missing.

Hopefully this can be fixed a bit more eloquently so we don't have to manually set that config param in a later release.

Russ

comment:9 in reply to: ↑ 8 Changed 3 years ago by felfert

Replying to rgalps:

Hopefully this can be fixed a bit more eloquently so we don't have to manually set that config param in a later release.

I intentionally left that to the core developers. In my opinion, that variable should not be necessary at all (if all targeted IMAP servers stick to RFCs, it could simply be hard coded to "\r\n"). Unfortunately, there is nothing in the SVN commit logs mentioning the reason why it was made configurable in the first place. Anyway, the current implementation of setting it automatically depending on the OS of the host running the PHP script is definitively wrong. If variable at all, the "automatic" should depend on the IMAP server being used.

Cheers

-Fritz

comment:10 Changed 3 years ago by alec

On http://pl.php.net/manual/en/function.mail.php is a note "If messages are not received, try using a LF (\n) only. Some poor quality Unix mail transfer agents replace LF by CRLF automatically (which leads to doubling CR if CRLF is used). This should be a last resort, as it does not comply with » RFC 2822." This may be why mail_header_delimiter has been provided. If so, I think we could use "\r\n" everywhere and just convert if needed for mail() function (just right before its use).

comment:11 Changed 3 years ago by thomasb

I agree to Alec's suggestion. Roundcube doesn't know much about the IMAP backend and therefore should stick to RFC. But (fortunately) we know when an installation is using PHP's crappy mail() function and we could react on that by converting CRLF with the value configured in mail_header_delimiter.

BTW: it's correct that this mail() function and it's unpredictable behavior was the reason why we introduced the mail_header_delimiter config option. There was an issue with Gmail which could not read messages sent with CRLF using mail().

comment:12 Changed 3 years ago by alec

  • Resolution set to fixed
  • Status changed from new to closed

Fixed in [ac8edbed].

comment:13 Changed 3 years ago by mschiff

  • Resolution fixed deleted
  • Severity changed from blocker to normal
  • Status changed from closed to reopened
  • Version changed from 0.4-beta to 0.4-stable

Sorry if reopening here is wrong, but I got the same issue with 0.4stable.

With the default setting of NULL (auto-detect) of mail_header_delimiter I am not able to store messages having attachments to the Sent folder (Cyrus IMAP)

After setting mail_header_delimiter to "\r\n" it works mostly.

But I found one reproducable case where the problem still occurs:

  1. Have RC using IMAP (Cyrus)
  2. Send a mail to yourself with an attachment: That works now.

(3.) Message arrives at your INBOX

  1. Select that message, then open message as new message
  2. Now in the composer, remove the attachment, then try to send -> Same old error again

Looking at the imap debug logs that last header line looks abit odd (opened text file using vi so I can see the different line endings)

----------------------------------------------------------
[02-Sep-2010 16:02:56 +0200]: C: MIME-Version: 1.0^M
Content-Type: text/plain;^M
 charset=UTF-8;^M
 format=flowed^M
Content-Transfer-Encoding: 7bit^M
Date: Thu, 02 Sep 2010 16:02:56 +0200^M
From: Foo Bar <foo@bar.net>^M
To: Foo Bar <foo@bar.net>^M
Subject: test^M
Message-ID: <5f121bda4c1c3aff7368cdcaa55d1bf3@localhost>^M
X-Sender: foo@bar.net^M
User-Agent: Roundcube Webmail/0.4\r\n\r\n
[02-Sep-2010 16:02:56 +0200]: C: --
[02-Sep-2010 16:02:56 +0200]: C: 8AAC 5F46 83B4 DB70 8317  3723 296C 6CCA 35A6 4134
[02-Sep-2010 16:02:56 +0200]: C:
----------------------------------------------------------

M is a real CR (\r) (vi shows this because it did not convert the file due to existing LF endings)

So it seems RC seems to do some escaping on the last header line... thats why I see "\r\n\r\n" instead of M and another one in the next line.

Can anybody confirm?

Last edited 2 years ago by alec (previous) (diff)

comment:15 Changed 3 years ago by alec

  • Milestone changed from 0.4-stable to 0.4.1

comment:16 Changed 3 years ago by alec

Check your config again and make sure you have no duplicated mail_header_delimiter setting, also remember that "\r\n" and '\r\n' is not the same.

comment:17 Changed 3 years ago by alec

  • Resolution set to fixed
  • Status changed from reopened to closed

I think [086767c8] and [272a7e5a] should fix the issue definitely.

Note: See TracTickets for help on using tickets.