Fix email headers when using long email subjects and \r\n as crlf. #1709

Merged
merged 3 commits into from Oct 8, 2012

Conversation

Projects
None yet
5 participants
@bbarao
Contributor

bbarao commented Aug 14, 2012

Issue

When using long email subjects and \r\n as crlf, the header is badly formatted, and either the email appears with headers on the body, or doesn't arrive at all.

The problem is that on the preg_replace, the multiline mode only matches the \n leaving the \r to be matched on the regex.

Examples

I'm showing the part of the header where the subject is set.

Current Code (notice the ^M on the first two lines)

Subject: =?utf-8?Q?Big_email_subject_with_some_non_ascii_chars:_=c3=a0_=c3=a1_=c3^M?=
 =?utf-8?Q?=a9_=c3=a8_=c3=b6_=c3=af_@_=e2=82=ac_=c2=a9_=c2=ae_>_=c2=bb_an^M?=
 =?utf-8?Q?d_so_on,_and_so_on...?=^M

Patched Code

Subject: =?utf-8?Q?Big_email_subject_with_some_non_ascii_chars:_=c3=a0_=c3=a1_=c3?=^M
 =?utf-8?Q?=a9_=c3=a8_=c3=b6_=c3=af_@_=e2=82=ac_=c2=a9_=c2=ae_>_=c2=bb_an?=^M
 =?utf-8?Q?d_so_on,_and_so_on...?=^M
system/libraries/Email.php
@@ -1228,7 +1228,7 @@ protected function _prep_q_encoding($str, $from = FALSE)
// wrap each line with the shebang, charset, and transfer encoding
// the preceding space on successive lines is required for header "folding"
- return trim(preg_replace('/^(.*)$/m', ' =?'.$this->charset.'?Q?$1?=', $output.$temp));
+ return trim(preg_replace('/^(.*?)(\n|\r)*$/m', ' =?'.$this->charset.'?Q?$1?=$2', $output.$temp));

This comment has been minimized.

Show comment Hide comment
@narfbg

narfbg Aug 15, 2012

Contributor

Why (.*?) ?

@narfbg

narfbg Aug 15, 2012

Contributor

Why (.*?) ?

This comment has been minimized.

Show comment Hide comment
@bbarao

bbarao Aug 15, 2012

Contributor

To make (.*) non-greedy, and prevent it to capture \r

@bbarao

bbarao Aug 15, 2012

Contributor

To make (.*) non-greedy, and prevent it to capture \r

@narfbg

This comment has been minimized.

Show comment Hide comment
@narfbg

narfbg Aug 16, 2012

Contributor

Could you add a changelog entry describing what's fixed? And if possible, can somebody confirm if this fixes #1409?

Contributor

narfbg commented Aug 16, 2012

Could you add a changelog entry describing what's fixed? And if possible, can somebody confirm if this fixes #1409?

@bbarao

This comment has been minimized.

Show comment Hide comment
@bbarao

bbarao Aug 17, 2012

Contributor

I will do it later today or tomorow.

Contributor

bbarao commented Aug 17, 2012

I will do it later today or tomorow.

@alexbilbie

This comment has been minimized.

Show comment Hide comment
@alexbilbie

alexbilbie Aug 30, 2012

Contributor

Did you add the changelog @bbarao so we can pull this?

Contributor

alexbilbie commented Aug 30, 2012

Did you add the changelog @bbarao so we can pull this?

@Chronial

This comment has been minimized.

Show comment Hide comment
@Chronial

Chronial Sep 19, 2012

Sry for this pointless comment, but please get this in the code – this bug is very annoying.

Sry for this pointless comment, but please get this in the code – this bug is very annoying.

@ckdarby

This comment has been minimized.

Show comment Hide comment
@ckdarby

ckdarby Sep 19, 2012

Contributor

@Chronial Please don't comment; Please fix & make the pull request

Contributor

ckdarby commented Sep 19, 2012

@Chronial Please don't comment; Please fix & make the pull request

@narfbg

This comment has been minimized.

Show comment Hide comment
@narfbg

narfbg Oct 5, 2012

Contributor

Still waiting on this one to be updated ... @bbarao ?

Contributor

narfbg commented Oct 5, 2012

Still waiting on this one to be updated ... @bbarao ?

@bbarao

This comment has been minimized.

Show comment Hide comment
@bbarao

bbarao Oct 8, 2012

Contributor

Sorry about the delay. Is the changelog entry in the right place?

Contributor

bbarao commented Oct 8, 2012

Sorry about the delay. Is the changelog entry in the right place?

narfbg added a commit that referenced this pull request Oct 8, 2012

Merge pull request #1709 from bbarao/develop
Fix email headers when using long email subjects and \r\n as crlf.

@narfbg narfbg merged commit 60e719b into bcit-ci:develop Oct 8, 2012

1 check passed

default The Travis build passed
Details

narfbg added a commit that referenced this pull request Oct 8, 2012

@narfbg

This comment has been minimized.

Show comment Hide comment
@narfbg

narfbg Oct 8, 2012

Contributor

Corrected what's needed, thanks @bbarao.

Contributor

narfbg commented Oct 8, 2012

Corrected what's needed, thanks @bbarao.

@narfbg

This comment has been minimized.

Show comment Hide comment
@narfbg

narfbg Oct 9, 2012

Contributor

@bbarao Can you tell if this is the same issue as #1409, #1498?

Contributor

narfbg commented Oct 9, 2012

@bbarao Can you tell if this is the same issue as #1409, #1498?

@Chronial

This comment has been minimized.

Show comment Hide comment
@Chronial

Chronial Oct 9, 2012

This did definitely not fix #1498

Chronial commented Oct 9, 2012

This did definitely not fix #1498

nonchip pushed a commit to nonchip/CodeIgniter that referenced this pull request Jun 29, 2013

Merge pull request #1709 from bbarao/develop
Fix email headers when using long email subjects and \r\n as crlf.

nonchip pushed a commit to nonchip/CodeIgniter that referenced this pull request Jun 29, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment