Support commas in names

[davidwindell]

Bad things happen when an email header has a comma in the name, this fixes that.

[Ocramius]

@davidwindell your branch needs to be rebased

[Ocramius]

@davidwindell are you based on current master? Check if your master is in sync with upstream

[DASPRiD]

Please add new tests instead of modifying existing ones.

[DASPRiD]

Why was this test modified?

[davidwindell]

Because I'm using str_getcsv it's removing the enclosure characters ", I'm not sure if this matters or if there is a clean way around it.

[DASPRiD]

This actively modifies the behaviour, thus it'd be a BC break. Existing tests should not be altered. Exception is if the test would be wrong of course.

[davidwindell]

Ok, I'll leave it to someone else to implement as not sure how else to do but with str_getcsv

Although, one could say the test was wrong on the basis that the enclosure is part of the "encoding" and this is getting the "decoded" version.

[DASPRiD]

Looking at the test, it does exactly what it should.

[DASPRiD]

I'm not too much into perl, but as far as I can see, $string_ref is a reference to the string there, and what the loop actually does is: Parse address from the string, then advance to the next comma, then repeat.

[davidwindell]

I don't think that is going to help us then :(, what we need is a simple regex that will split a string on commas, but not where they are enclosed by speech marks (")

[DASPRiD]

That's not that easy, because quotation marks can be escaped.

[DASPRiD]

Anyway, I'm gonna take a look at a possible regex solution later today.

[davidwindell]

That's why I had hoped str_getcsv would work ;) :(

Thank you sir!

[davidwindell]

As per previous discussion, tests have been reverted. There is a problem however in that speech marks are stripped out by str_getcsv. @DASPRiD is looking into a regex solution for this instead.

[DASPRiD]

The problem seems to be larger than expected. Zend\Mail does not handle quoted names at all right now, so this will take a more complex solution.

[Ocramius]

Clearing milestone, re-assigning to @DASPRiD

[adamlundrigan]

@DASPRiD @davidwindell could you add a brief-ish synopsis of what's left to do on this?

[weierophinney]

I'm assigning to 2.4. While we likely need a more robust solution, we simply need any solution at this time.

[weierophinney]

@davidwindell I just attempted to merge this, but have a new failing test: ZendTest\Mail\Storage\MessageTest::testGetDecodedHeader now fails, asserting that:

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'"Peter Müller" <peter-mueller@example.com>'
+'Peter Müller <peter-mueller@example.com>'

Could you please investigate?

[davidwindell]

@weierophinney Are you happy for test to be modified to remove the speech marks around the name? That's what str_getcsv will do and was what prompted Dasprid's concerns with the PR.

[weierophinney]

@davidwindell I think that would be fine; they're an optional part of the standard, and do not affect usability within messages in any meaningful way.

[davidwindell]

I agree, I have updated the test so this should be ready to merge.

[weierophinney]

Merged to develop for release with 2.4.