Skip to content

Conversation

garydevenay
Copy link

@garydevenay garydevenay commented Oct 27, 2020

Fixes #412

Previously we could not infer the content type of an attachment from it's contents (see #412) . I've updated the ParsedEmail Attachments property to be of (new) type ParsedAttachment. This new struct has it's "Content-Type" header populated on email parse and included tests.

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 27, 2020
@garydevenay garydevenay changed the title Fix!: The ability to get Content-Type value from attachments Fix!: The ability to get Content-Type value from attachments (Inbound Parse) Oct 27, 2020
@thinkingserious
Copy link
Contributor

Thank you for the PR!

Could you please update your branch with the latest updates from main? It looks like I don't have permission to do so.

@thinkingserious thinkingserious added status: waiting for feedback waiting for feedback from the submitter type: community enhancement feature request not on Twilio's roadmap and removed status: code review request requesting a community code review or review from Twilio labels Oct 28, 2020
headers := make(map[string]string)
content := readBody(emailPart)

headers["Content-Type"] = emailPart.Header.Get("Content-Type")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just headers := emailPart.Header (would need to change type). Alternatively, instead of storing headers, make ContentType one of the properties of ParsedAttachment, and do some simple string parsing to extract the MIME type from the string returned by emailPart.Header.Get("Content-Type")

Copy link
Author

Choose a reason for hiding this comment

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

I've changed the ParsedAttachment.Headers to type MIMEHeader and stored the full attachment headers in there. Also added a string ContentType to take the content type (Without the name="***" part). Updated the tests for this too.

Copy link
Contributor

@eshanholtz eshanholtz left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting for feedback waiting for feedback from the submitter type: community enhancement feature request not on Twilio's roadmap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inbound Parse: All attachments have Content-Type: text/plain

3 participants