[Mailman-Developers] message posting in a loop with mailman 2.1b1

Barry A. Warsaw barry@zope.com
Wed, 17 Apr 2002 00:10:50 -0400


>>>>> "MM" == Marc MERLIN <marc_news@vasoftware.com> writes:
 
    MM> I'm not saying that mailman is incorrect on the interpretation
    MM> of the RFC, I'm saying that if mailman feeds an incorrect
    MM> Email address or something that causes the MTA to reject the
    MM> mail, it will endlessly spam all the subscribers that are
    MM> being delivered to every time mailman tries.

Ah, now I understand.

    MM> This can't be the desired behaviour...

What?  You mean you don't like mailbombing your innocent list members?
That's a marketing gimmick I use on people until they come out to see
the band and buy at least 3 CDs!  You've discovered my backdoor, so
now I guess I have to rip it out.

    MM> What if mailman gets a 5xx? Does it give up on the message and
    MM> drop it on the floor?

When MM gets a 5xx, it records that chunk's recipients as
permfailures.  For 4xx's it records them as tempfailures.  Once
delivery has been attempted to all the chunks, it will then process
the two types of failures.  For permanent fail recipients, it will
periodically lock the affected mailing list and do bounce registration
on them.

For temporary fail recipients, it will attempt redelivery until 1) it
makes no progress (i.e. the number of undelivered recips from the last
attempt is the same as the number from this attempt), and 2) until
mm_cfg.DELIVERY_RETRY_PERIOD is elapsed.  If these conditions are met,
it discards the message (this may not be the right thing to do).

However, in eyeballing the code to write this response, I think I see
the bug!  The list of all recipients for this message is kept in the
metadata dict, under the key `recips'.  When we requeue a message with
tempfailures for retry later, I failed to reset the `recips' value to
just the list of tempfailed recipients.  This would indeed cause the
message to be later redelivered to everybody.

I believe the fix is simple.  Attached below is an untested patch.
I'm way too tired to test this tonight, but I'll try to craft a test
for this tomorrow to make sure it's correct, and if so I'll commit it
for 2.1b2, perhaps also to be releaed tomorrow (I also need to get
2.0.10 out the door).

Thanks, this was a real bug.
-Barry

-------------------- snip snip --------------------
Index: OutgoingRunner.py
===================================================================
RCS file: /cvsroot/mailman/mailman/Mailman/Queue/OutgoingRunner.py,v
retrieving revision 2.14
diff -u -r2.14 OutgoingRunner.py
--- OutgoingRunner.py	10 Apr 2002 04:48:01 -0000	2.14
+++ OutgoingRunner.py	17 Apr 2002 04:09:30 -0000
@@ -101,15 +101,16 @@
             last_recip_count = msgdata.get('last_recip_count', 0)
             deliver_until = msgdata.get('deliver_until', now)
             if len(recips) == last_recip_count:
-                # We didn't make any progress.
+                # We didn't make any progress, so don't attempt delivery any
+                # longer.  BAW: is this the best disposition?
                 if now > deliver_until:
-                    # We won't attempt delivery any longer.
                     return 0
             else:
                 # Keep trying to delivery this for 3 days
                 deliver_until = now + mm_cfg.DELIVERY_RETRY_PERIOD
             msgdata['last_recip_count'] = len(recips)
             msgdata['deliver_until'] = deliver_until
+            msgdata['recips'] = recips
             # Requeue
             return 1
         # We've successfully completed handling of this message