The current code makes the assumption that a Cc header will be "Cc" and the Bcc header "Bcc", and does not protect against different capitalizations, such as "cc", "BCC", etc.
This patch ensures that these headers will be suppressed regardless of case.
I removed the test for if (isset($message['headers']) && is_array($message['headers']) because that seemed a little redundant immediately after
$message['headers']['X-Rerouted-Website'] = $base_url;
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | Re-rolled patch from #1 with a few minor changes to improve the robustness of Cc/Bcc headers suppression. | 1.59 KB | dydave |
| #1 | cc_bcc-1571508.patch | 2.21 KB | jweowu |
Comments
Comment #1
jweowu commentedComment #2
jweowu commentedComment #3
dydave commentedHi jweowu,
I certainly apologize for the delay of this reply, but it took me a while to get back to you after going through the stack of issues in the tracker.
Thank you so much for that: This patch really looks great!
Indeed, there seems to be an issue with the case that is used for the keys in the
$message['headers']associative Array and I really like the solution you suggested trying to flatten all header keys in lower case.Obviously, this is something that has been reported at #1701578: cc'd recipients get email when reroute is on, and that we're most likely going to want to roll-in after some more reviews and testing from the community.
I allowed myself to re-roll the initial patch and make a few minor changes.
Please find attached to this comment a patch against reroute_email-7.x-1.x at 4930477 which is just a re-roll of the initial patch submitted at #1 by @jweowu.
File attached as: reroute_email-more-robust-cc-bcc-suppression-1571508-3.patch.
This new patch has been tested and seems to work as expected, but I would greatly appreciate to have your feedback, questions, comments, reviews, suggestions, recommendations, improvements and testing of this new patch.
Feel free to let me know if you would have any further comments, issues, questions, objections, recommendations, suggestions, testing, reporting or concerns on the attached patch or any other aspects of this ticket in general, I would be glad to provide more information or explain in further details.
Thanks in advance to all for your feedback, reviews, testing and reporting.
Cheers!
Comment #4
dydave commentedHi guys,
Sorry again for stalling on this for almost a month, but since there hasn't been any feedback on the patch from #3, I wanted to wait a bit more.
In any case, I have tested this in code and it seems to work as expected.
Since there was no further objections, I went ahead and committed the patch at:
I allowed myself to mark this issue as fixed for now, but feel free to re-open it, or post a new ticket, at any time if you have any further objections with the solution suggested at #1 (we would surely be happy to hear your feedback).
Please let me know if you would have any further comments, feedback, questions, issues, objections, suggestions or concerns on any of these commits or this ticket in general, I would be glad to provide more information or explain in more details.
Thanks again to everyone for your help, reviews, feedback and comments on this issue.
Cheers!
Comment #6
dydave commentedHi guys,
I wanted to get back to this issue quickly because I think we may have forgotten one more important thing with this feature, perhaps updating Reroute Email test cases and getting this change to be tested in code.
So I modified the Test Case
RerouteEmailSpecialTestCasewhich was added at #488032-32: Respect a string in body when that's what we get (committed at 0439561 to test Reroute Email's handling of the message body when it is passed as a string) and added a few more tests to check rerouted email's header keys when they are provided with an unexpected case.The email sent for this test is triggered by a hidden test helper sub-module reroute_email_test, which was modified to provide Cc and Bcc key headers with an unexpected case.
In this particular case, we are testing with
cCandbCcheader keys.After getting some feedback and discussing these tests at #488032: Respect a string in body when that's what we get, I allowed myself to commit these changes directly against the 7.x-1.x branch at 8a5ed26.
Feel free to re-open this issue, or post a new ticket, at any time if you have any further objections with this update/change or any of the related commits (0439561, 8a5ed26 - we would surely be happy to hear your feedback).
Please let me know if you would have any further comments, feedback, questions, issues, objections, suggestions or concerns on any of these commits or this ticket in general, I would be glad to provide more information or explain in more details.
Thanks again to everyone for your help, reviews, feedback and comments on this issue.
Cheers!