Maximal number of errors recorded in Part limited

[pavelbazika]

Hi,

I use enmime to parse emails in my service. It happened that instead of valid mime structure, client sent several MBs of base64 data as input. In such case, enmime consumes a lot of memory (several GBs). It's because readHeader function adds many many warnings in order to "Attempt to detect and repair a non-indented continuation of previous line" as comment says.

In the end, readHeader calls ReadMIMEHeader, which doesn't find valid MIME structure and fails with error and all these warnings are freed. However the memory peak remains and can lead to swapping.

To make it more robust, I'm adding a MaxPartErrors global public variable, which does not allow to record more than limit errors in one part. Default value is 1000 in my PR, but it can be also 0 if you desire, which means no limit and thus there will be no change in current behavior.

Best regards
Pavel

[iredmail]

Maybe default 1000 is too many. :)

[pavelbazika]

Ok, it was meant primarily to avoid extreme memory consumption, what about 100?

[jhillyerd]

I think 100 is probably reasonable, in that it's not likely to provide any useful feedback beyond that. For example, my Inbucket project displays these errors to the user to help them improve the quality of the email they send.

In my mind, this should probably be blocked by #90, so they it can be controlled without a global. Let me think on accepting the PR without that though, since I expect 99% of users will never change it.

[iredmail]

for backward compatible, default value should be unlimited which is same as current release.

[jhillyerd]

That's good point, let's give it a high or infinite default for now. I think we are about due to release 0.9.4. I need to look at #90 more and determine if that should be before or after a 1.0.

[pavelbazika]

I'll give it default value 0, which means infinite. One more question, should I rather hide the global limit variable behind a getter/setter pair? Could be handy in the future, as you plan some options struct.

[jhillyerd]

No need for get/set pair: my plan for options is that they'd would be passed in with each call to decode; they wouldn't be global.

Edit: filed #241 for the broken pull request checks

[jhillyerd]

Please set default to 0, and extend comment to note that errors after the limit are ignored, it does not cause parsing to fail.

[pavelbazika]

Ok, done

[jhillyerd]

Thank you!

[jhillyerd]

Heads up that this max errors setting will be available as a Parser option (see #274) after the next release. I will remove the global variable for the release after that.