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

Improve 'To' and 'Reply-To' headers handling #16

Merged
merged 6 commits into from
Nov 18, 2017
Merged

Improve 'To' and 'Reply-To' headers handling #16

merged 6 commits into from
Nov 18, 2017

Conversation

pataquets
Copy link
Contributor

After feedback on #13 and testing 0086ec0:

  • The resulting 'To:' field handling doesn't support multi-recipient field values (getaddresses() vs. parseaddr(). Attempted to fix that.
  • Added a specific, stand-alone 'reply-to' field. This might enable in a future to use the 'to' property to just only the 'To:' header, but not sure, though.
  • Added an examplo 'To:' header to a test email to easily demo a multi-recipient header (a tricky one, containing the field value separator, which is comma).

Needed/broken:

  • Tests. I've seen where and how they're done.
  • The to() property is currently ignoring the 'reply-to' (ouch!). How do I proceed? Merge 'to'+'reply-to' for compatibility or separate fields and break?
  • Add test running to Dockerfile? (I wil grab them from .travis.yml).

I've paused work on #14 until I get more feedback and learn a bit more about the software, since they're related.
Disclaimer: Python is not my main language :)

@fedelemantuano
Copy link
Contributor

Hi @pataquets:

  • The resulting 'To:' field handling doesn't support multi-recipient... Ok I will test your suggestion and I will replace it.
  • Added a specific, stand-alone 'reply-to' field. The fist version of mail-parser was in SpamScope, but now I will split to and reply-to and I will add cc. So for me it's ok.
  • For unittest don't worry I will fix it.
  • For Dockerfile I will do like in SpamScope. Travis will build and push images for me here

@fedelemantuano fedelemantuano merged commit 2ad729b into SpamScope:develop Nov 18, 2017
@pataquets pataquets deleted the parse-to-into-a-list branch November 20, 2017 11:33
fedelemantuano pushed a commit that referenced this pull request Nov 24, 2017
* Add headers parsing into a list in addition to a string. Also, use it for JSON output.

* Remove unneded property resetting.

* Allow 'To' field to have multiple addresses.

* Add 'delivered-to' field as a stand-alone property.

* Add multi-recipient 'To:' header to test email.
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.

None yet

2 participants