Message::getSender creates a blank Sender header #93
Conversation
RFC5322 section 3.6.2 states that if the originator of the message can be indicated by a single mailbox and the author and transmitter are identical, the "Sender:" field SHOULD NOT be used.
Message::getSender() unintentionally creates an empty Sender header.
|
Bump? |
|
@Ocramius Any chance you can review this? It seems to have gotten lost in the background noise. |
| @@ -314,6 +314,10 @@ public function setSender($emailOrAddress, $name = null) | |||
| */ | |||
| public function getSender() | |||
| { | |||
| $headers = $this->getHeaders(); | |||
There was a problem hiding this comment.
This change seems unrelated: what call did cause the header to be added? You are only fixing the symptom here, IMO
There was a problem hiding this comment.
Calling Message::getHeaderByName will create a new blank header if the requested header does not exist, as per the docbloc.
The existing strategy as per Message::getSubject is to check if the header exists before calling getHeaderByName. (The existing code also uses a local variable for $headers.)
There was a problem hiding this comment.
@Ocramius Could you review please?
|
EDIT: This comment was completely insensitive and wrong, please see my response below. Leaving the original message here for historical purposes. @weierophinney @Ocramius @akrabat @Bittarman @EvanDotPro @ezimuel @Maks3w @Freeaqingme Does no one care about bugs in Zend Framework anymore? This is a well defined, tested and audited bugfix that has been ignored for over 6 months. I have done all the work, all that is needed is 20 seconds of confirmation and a merge. I don't know what more I can do. |
|
I'll be honest here: I'm sitting at work doing my own shit. Next time you
even assume that I don't care (I speak for myself) I will effectively stop
caring, and you'll be free to fork the project.
…On 13 Feb 2017 11:00 a.m., "Michael Gooden" ***@***.***> wrote:
@weierophinney <https://github.com/weierophinney> @Ocramius
<https://github.com/Ocramius> @akrabat <https://github.com/akrabat>
@Bittarman <https://github.com/Bittarman> @EvanDotPro
<https://github.com/EvanDotPro> @ezimuel <https://github.com/ezimuel>
@Maks3w <https://github.com/Maks3w> @Freeaqingme
<https://github.com/Freeaqingme> Does no one care about bugs in Zend
Framework anymore? This is a well defined, tested and audited bugfix that
has been ignored for over 6 months. I have done all the work, all that is
needed is 20 seconds of confirmation and a merge. I don't know what more I
can do.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#93 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakPFKzQhNCWF5sDFRt97ZYkBGpJadks5rcCnBgaJpZM4Iyzgc>
.
|
|
You think that people don't care because they haven't had time to merge your specific bug? I'm saddened by the level of entitlement in open source at the moment. I honestly appreciate the effort you've put into this fix, but if you think that merging this is a 20-second job you are sadly mistaken. The community review team are now supporting 131 repositories according to GitHub, which is an extraordinary amount of work for such a small team, and the number of paid contributors is exactly two. I've pinged @weierophinney internally to ask him to merge this, but please, when responding to other open source maintainers, please practice empathy. If you think you can do a better job or would like to volunteer to be a maintainer of this repository then please step up and email Matthew - at this point, all your comments are doing is alienating people and making them frustrated and angry. Gary. |
|
@Ocramius, @GeeH, and others: I am massively, entirely sorry for the comment I left on this issue. I am not going to make any excuses because what I said was just plain wrong. I am hugely grateful and appreciative towards all the contributors for creating the tools that allow me to eek out a livelihood for myself. I hope you can accept my apologies and believe me when I say I had no intention of offending anyone or acting entitled. Michael |
|
@MichaelGooden 🍻 alright! Sorry for the over-reaction, probably a bad day for me as well (had another few of these reactions lately, so I'm a bit jittery when they come up) |
|
Indeed, I don't think you need to apologise @MichaelGooden - I was just wanting to make a point. However apologising definitely makes you the better person 😄 . 🍺 |
Message::getSender creates a blank Sender header
|
Thanks, @MichaelGooden |
The
getSender()method ofMessageunintentionally creates a blank Sender header.The expected behaviour is to return
nullif the Sender header does not exist however the current implementation creates a blank Sender header in the process. Subsequent calls to getSender will return a blank header instead of thenullthat is expected.This directly affects Transport\Smtp as getSender is used in the sending logic. Therefore, this PR has a test against Smtp as well as against Message. This might be redundant. It does however test expected Smtp functionality, so comments on that tests inclusion in the final patch are welcome.