humanmade / aws-ses-wp-mail

An AWS SES wp_mail() drop-in
184 stars 51 forks source link

Headers should be handled case-insensitively #20

Closed robneu closed 7 years ago

robneu commented 7 years ago

Not sure what the deal is, but GravityForms sends notifications in plain text format when the drop-in is activated. Seems to be related to the content type headers somehow as they come through as text/plain

robneu commented 7 years ago

Definitely seems to be an issue with the headers, setting the $message_args['headers']['Content-Type'] default to apply_filters( 'wp_mail_content_type', 'text/html' ) fixes the issue.

I'm sure it probably breaks other things though. I'm not sure why that filter isn't being set correctly by gforms.

rmccue commented 7 years ago

My guess would be differing defaults. Can you trace down the wp_mail call and find what's passed into it?

robneu commented 7 years ago

Looks like it's a case sensitivity thing. It passes in Content-type rather than Content-Type in the headers.

rmccue commented 7 years ago

That was my second guess :) Headers are case-insensitive, so we should take this into account when we're merging this. Potentially, we can use Requests_Utility_CaseInsensitiveDictionary which is designed for exactly this purpose, but likely overkill here.

robneu commented 7 years ago

After a bit more digging, it looks like the real issue is that in this case the headers are being unset here.

You end up with an empty array before the camel case conversion runs and the defaults are merged.

I'm having a little trouble wrapping my head around exactly why all the values are being unset though...

rmccue commented 7 years ago

Aha, discovered what's happening here.

Gravity Forms sets $headers['Content-type'] = "Content-type: text/html ...", which is redundant. The unset is there so that the old header is deleted when we convert, but that breaks this use case.

Easy enough to fix.