Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Sendmail.php #235

Open
wants to merge 3 commits into
base: 2.23.x
Choose a base branch
from
Open

Update Sendmail.php #235

wants to merge 3 commits into from

Conversation

cloudseeder
Copy link

Remove the \r\n translation for mail headers per the PHP mail() page.

additional_headers (optional)

String or array to be inserted at the end of the email header.

This is typically used to add extra headers (From, Cc, and Bcc). Multiple extra headers should be separated with a CRLF (\r\n). If outside data are used to compose this header, the data should be sanitized so that no unwanted headers could be injected.

Q A
Documentation no
Bugfix yes
BC Break no
New Feature no
RFC no
QA no

Description

This is fully documented in osTicket BR #230.

Remove the \r\n translation for mail headers per the PHP mail() page. 

additional_headers (optional)
----------
String or array to be inserted at the end of the email header.

This is typically used to add extra headers (From, Cc, and Bcc). Multiple extra headers should be separated with a CRLF (\r\n). If outside data are used to compose this header, the data should be sanitized so that no unwanted headers could be injected.



Signed-off-by: cloudseeder <[email protected]>
@Slamdunk
Copy link
Contributor

Slamdunk commented May 4, 2023

Hi, can you add a test for the bugfix please?

Signed-off-by: cloudseeder <[email protected]>
Signed-off-by: cloudseeder <[email protected]>
@cloudseeder
Copy link
Author

@Slamdunk

Sure. Can you point me to an example? Looks like there are a lot of good examples in MessageTest.php. Should I just copy/mod one of those tests? With all those tests I'm really surprised this wasn't caught earlier.

So what I think I need to do is:

  1. Edit MessageTest.php
  2. Copy and modify one of the existing tests to meet the needs of this PR
  3. Ensure it works
  4. Add the test to this PR (is that correct?)

I added a new file into my fork ([cloudseeder/laminas-mail/tree/2.23.x/test)/Storage/HeaderTest.php) and some ci event kicked off from the main branch. Just want to make sure I do this correctly. I'm more of a ops guy than a programmer.

@Ocramius Ocramius linked an issue May 22, 2023 that may be closed by this pull request
@Slamdunk
Copy link
Contributor

That's correct: go for it 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use "text/html" content type instead of "multipart/alternative"
2 participants