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

Multiple Received headers #13

Open
janvarljen opened this issue Apr 13, 2016 · 3 comments
Open

Multiple Received headers #13

janvarljen opened this issue Apr 13, 2016 · 3 comments

Comments

@janvarljen
Copy link

I noticed an issue when email has multiple "Received" headers.

The extract_headers method

https://github.com/bradpauly/griddler-mailgun/blob/master/lib%2Fgriddler%2Fmailgun%2Fadapter.rb#L51

i.e. the line

parsed_headers.each{ |h| extracted_headers[h[0]] = h[1] }

overrides the "Received" header x times and return only the last one.

I could submit a PR, just need your opinion on this.

@bradpauly
Copy link
Owner

@janvarljen Thanks for pointing that out. Do you have a solution in mind? I'd have to look more closely at what Griddler is expecting from us to make sure it would work.

@janvarljen
Copy link
Author

I think we have two solutions: concatenate strings or build an array. Both ways have pros and cons.

If we concatenate strings we can always expect the same data type -> String but we lose the ability to differentiate multiple headers (it gets mashed into one String).

If we build arrays we can differentiate multiple headers but the problem is how to decide which header should be an array and which not.

I prefer the array what and probably the best way to do it is:

I was using griddler-mandrill before switching to mailgun and header['Received'] was always an array (even with only one item). This way we can ensure compatibility between adapters (I had to rewrite my email_processor to make it work correctly)

Also I think we need a good readme section describing headers section.

@bradpauly
Copy link
Owner

This sounds like a good approach. I also agree that there should be better documentation about the headers. The way they are handled is a little confusing. Thanks for taking the time to make a clear proposal.

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

No branches or pull requests

2 participants