Edgewall Software
Modify

Opened 11 years ago

Closed 11 years ago

#10377 closed defect (fixed)

[PATCH] remove spurious CR characters in notification via sendmail

Reported by: d.love@… Owned by: Remy Blank
Priority: normal Milestone: 0.12.3
Component: notification Version: 0.12.2
Severity: normal Keywords:
Cc: Thijs Triemstra Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Part of the notification email sent via sendmail has spurious CRLF line endings which either show up for the recipient as literal CR characters (and maybe provoke a non-text/plain MIME type) or double-spaced lines. At least that's the case with Postfix on a GNU/Linux system, and I guess it's a general issue with sendmail programs.

I'm using this patch to fix it with 0.12.2; there doesn't seem to be any relevant change in the trunk. It's not obvious how to write an appropriate test.

Attachments (2)

notification.py.diff (666 bytes ) - added by d.love@… 11 years ago.
10377-notification-eol-r10809.patch (7.8 KB ) - added by Remy Blank 11 years ago.
Construct notification message with '\n' instead of CRLF.

Download all attachments as: .zip

Change History (6)

by d.love@…, 11 years ago

Attachment: notification.py.diff added

comment:1 by Remy Blank, 11 years ago

Milestone: 0.12.3
Owner: set to Remy Blank

Thanks for the patch. I'll check that. The strange thing is, I tested the implementation with Potstfix on Linux as well, so I wonder how I could miss that.

comment:2 by Thijs Triemstra, 11 years ago

Cc: Thijs Triemstra added

by Remy Blank, 11 years ago

Construct notification message with '\n' instead of CRLF.

comment:3 by Remy Blank, 11 years ago

I tested again, and I see no spurious CR characters here (Postfix 2.8.4 on Linux). sendmail seems to be resilient against CRLF.

But your patch is still correct. CRLF is required when talking to a SMTP server, but not with a local sendmail program. I have applied a slightly different patch in [10809], by using the same code as for ensuring the message has CRLF everywhere for SmtpMessageSender.

I wonder, however, why we construct the message with CRLF instead of '\n'. The result is a hybrid text where the headers are separated by '\n' and the body lines are separated by CRLF. As both email senders now convert the message to the correct EOL, we should generate the message with only '\n' line endings. See 10377-notification-eol-r10809.patch. All tests pass.

Opinions?

comment:4 by Remy Blank, 11 years ago

Resolution: fixed
Status: newclosed

After discussion with cboos, applied the patch in [10810], together with a refactoring of the EOL-fixing code.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.