Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

unable to parse To headers with semicolon separator #147

Merged

Conversation

glensc
Copy link
Contributor

@glensc glensc commented May 20, 2017

i wonder why Zend\Mail does not split on semicolon?

  • it causes parse error on headers that are semicolon separated: To: user1@domain.com; user2@domain.org. to my knowledge outlook sends out such mails: link
  • it causes "group" feature to be misparsed: To: undisclosed recipients:; . another example

this PR adds testcase that should not fail

glensc added a commit to eventum/eventum that referenced this pull request May 20, 2017
parser gives "The input exceeds the allowed length" error
if headers are separated by semicolon

zendframework/zend-mail#147

\Zend\Mail\Header\Exception\InvalidArgumentException extends
\Zend\Mail\Exception\InvalidArgumentException

so both exceptions covered
@weierophinney
Copy link
Member

Your unit test is malformed. Instead of a failing assertion, an exception is being raised due to a line that is longer than allowed.

Additionally, I'm not quite sure how to address this. The , character is the one documented as separating fields in the various IETF mail specifications, which makes adding an alternate case harder.

Please try and fix the unit test so that we know we're failing due to the assertEquals() assertion, and then we can potentially start working on this.

@glensc
Copy link
Contributor Author

glensc commented Jun 8, 2017

the test case is written with "fixed" code in mind. it will pass if code behaves as expected.

in other words, the code paths used should not throw, but it throws because it encounters different error (field too long)

i don't see how you want this to be fixed or why. assertions are also exceptions internally. looks pointless eeb0031 the wrap is not needed once the code is fixed.

glensc referenced this pull request Jun 8, 2017
@glensc
Copy link
Contributor Author

glensc commented Jun 12, 2017

@weierophinney requested changes are made altho i slightly disagree with them!

@Ocramius Ocramius requested a review from Xerkus March 1, 2018 18:12
@weierophinney weierophinney force-pushed the address-semicolon-separator branch from eeb0031 to 8ededca Compare June 6, 2018 16:43
weierophinney added a commit to eventum/zend-mail that referenced this pull request Jun 6, 2018
This patch adds a new class, `AddressListParser`, with a single static
method `parse()`. It loops through each character of the value to
identify non-escaped, non-quoted delimiters, allowing either `,` or `;`
to be used.

`AbstractAddressList::fromString()` now uses the above method instead of
`str_getcsv()` to extract the list of addresses.

The patch also updates the test provided in zendframework#147 to test for the
existence of all addresses in the test string.
weierophinney added a commit to eventum/zend-mail that referenced this pull request Jun 6, 2018
Also removes stub for 3.0.0; master branch is targeted at 2.9.x-dev.
@weierophinney weierophinney force-pushed the address-semicolon-separator branch from c923715 to 1345849 Compare June 6, 2018 17:12
weierophinney added a commit to eventum/zend-mail that referenced this pull request Jun 6, 2018
This patch adds a new class, `AddressListParser`, with a single static
method `parse()`. It loops through each character of the value to
identify non-escaped, non-quoted delimiters, allowing either `,` or `;`
to be used.

`AbstractAddressList::fromString()` now uses the above method instead of
`str_getcsv()` to extract the list of addresses.

The patch also updates the test provided in zendframework#147 to test for the
existence of all addresses in the test string.
weierophinney added a commit to eventum/zend-mail that referenced this pull request Jun 6, 2018
Also removes stub for 3.0.0; master branch is targeted at 2.9.x-dev.
This patch adds a new class, `AddressListParser`, with a single static
method `parse()`. It loops through each character of the value to
identify non-escaped, non-quoted delimiters, allowing either `,` or `;`
to be used.

`AbstractAddressList::fromString()` now uses the above method instead of
`str_getcsv()` to extract the list of addresses.

The patch also updates the test provided in zendframework#147 to test for the
existence of all addresses in the test string.
Also removes stub for 3.0.0; master branch is targeted at 2.9.x-dev.
@weierophinney weierophinney force-pushed the address-semicolon-separator branch from 1345849 to 83f9dde Compare June 6, 2018 18:19
@weierophinney weierophinney merged commit 83f9dde into zendframework:master Jun 6, 2018
weierophinney added a commit that referenced this pull request Jun 6, 2018
weierophinney added a commit that referenced this pull request Jun 6, 2018
@weierophinney
Copy link
Member

Thanks, @glensc, for the test; got a good fix in place now.

@glensc glensc deleted the address-semicolon-separator branch June 6, 2018 18:40
glensc added a commit to eventum/zend-mail that referenced this pull request Mar 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants