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 the tolerance of base64Cleaner #47

Merged
merged 3 commits into from
Jan 7, 2018
Merged

Improve the tolerance of base64Cleaner #47

merged 3 commits into from
Jan 7, 2018

Conversation

XiaoMouR
Copy link
Contributor

@XiaoMouR XiaoMouR commented Dec 23, 2017

Inspired by python, for this issure

Inspired by table_a2b_base64 in binascii module of python source code
The PAD will be stripped in cleaning process.
@XiaoMouR XiaoMouR closed this Dec 23, 2017
@coveralls
Copy link

coveralls commented Dec 23, 2017

Coverage Status

Coverage remained the same at 86.315% when pulling 8abec6c on leibiao:master into 0edd8bb on jhillyerd:master.

@XiaoMouR XiaoMouR reopened this Dec 23, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.315% when pulling 8abec6c on leibiao:master into 0edd8bb on jhillyerd:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.315% when pulling 8abec6c on leibiao:master into 0edd8bb on jhillyerd:master.

@jhillyerd
Copy link
Owner

Thanks, I hadn't seen that python table before. I like this idea, but I don't like that we are not capturing any errors to send back to the software using enmime.

From my comment on #46 - It would make sense to add an Errors slice to the base64Cleaner struct, and an errorMalformedBase64 const to errors.go, plus whatever other plumbing is required to surface those errors to the consumer.

Perhaps we can keep the original case ' ', '\t', '\r', '\n': statement, and add '=' to it, allowing all those to be stripped silently. But any other characters returning -1 would trigger an error to be added to the list.

Let me know if you are interested in pursuing this, otherwise I can merge this PR into a feature branch and add the error part myself.

@jhillyerd jhillyerd changed the base branch from master to develop December 23, 2017 18:17
@XiaoMouR
Copy link
Contributor Author

XiaoMouR commented Dec 24, 2017

Maybe we can add a switch that controls the strategy of enmime.For example, if use Strict mode, enmime will generate more errors when meets malformed strings.And use Tolerant mode, enmime will try it best to rebuild the content while only generates some necessary errors.
BTW, it's common to analyze malformed spam in my work.I'd like to see this switch added to enmime. :)

@jhillyerd
Copy link
Owner

Take a closer look at https://github.com/jhillyerd/enmime/blob/master/error.go#L19

We don't need the flag, as the client can filter on Severe or Name to only examine errors it cares about. Or it can completely ignore the Part/Envelope.Errors slice. In which case it only need to pay attention to the hard Go error returned from ReadEnvelope to know that parsing completely failed.

@jhillyerd jhillyerd changed the base branch from develop to feature/base64 January 7, 2018 21:02
@jhillyerd jhillyerd merged commit 124ffd3 into jhillyerd:feature/base64 Jan 7, 2018
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