testcase for broken Header\AbstractAddressList::fromString

[glensc]

Problem:

1. GenericHeader loads the header in file, decodes it to utf-8
2. the Headers::get attempts to Lazy-Load "To" header class

Lazyload does stringify and load in from string

$encoding = $current->getEncoding();
$headers  = $class::fromString($current->toString());

However, toString does not encode comma
AND To header class does split on comma!

see \Zend\Mail\Header\AbstractAddressList::fromString

this PR shows only the problem.

[glensc]

toString invocation path:

• GenericHeader::toString
• GenericHeader::getFieldValue
• HeaderWrap::wrap
• HeaderWrap::wrapUnstructuredHeader
• HeaderWrap::mimeEncodeValue
• [Mime::encodeQuotedPrintableHeader]
• [Mime::_encodeQuotedPrintable]
• ... it got scary here

[glensc]

perhaps the fix is in zend-mime project:
zendframework/zend-mime#26

if it gets accepted!

[glensc]

after zendframework/zend-mime#26 being applied,
such code with Zend\Mail:

$headerLine = 'To: "=?iso-8859-1?Q?W=2C_bj=F8rn?=" <user@example.org>';
echo Mail\Header\GenericHeader::fromString($headerLine)->toString();

before:

To: =?UTF-8?Q?"W,=20bj=C3=B8rn"=20<user@example.org>?=

now:

To: =?UTF-8?Q?"W=2C=20bj=C3=B8rn"=20<user@example.org>?=

and

// with headerline without comma:
$headerLine = 'To: =?UTF-8?Q?"W=2C=20bj=C3=B8rn"=20<user@example.org>?=';
echo Mail\Header\To::fromString($headerLine)->toString();

To: =?UTF-8?Q?"W=2C=20bj=C3=B8rn"?= <user@example.org>

[glensc]

@weierophinney closed this in zendframework/zend-mime@73e6d05 15 days ago

this is not correct. reopen please.

[weierophinney]

@glensc I think I'm not understanding something.

The original string includes an encoded comma (=2c).

Why should the comma NOT be encoded when the To header is cast to a string? Is it because of the different encodings (iso-8859-1 vs UTF-8)?

(I need to understand why your expectation should be the expected behavior, basically.)

[glensc]

@weierophinney it's combination of these two statements from Pull Description:

• However, toString does not encode comma
• AND To header class does split on comma!

i tried to explain the problem in test comments as well: https://github.com/zendframework/zend-mail/pull/146/files

if my explanation is not understandable (not sure i catched your question), just see the test code and how it behaves.

it's a lot of information and i dealed with this problem more than year ago....

so, this seemed the simplest way to solve the problem. it's not forbidden to zealously encode

as zendframework/zend-mime@73e6d05 was accepted in zendframework/zend-mime#26

this just updates unit test data.

[weierophinney]

This test case doesn't make sense given the assertions in your comments and the issue description.

You're making the case that AbstractAddressList::fromString() is splitting on a comma, but that toString() on such headers is not encoding it.

What I'm seeing here is quite different:

• You're testing the behavior of a GenericHeader, not one that is based on AbstractAddressList.
• Your assertion is that the expected behavior of toString() is NOT to encode the comma.

That's the crux of my confusion: the test is not setting up the conditions you describe, nor testing the expectation you describe.

Can you please clarify?

[weierophinney]

This repository has been moved to laminas/laminas-mail. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

• Squash all commits in your branch (git rebase -i origin/{branch})
• Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
• Run the laminas/laminas-migration tool on the code.
• Clone laminas/laminas-mail to another directory.
• Copy the files from the second bullet point to the clone of laminas/laminas-mail.
• In your clone of laminas/laminas-mail, commit the files, push to your fork, and open the new PR.
  We will be providing tooling via laminas/laminas-migration soon to help automate the process.

[michalbundyra]

This repository has been closed and moved to laminas/laminas-mail; a new issue has been opened at laminas/laminas-mail#44.