Fix bug 3071#3075
Open
kreshot wants to merge 1 commit into
Open
Conversation
Additionally, if a read error is encountered, skip the message and move to the next one.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Issues Found
Critical (P0/P1)
- [P1] Prevent skipped multipart messages from being deleted unprocessed (
internal/bounce/mailbox/pop.go:137-143)- When
mr.NextPart()returns a non-EOF error, the newcontinue MESSAGES_LOOPskips emitting aBouncefor that message, but the function still runs the unconditional delete loop (Dele(1..count)) afterward. This causes malformed/problematic multipart bounces to be silently dropped (not processed and not left in the mailbox for inspection), which is especially likely with MIME boundary corruption—the exact scenario this change is trying to handle.
- When
Summary
Total issues: 1 critical, 0 important, 0 minor.
Overall Verdict
Status: Patch has blocking issues
Explanation: The new multipart error handling can skip processing a message but the later unconditional POP3 deletion still removes it, leading to lost bounce events and preventing later inspection of problematic messages.
Review generated by Hodor (model: gpt-5.2)
Review Metrics — 10 turns, 9 tool calls, 1m 43s
- Tokens: in
8.4K| cached72.1K| out5.7K(total86.2K) - Cost:
$0.1075
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In addition to the last message part, allow the text/plain part for bounce reason parsing.
Also, if a read error is encountered, skip the message and move to the next one. This will allow the bounce processor to not get stuck when an error happens, and eventually the bounces mailbox will only contain problematic messages. This will allow easier examination of future issues.