Skip to content

Conversation

@yohgaki
Copy link
Contributor

@yohgaki yohgaki commented Aug 6, 2016

Implement optional array extra mail headers.
https://bugs.php.net/bug.php?id=69791

/* https://tools.ietf.org/html/rfc2822#section-3.6 */
switch(ZSTR_LEN(key)) {
case sizeof("orig-date")-1:
if (!strncasecmp("orig-date", ZSTR_VAL(key), ZSTR_LEN(key))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This set of forbidden headers will most certainly break existing libraries. See e.g. here.

Copy link
Contributor Author

@yohgaki yohgaki Aug 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lstrojny TO and SUBJECT headers can be used only once and mail()/mb_send_mail() sets these by "to" and "subject" parameters. mail()/mb_send_mail() users can only set "extra_headers" once and illegal header counts is illegal.
I'm not sure why it causes BC. Could you explain why?

NOTE: It does not care for MIME headers inside mail body at all, but only mail headers of SMTP.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yohgaki sorry, I misread the patch, all good 👍

} \
php_mail_build_headers_elems(&s, key, val); \
} else { \
php_error_docref(NULL, E_WARNING, "Extra header element '%s' cannot be other than stirng or array.", ZSTR_VAL(key)); \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in the word 'string' here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aboks Thank you!!

@yohgaki
Copy link
Contributor Author

yohgaki commented Sep 14, 2016

merged

commit 6e53050

Sorry bug # was wrong... Fixed in NEWS later

@yohgaki yohgaki closed this Sep 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants