axllent / imap-scrub

A ultility to trim down your IMAP mailbox by saving & removing attachments or deleting messages based on rules
MIT License
20 stars 5 forks source link

Avoid altering messages that don't have attachments #11

Closed michalfapso closed 5 months ago

michalfapso commented 5 months ago

Probably fixes #7.

Seems that the issue occurred when the email message contained a mail.InlineHeader part with the Content-Type other than image/.*

Using len(deleted) instead of msgParts could fix it.

When len(deleted) == 0 an error is returned from the lib.HandleMessage() and further processing of the message in main() is skipped.

axllent commented 5 months ago

Sorry, I'm definitely not ignoring your PR (which I really appreciate by the way, thank you) - I'm just trying to work out my own logic & handling here. I think your fix would prevent the message modification, however it would also cause imap-scrub to print an error - whereas we'd probably just prefer it to silently skip the message (or potentially print a warning or something).

Either way, you have identified where the source of the problem is (good catch!) - now I just need to spend a bit more time (than I have today) and consider how best to handle that from a user perspective.

michalfapso commented 5 months ago

Sure, @axllent, that makes sense!

First I thought that lib.HandleMessage() could also return the deleted list and then handle the "no deleted attachment" case on the calling site, but returning the error seemed just simpler.

Thanks for looking deeper at it.

axllent commented 5 months ago

I ended up rewriting the fix as it was a bit more complicated than what I had thought, plus I discovered a "bug" detecting filenames for some inline images (that weren't quoted).

As I said before, it was your PR that identified where the issue was happening, so thank you again!

michalfapso commented 5 months ago

Perfect! Thanks @axllent, your approach looks nicer indeed.