[Mailman-Developers] Preventing admin-notice bounce loops!

Ken Manheimer klm@python.org
Wed, 20 May 1998 20:36:27 -0400 (EDT)


I think i've got the right solution to the nasty bounce loop by which
janne sinkonnen got around 22,000 messages last weekend.  The approach
is what i suggested earlier today - check the bouncing address to see
if it's among the list administrators, and if so log an error and
*don't* send out another notice.  I have tested it as far as i was
able, and it seems to behave just the way we'd want.

This patch also has a few other bounce improvements, one being
important, in a slightly different manner: a copy of the offending
bounce notification is included with the admin-notice when a user is
being disabled or unsubscribed.  This enables the admin to assess the
correctness of the action - something otherwise impossible, since no
other record of the bounce notice is retained.

(Ultimately it would be nice to have some kind of cataloguing system
for things like subscribe and admin-approval requests, since the
entries can be discarded once the administrator has reviewed and
decided their fate.  For bounce notices, just sending them with the
automatic action seems to make the most sense to me...)

If you give this a try, please let me know how it goes.  I don't think 
it depends on any other local changes, but would like to have that
checked...

See the checkin log entries, below, for more details.

ken manheimer
klm@python.org

----------------------------
revision 1.16
date: 1998/05/21 00:20:00;  author: klm;  state: Exp;  lines: +65 -30
Prevent bounce loops when admin-action recipient is itself the
bouncing address - before sending the notice, check whether the
bouncing address is among the list owner addresses.

And otherwise, include (attach) a copy of the actual bounce notice in
the admin-action message.  Otherwise there is no record the
administrator can examine.

Finally, send both failed and successful admin-action notifications to
the list administrator.  Before, failed action (e.g. inability to
locate the specific user) were sent to the mailman site admin - this
was while i was debugging the bounce handling, but it looks a lot
better now.
----------------------------
revision 1.15
date: 1998/05/20 17:18:19;  author: mailman;  state: Exp;  lines: +10 -10
Neglected to mention on last checkin:

ScanMessage(): added final-step massage of all candidates to remove
encompassing <, > angle brackets, if any, and check against already
processed candidates.
----------------------------
revision 1.14
date: 1998/05/20 17:13:40;  author: mailman;  state: Exp;  lines: +35 -17
Do not send "user disabled" notices to admin when user is already
disabled.  The idea is that numerous outstanding bounces do not each
require renotification of the admin.

DisableBouncingAddress(), RemoveBouncingAddress(): changed return
signatures to indicate whether email notification is necessary for
this action, and in Disable version, indicate not necessary when user
is already disabled.  In all other cases, notification is indicated,
whatever the status.


Index: mm_bouncer.py
===================================================================
RCS file: /projects/cvsroot/mailman/modules/mm_bouncer.py,v
retrieving revision 1.13
retrieving revision 1.16
diff -c -r1.13 -r1.16
*** mm_bouncer.py	1998/04/23 16:26:33	1.13
--- mm_bouncer.py	1998/05/21 00:20:00	1.16
***************
*** 1,6 ****
  "Handle delivery bounce messages, doing filtering when list is set for it."
  
! __version__ = "$Revision: 1.13 $"
  
  # It's possible to get the mail-list senders address (list-admin) in the
  # bounce list.   You probably don't want to have list mail sent to that
--- 1,6 ----
  "Handle delivery bounce messages, doing filtering when list is set for it."
  
! __version__ = "$Revision: 1.16 $"
  
  # It's possible to get the mail-list senders address (list-admin) in the
  # bounce list.   You probably don't want to have list mail sent to that
***************
*** 53,59 ****
  	if self.bounce_info.has_key(email):
  	    del self.bounce_info[email]
  
!     def RegisterBounce(self, email):
  	report = "%s: %s - " % (self.real_name, email)
  	bouncees = self.bounce_info.keys()
  	this_dude = mm_utils.FindMatchingAddresses(email, bouncees)
--- 53,59 ----
  	if self.bounce_info.has_key(email):
  	    del self.bounce_info[email]
  
!     def RegisterBounce(self, email, msg):
  	report = "%s: %s - " % (self.real_name, email)
  	bouncees = self.bounce_info.keys()
  	this_dude = mm_utils.FindMatchingAddresses(email, bouncees)
***************
*** 84,90 ****
                   self.minimum_post_count_before_bounce_action)
  		and difference > self.minimum_removal_date * 24 * 60 * 60):
  		self.LogMsg("bounce", report + "exceeded limits")
! 		self.HandleBouncingAddress(addr)
  		return
  	    else:
  		post_count = (self.minimum_post_count_before_bounce_action - 
--- 84,90 ----
                   self.minimum_post_count_before_bounce_action)
  		and difference > self.minimum_removal_date * 24 * 60 * 60):
  		self.LogMsg("bounce", report + "exceeded limits")
! 		self.HandleBouncingAddress(addr, msg)
  		return
  	    else:
  		post_count = (self.minimum_post_count_before_bounce_action - 
***************
*** 107,113 ****
  		return
  	    if difference > self.minimum_removal_date * 24 * 60 * 60:
  		self.LogMsg("bounce", "exceeded limits (D)")
! 		self.HandleBouncingAddress(addr)
  		return 
  	    self.LogMsg("bounce",
  			"digester lucked out")
--- 107,113 ----
  		return
  	    if difference > self.minimum_removal_date * 24 * 60 * 60:
  		self.LogMsg("bounce", "exceeded limits (D)")
! 		self.HandleBouncingAddress(addr, msg)
  		return 
  	    self.LogMsg("bounce",
  			"digester lucked out")
***************
*** 117,164 ****
  			self._internal_name,
  			addr)
  
!     def HandleBouncingAddress(self, addr):
          """Disable or remove addr according to bounce_action setting."""
          if self.automatic_bounce_action == 0:
              return
          elif self.automatic_bounce_action == 1:
!             succeeded = self.DisableBouncingAddress(addr)
              did = "disabled"
-             send = 1
          elif self.automatic_bounce_action == 2:
!             succeeded = self.DisableBouncingAddress(addr)
              did = "disabled"
              send = 0
          elif self.automatic_bounce_action == 3:
!             succeeded = self.RemoveBouncingAddress(addr)
!             did = "removed"
              send = 1
          if send:
              if succeeded != 1:
                  negative="not "
-                 recipient = mm_cfg.MAILMAN_OWNER
              else:
                  negative=""
!                 recipient = self.GetAdminEmail()
!             text = ("This is a mailman maillist administrator notice.\n"
!                     "\n\tMaillist:\t%s\n"
!                     "\tMember:\t\t%s\n"
!                     "\tAction:\t\tSubscription %s%s.\n"
!                     "\tReason:\t\tExcessive or fatal bounces.\n"
!                     % (self.real_name, addr, negative, did))
              if succeeded != 1:
!                 text = text + "\tBUT:\t\t%s\n\n" % succeeded
!             else:
!                 text = text + "\n"
              if did == "disabled" and succeeded == 1:
!                 text = text + (
!                     "You can reenable their subscription by visiting "
!                     "their options page\n"
!                     "(via %s) and using your\n"
!                     "list admin password to authorize the option change.\n\n"
!                     % self.GetScriptURL('listinfo'))
!             text = text + ("Questions?  Contact the mailman site admin,\n%s"
!                            % mm_cfg.MAILMAN_OWNER)
              if negative:
                  negative = string.upper(negative)
              self.SendTextToUser(subject = ("%s member %s %s%s due to bounces"
--- 117,198 ----
  			self._internal_name,
  			addr)
  
!     def HandleBouncingAddress(self, addr, msg):
          """Disable or remove addr according to bounce_action setting."""
          if self.automatic_bounce_action == 0:
              return
          elif self.automatic_bounce_action == 1:
! 	    # Only send if call works ok.
!             (succeeded, send) = self.DisableBouncingAddress(addr)
              did = "disabled"
          elif self.automatic_bounce_action == 2:
!             (succeeded, send) = self.DisableBouncingAddress(addr)
              did = "disabled"
+ 	    # Never send.
              send = 0
          elif self.automatic_bounce_action == 3:
!             (succeeded, send) = self.RemoveBouncingAddress(addr)
! 	    # Always send.
              send = 1
+             did = "removed"
          if send:
              if succeeded != 1:
                  negative="not "
              else:
                  negative=""
!             recipient = self.GetAdminEmail()
!             if addr in self.owner + [recipient]:
!                 # Whoops!  This is a bounce of a bounce notice - do not
!                 # perpetuate the bounce loop!  Log it prominently and be
!                 # satisfied with that.
!                 self.LogMsg("error",
!                             "%s: Bounce recipient loop"
!                             " encountered!\n\t%s\n\tBad admin recipient: %s",
!                             self._internal_name,
!                             "(Ie, bounce notification addr, itself, bounces.)",
!                             addr)
!                 return
!             import mimetools
!             boundary = mimetools.choose_boundary()
!             text = [""]
!             text.append("(This MIME message should be"
!                         " readable as plain text.)")
!             text.append("")
!             text.append("--" + boundary)
!             text.append("Content-type: text/plain; charset=us-ascii")
!             text.append("")
!             text.append("This is a mailman mailing list bounce action notice:")
!             text.append("")
!             text.append("\tMaillist:\t%s" % self.real_name)
!             text.append("\tMember:\t\t%s" % addr)
!             text.append("\tAction:\t\tSubscription %s%s." % (negative, did))
!             text.append("\tReason:\t\tExcessive or fatal bounces.")
              if succeeded != 1:
!                 text.append("\tBUT:\t\t%s\n" % succeeded)
!             text.append("")
              if did == "disabled" and succeeded == 1:
!                 text.append("You can reenable their subscription by visiting "
!                             "their options page")
!                 text.append("(via %s) and using your"
!                             % self.GetScriptURL('listinfo'))
!                 text.append(
!                     "list admin password to authorize the option change.")
!             text.append("")
!             text.append("The triggering bounce notice is attached below.")
!             text.append("")
!             text.append("Questions?  Contact the mailman site admin,")
!             text.append("\t" + mm_cfg.MAILMAN_OWNER)
! 
!             text.append("")
!             text.append("--" + boundary)
!             text.append("Content-type: text/plain; charset=us-ascii")
!             text.append("")
!             text.append(string.join(msg.headers, ''))
!             text.append("")
!             text.append(mm_utils.QuotePeriods(msg.body))
!             text.append("")
!             text.append("--" + boundary + "--")
! 
              if negative:
                  negative = string.upper(negative)
              self.SendTextToUser(subject = ("%s member %s %s%s due to bounces"
***************
*** 166,209 ****
                                                negative, did)),
                                  recipient = recipient,
                                  sender = mm_cfg.MAILMAN_OWNER,
!                                 add_headers = ["Errors-To: %s"
! 					       % mm_cfg.MAILMAN_OWNER],
!                                 text = text)
      def DisableBouncingAddress(self, addr):
          if not self.IsMember(addr):
              reason = "User not found."
  	    self.LogMsg("bounce", "%s: NOT disabled %s: %s",
                          self.real_name, addr, reason)
!             return reason
  	try:
! 	    self.SetUserOption(addr, mm_cfg.DisableDelivery, 1)
! 	    self.LogMsg("bounce", "%s: disabled %s", self.real_name, addr)
!             self.Save()
!             return 1
  	except mm_err.MMNoSuchUserError:
  	    self.LogMsg("bounce", "%s: NOT disabled %s: %s",
                          self.real_name, addr, mm_err.MMNoSuchUserError)
  	    self.ClearBounceInfo(addr)
              self.Save()
!             return mm_err.MMNoSuchUserError
  	    
      def RemoveBouncingAddress(self, addr):
          if not self.IsMember(addr):
              reason = "User not found."
  	    self.LogMsg("bounce", "%s: NOT removed %s: %s",
                          self.real_name, addr, reason)
!             return reason
  	try:
  	    self.DeleteMember(addr, "bouncing addr")
  	    self.LogMsg("bounce", "%s: removed %s", self.real_name, addr) 
              self.Save()
!             return 1
  	except mm_err.MMNoSuchUserError:
  	    self.LogMsg("bounce", "%s: NOT removed %s: %s",
                          self.real_name, addr, mm_err.MMNoSuchUserError)
  	    self.ClearBounceInfo(addr)
              self.Save()
!             return mm_err.MMNoSuchUserError
  
      # Return 0 if we couldn't make any sense of it, 1 if we handled it.
      def ScanMessage(self, msg):
--- 200,259 ----
                                                negative, did)),
                                  recipient = recipient,
                                  sender = mm_cfg.MAILMAN_OWNER,
!                                 add_headers = [
!                                     "Errors-To: %s" % mm_cfg.MAILMAN_OWNER,
!                                     "MIME-version: 1.0",
!                                     "Content-type: multipart/mixed;"
!                                     ' boundary="%s"' % boundary],
!                                 text = string.join(text, '\n'))
      def DisableBouncingAddress(self, addr):
+ 	"""Disable delivery for bouncing user address.
+ 
+ 	Returning success and notification status."""
          if not self.IsMember(addr):
              reason = "User not found."
  	    self.LogMsg("bounce", "%s: NOT disabled %s: %s",
                          self.real_name, addr, reason)
!             return reason, 1
  	try:
! 	    if self.GetUserOption(addr, mm_cfg.DisableDelivery):
! 		# No need to send out notification if they're already disabled.
! 		self.LogMsg("bounce",
! 			    "%s: already disabled %s", self.real_name, addr)
! 		return 1, 0
! 	    else:
! 		self.SetUserOption(addr, mm_cfg.DisableDelivery, 1)
! 		self.LogMsg("bounce",
! 			    "%s: disabled %s", self.real_name, addr)
! 		self.Save()
! 		return 1, 1
  	except mm_err.MMNoSuchUserError:
  	    self.LogMsg("bounce", "%s: NOT disabled %s: %s",
                          self.real_name, addr, mm_err.MMNoSuchUserError)
  	    self.ClearBounceInfo(addr)
              self.Save()
!             return mm_err.MMNoSuchUserError, 1
  	    
      def RemoveBouncingAddress(self, addr):
+ 	"""Unsubscribe user with bouncing address.
+ 
+ 	Returning success and notification status."""
          if not self.IsMember(addr):
              reason = "User not found."
  	    self.LogMsg("bounce", "%s: NOT removed %s: %s",
                          self.real_name, addr, reason)
!             return reason, 1
  	try:
  	    self.DeleteMember(addr, "bouncing addr")
  	    self.LogMsg("bounce", "%s: removed %s", self.real_name, addr) 
              self.Save()
!             return 1, 1
  	except mm_err.MMNoSuchUserError:
  	    self.LogMsg("bounce", "%s: NOT removed %s: %s",
                          self.real_name, addr, mm_err.MMNoSuchUserError)
  	    self.ClearBounceInfo(addr)
              self.Save()
!             return mm_err.MMNoSuchUserError, 1
  
      # Return 0 if we couldn't make any sense of it, 1 if we handled it.
      def ScanMessage(self, msg):
***************
*** 268,274 ****
  	messy_pattern_6 = regex.compile('^[ \t]*[^ ]+: User unknown.*$')
  	messy_pattern_7 = regex.compile('^[^ ]+ - User currently disabled.*$')
  
! 	message_groked = 0
  
  	for line in string.split(relevant_text, '\n'):
  	    for pattern, action in simple_bounce_pats:
--- 318,324 ----
  	messy_pattern_6 = regex.compile('^[ \t]*[^ ]+: User unknown.*$')
  	messy_pattern_7 = regex.compile('^[^ ]+ - User currently disabled.*$')
  
! 	message_grokked = 0
  
  	for line in string.split(relevant_text, '\n'):
  	    for pattern, action in simple_bounce_pats:
***************
*** 276,331 ****
  		    email = self.ExtractBouncingAddr(line)
  		    if action == REMOVE:
  			candidates = candidates + string.split(email,',')
! 			message_groked = 1
  			continue
  		    elif action == BOUNCE:
  			emails = string.split(email,',')
  			for email_addr in emails:
! 			    self.RegisterBounce(email_addr)
! 			message_groked = 1
  			continue
  		    else:
! 			message_groked = 1
  			continue
  
  	    # Now for the special case messages that are harder to parse...
  	    if (messy_pattern_1.match(line) <> -1
                  or messy_pattern_2.match(line) <> -1):
  		username = string.split(line)[1]
! 		self.RegisterBounce('%s@%s' % (username, remote_host))
! 		message_groked = 1
  		continue
  	    if (messy_pattern_3.match(line) <> -1
                  or messy_pattern_4.match(line) <> -1
                  or messy_pattern_5.match(line) <> -1):
  		username = string.split(line)[1]
                  candidates.append('%s@%s' % (username, remote_host))
! 		message_groked = 1
  		continue
  	    if messy_pattern_6.match(line) <> -1:
  		username = string.split(string.strip(line))[0][:-1]
                  candidates.append('%s@%s' % (username, remote_host))
! 		message_groked = 1
  		continue
  	    if messy_pattern_7.match(line) <> -1:
  		username = string.split(string.strip(line))[0]
                  candidates.append('%s@%s' % (username, remote_host))
! 		message_groked = 1
  		continue
  
          did = []
          for i in candidates:
  	    el = string.find(i, "...")
! 	    if el != -1: i = i[:el]
              if i not in did:
!                 self.HandleBouncingAddress(i)
                  did.append(i)
! 	return message_groked
  
      def ExtractBouncingAddr(self, line):
  	email = regsub.splitx(line, '[^ \t@<>]+@[^ \t@<>]+\.[^ \t<>.]+')[1]
  	if email[0] == '<':
- 	    # Remove what's within the angles.
  	    return regsub.splitx(email[1:], ">")[0]
  	else:
  	    return email
--- 326,384 ----
  		    email = self.ExtractBouncingAddr(line)
  		    if action == REMOVE:
  			candidates = candidates + string.split(email,',')
! 			message_grokked = 1
  			continue
  		    elif action == BOUNCE:
  			emails = string.split(email,',')
  			for email_addr in emails:
! 			    self.RegisterBounce(email_addr, msg)
! 			message_grokked = 1
  			continue
  		    else:
! 			message_grokked = 1
  			continue
  
  	    # Now for the special case messages that are harder to parse...
  	    if (messy_pattern_1.match(line) <> -1
                  or messy_pattern_2.match(line) <> -1):
  		username = string.split(line)[1]
! 		self.RegisterBounce('%s@%s' % (username, remote_host), msg)
! 		message_grokked = 1
  		continue
  	    if (messy_pattern_3.match(line) <> -1
                  or messy_pattern_4.match(line) <> -1
                  or messy_pattern_5.match(line) <> -1):
  		username = string.split(line)[1]
                  candidates.append('%s@%s' % (username, remote_host))
! 		message_grokked = 1
  		continue
  	    if messy_pattern_6.match(line) <> -1:
  		username = string.split(string.strip(line))[0][:-1]
                  candidates.append('%s@%s' % (username, remote_host))
! 		message_grokked = 1
  		continue
  	    if messy_pattern_7.match(line) <> -1:
  		username = string.split(string.strip(line))[0]
                  candidates.append('%s@%s' % (username, remote_host))
! 		message_grokked = 1
  		continue
  
          did = []
          for i in candidates:
  	    el = string.find(i, "...")
! 	    if el != -1:
! 		i = i[:el]
! 	    if len(i) > 1 and i[0] == '<':
! 		# Use stuff after open angle and before (optional) close:
! 		i = regsub.splitx(i[1:], ">")[0]
              if i not in did:
!                 self.HandleBouncingAddress(i, msg)
                  did.append(i)
! 	return message_grokked
  
      def ExtractBouncingAddr(self, line):
  	email = regsub.splitx(line, '[^ \t@<>]+@[^ \t@<>]+\.[^ \t<>.]+')[1]
  	if email[0] == '<':
  	    return regsub.splitx(email[1:], ">")[0]
  	else:
  	    return email