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

add new option SetCustomParseMediaType to customise mediatype parsing #308

Merged
merged 4 commits into from
Oct 18, 2023

Conversation

vadzappa
Copy link
Contributor

@vadzappa vadzappa commented Oct 13, 2023

Problem

we've encountered real mail bodies where Content-Type is set to "application/Pamir Viewer; name=\"some-thing.pmrv\"", which is not RFC compliant, but is still valuable for us to process it, so it can't be skipped using SkipMalformedParts

Proposed solution

New option SetCustomParseMediaType which accepts function which has signature exactly the same as ParseMediaType. This option can be utilized in similar scenarios, see provided example: ExampleSetCustomParseMediaType

will allow to fix this #293 indirectly

@coveralls
Copy link

coveralls commented Oct 13, 2023

Coverage Status

coverage: 86.195% (+0.1%) from 86.046% when pulling 2b14009 on vadzappa:support-custom-mediatype-parser into 9d1f8fe on jhillyerd:main.

@vadzappa vadzappa marked this pull request as ready for review October 13, 2023 10:13
options.go Outdated Show resolved Hide resolved
options_test.go Outdated
return ParseMediaType(modifiedStr)
}

invalidMessageContent := `Return-path: <enmime.one@parser.git>
Copy link
Owner

Choose a reason for hiding this comment

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

I like that this is inline content for the example function, but I think we should remove any unnecessary headers from the message to keep it compact and readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since it's a bit unclear from contribution file, would it be feasible to keep commits for fixing proposals as separate commits, or do a rebase after review, so PR will still one commit?

part.go Outdated
return ParseMediaType(ctype)
}

if p.parser.customParseMediaType == nil {
Copy link
Owner

Choose a reason for hiding this comment

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

Could be combined with if statement above.

@jhillyerd
Copy link
Owner

jhillyerd commented Oct 16, 2023

I always squash PR merges via the GitHub button so you don't need to squash/rebase yourself.

@vadzappa vadzappa force-pushed the support-custom-mediatype-parser branch from 7fefdfe to 9ebf13f Compare October 17, 2023 07:48
@jhillyerd
Copy link
Owner

Looks good now, but I think we have some merge conflicts after I merged the previous PR

@vadzappa vadzappa force-pushed the support-custom-mediatype-parser branch from 9ebf13f to 2b14009 Compare October 18, 2023 06:25
@vadzappa
Copy link
Contributor Author

Looks good now, but I think we have some merge conflicts after I merged the previous PR

did rebase, should be no conflicts now

@jhillyerd jhillyerd merged commit b88939a into jhillyerd:main Oct 18, 2023
5 checks passed
@jhillyerd
Copy link
Owner

Thanks!

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

Successfully merging this pull request may close these issues.

3 participants