mailjet / mailjet-gem

[API v3] Mailjet official Ruby GEM
https://dev.mailjet.com
Other
130 stars 72 forks source link

Unable to set List-Unsubscribe header #135

Closed ArthurWD closed 3 years ago

ArthurWD commented 6 years ago

I try to set the List-Unsubscribe header, but on mailer.rb line 113, the header is excluded:

if header.name.start_with?('X-') && !header.name.start_with?('X-MJ') && !header.name.start_with?('X-Mailjet')

Why not allow additional headers? Or am I missing something?

Might be better to blacklist the headers listed here, instead of whitelisting, or just let the api handle unpermitted headers.

Lorel commented 6 years ago

hello @ArthurWD

the purpose of this filtering is to allow only custom headers, i.e. those starting by X-, except those specific to Mailjet (!header.name.start_with?('X-MJ') && !header.name.start_with?('X-Mailjet')): https://github.com/mailjet/mailjet-gem/blob/master/lib/mailjet/mailer.rb#L108-L117

what is the purpose of the header List-Unsubscribe? to be honest, I've never seen it is it used by some email clients?

if it is something you use yourself, why don't you instead X-List-Unsubscribe?

ArthurWD commented 6 years ago

Thanks for your rapid reply. The header is defined in a RFC spec, you can read about it here:

We provide an endpoint to unsubscribe from a discussion with one click, which should be rendered by mailclients as a button when using the List-Unsubscribe header.

Lorel commented 6 years ago

Ok, I see I don't really know why only custom headers are allowed Do you try to fix that by removing header.name.start_with?('X-') && on mailer.rb line 113?

ArthurWD commented 6 years ago

Just tried that, it now adds the following headers: Date, From, To, Message-ID, Subject, Mime-Version, Content-Type, Content-Transfer, List-Unsubscribe According to the mailjet docs the first 5 of these are forbidden, so except for my custom List-Unsubscribe header, Rails also adds

    "Mime-Version":"1.0",
    "Content-Type":"text/html",
    "Content-Transfer-Encoding":"quoted-printable"

Guess that's alright?

I did not yet try what happens when including forbidden headers. I hope Mailjet just ignores these, instead of raising an error?

Lorel commented 6 years ago

I don't know how the API works exactly, sorry If the API rejects your request, you can rewrite

if header.name.start_with?('X-') && !header.name.start_with?('X-MJ') && !header.name.start_with?('X-Mailjet')

with

if header.name.start_with?('X-') && !header.name.start_with?('X-MJ') && !header.name.start_with?('X-Mailjet') || (header.name == List-Unsubscribe)

for a quick fix

To do that well, a whitelist of authorized headers to forward to the API should be considered Let me know if the API accepts this header

ArthurWD commented 6 years ago

The api did raise an error on forbidden headers. I forked the project and created a PR to allow setting custom headers that are not blacklisted.

xab3r commented 3 years ago

@ArthurWD Thank you for your contribution. Closing as resolved