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

Fix!: The ability to get Content-Type value from attachments (Inbound Parse) #416

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

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.

helpers/inbound/inbound_test.go Outdated Show resolved Hide resolved
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