Move duplicate checking till after routing. Should fix several subtle
[exim.git] / src / src / deliver.c
index 14d2217b8b2490c56a4285acfb038b1574546973..d610b38ca0adae80dd43f17e70c25935d32605b6 100644 (file)
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/deliver.c,v 1.7 2005/02/17 11:58:25 ph10 Exp $ */
+/* $Cambridge: exim/src/src/deliver.c,v 1.12 2005/04/07 15:37:14 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -729,9 +729,27 @@ else if (driver_type == DTYPE_ROUTER)
 
 /* If there's an error message set, ensure that it contains only printing
 characters - it should, but occasionally things slip in and this at least
-stops the log format from getting wrecked. */
+stops the log format from getting wrecked. We also scan the message for an LDAP
+expansion item that has a password setting, and flatten the password. This is a
+fudge, but I don't know a cleaner way of doing this. (If the item is badly
+malformed, it won't ever have gone near LDAP.) */
 
-if (addr->message != NULL) addr->message = string_printing(addr->message);
+if (addr->message != NULL)
+  {
+  addr->message = string_printing(addr->message);
+  if (Ustrstr(addr->message, "failed to expand") != NULL &&
+      (Ustrstr(addr->message, "ldap:") != NULL ||
+       Ustrstr(addr->message, "ldapdn:") != NULL ||
+       Ustrstr(addr->message, "ldapm:") != NULL))
+    {
+    uschar *p = Ustrstr(addr->message, "pass=");
+    if (p != NULL)
+      {
+      p += 5;
+      while (*p != 0 && !isspace(*p)) *p++ = 'x';
+      }
+    }
+  }
 
 /* If we used a transport that has one of the "return_output" options set, and
 if it did in fact generate some output, then for return_output we treat the
@@ -1431,18 +1449,21 @@ return rc;
 *************************************************/
 
 /* Check that this base address hasn't previously been delivered to its routed
-transport. The check is necessary at delivery time in order to handle homonymic
-addresses correctly in cases where the pattern of redirection changes between
-delivery attempts (so the unique fields change). Non-homonymic previous
-delivery is detected earlier, at routing time (which saves unnecessary
-routing).
+transport. If it has been delivered, mark it done. The check is necessary at
+delivery time in order to handle homonymic addresses correctly in cases where
+the pattern of redirection changes between delivery attempts (so the unique
+fields change). Non-homonymic previous delivery is detected earlier, at routing
+time (which saves unnecessary routing).
+
+Arguments:
+  addr      the address item
+  testing   TRUE if testing wanted only, without side effects
 
-Argument:   the address item
 Returns:    TRUE if previously delivered by the transport
 */
 
 static BOOL
-previously_transported(address_item *addr)
+previously_transported(address_item *addr, BOOL testing)
 {
 (void)string_format(big_buffer, big_buffer_size, "%s/%s",
   addr->unique + (testflag(addr, af_homonym)? 3:0), addr->transport->name);
@@ -1452,7 +1473,7 @@ if (tree_search(tree_nonrecipients, big_buffer) != 0)
   DEBUG(D_deliver|D_route|D_transport)
     debug_printf("%s was previously delivered (%s transport): discarded\n",
     addr->address, addr->transport->name);
-  child_done(addr, tod_stamp(tod_log));
+  if (!testing) child_done(addr, tod_stamp(tod_log));
   return TRUE;
   }
 
@@ -2039,7 +2060,7 @@ while (addr_local != NULL)
   attempts. Non-homonymic previous delivery is detected earlier, at routing
   time. */
 
-  if (previously_transported(addr)) continue;
+  if (previously_transported(addr, FALSE)) continue;
 
   /* There are weird cases where logging is disabled */
 
@@ -2080,6 +2101,7 @@ while (addr_local != NULL)
     same characteristics. These are:
 
       same transport
+      not previously delivered (see comment about 50 lines above)
       same local part if the transport's configuration contains $local_part
       same domain if the transport's configuration contains $domain
       same errors address
@@ -2093,6 +2115,7 @@ while (addr_local != NULL)
       {
       BOOL ok =
         tp == next->transport &&
+        !previously_transported(next, TRUE) &&
         (!uses_lp  || Ustrcmp(next->local_part, addr->local_part) == 0) &&
         (!uses_dom || Ustrcmp(next->domain, addr->domain) == 0) &&
         same_strings(next->p.errors_address, addr->p.errors_address) &&
@@ -3370,7 +3393,7 @@ for (delivery_count = 0; addr_remote != NULL; delivery_count++)
   attempts. Non-homonymic previous delivery is detected earlier, at routing
   time. */
 
-  if (previously_transported(addr)) continue;
+  if (previously_transported(addr, FALSE)) continue;
 
   /* Force failure if the message is too big. */
 
@@ -4251,6 +4274,58 @@ else
 
 
 
+/*************************************************
+*     Check list of addresses for duplication    *
+*************************************************/
+
+/* This function was introduced when the test for duplicate addresses that are
+not pipes, files, or autoreplies was moved from the middle of routing to when
+routing was complete. That was to fix obscure cases when the routing history
+affects the subsequent routing of identical addresses. If that change has to be
+reversed, this function is no longer needed. For a while, the old code that was
+affected by this change is commented with !!!OLD-DE-DUP!!! so it can be found
+easily.
+
+This function is called after routing, to check that the final routed addresses
+are not duplicates. If we detect a duplicate, we remember what it is a
+duplicate of. Note that pipe, file, and autoreply de-duplication is handled
+during routing, so we must leave such "addresses" alone here, as otherwise they
+will incorrectly be discarded.
+
+Argument:     address of list anchor
+Returns:      nothing
+*/
+
+static void
+do_duplicate_check(address_item **anchor)
+{
+address_item *addr;
+while ((addr = *anchor) != NULL)
+  {
+  tree_node *tnode;
+  if (testflag(addr, af_pfr))
+    {
+    anchor = &(addr->next);
+    }
+  else if ((tnode = tree_search(tree_duplicates, addr->unique)) != NULL)
+    {
+    DEBUG(D_deliver|D_route)
+      debug_printf("%s is a duplicate address: discarded\n", addr->unique);
+    *anchor = addr->next;
+    addr->dupof = tnode->data.ptr;
+    addr->next = addr_duplicate;
+    addr_duplicate = addr;
+    }
+  else
+    {
+    tree_add_duplicate(addr->unique, addr);
+    anchor = &(addr->next);
+    }
+  }
+}
+
+
+
 
 /*************************************************
 *              Deliver one message               *
@@ -4639,6 +4714,8 @@ else if (system_filter != NULL && process_recipients != RECIP_FAIL_TIMEOUT)
       RDO_REWRITE,
     NULL,                   /* No :include: restriction (not used in filter) */
     NULL,                   /* No sieve vacation directory (not sieve!) */
+    NULL,                   /* No sieve user address (not sieve!) */
+    NULL,                   /* No sieve subaddress (not sieve!) */
     &ugid,                  /* uid/gid data */
     &addr_new,              /* Where to hang generated addresses */
     &filter_message,        /* Where to put error message */
@@ -5265,6 +5342,18 @@ while (addr_new != NULL)           /* Loop until all addresses dealt with */
       continue;
       }
 
+
+    /* !!!OLD-DE-DUP!!!  We used to test for duplicates at this point, in order
+    to save effort on routing duplicate addresses. However, facilities have
+    been added to Exim so that now two identical addresses that are children of
+    other addresses may be routed differently as a result of their previous
+    routing history. For example, different redirect routers may have given
+    them different redirect_router values, but there are other cases too.
+    Therefore, tests for duplicates now take place when routing is complete.
+    This is the old code, kept for a while for the record, and in case this
+    radical change has to be backed out for some reason.
+
+    #ifdef NEVER
     /* If it's a duplicate, remember what it's a duplicate of */
 
     if ((tnode = tree_search(tree_duplicates, addr->unique)) != NULL)
@@ -5280,6 +5369,9 @@ while (addr_new != NULL)           /* Loop until all addresses dealt with */
     /* Record this address, so subsequent duplicates get picked up. */
 
     tree_add_duplicate(addr->unique, addr);
+    #endif
+
+
 
     /* Get the routing retry status, saving the two retry keys (with and
     without the local part) for subsequent use. Ignore retry records that
@@ -5592,6 +5684,21 @@ Ensure they are not set in transports. */
 local_user_gid = (gid_t)(-1);
 local_user_uid = (uid_t)(-1);
 
+
+/* !!!OLD-DE-DUP!!! The next two statement were introduced when checking for
+duplicates was moved from within routing to afterwards. If that change has to
+be backed out, they should be removed. */
+
+/* Check for any duplicate addresses. This check is delayed until after
+routing, because the flexibility of the routing configuration means that
+identical addresses with different parentage may end up being redirected to
+different addresses. Checking for duplicates too early (as we previously used
+to) makes this kind of thing not work. */
+
+do_duplicate_check(&addr_local);
+do_duplicate_check(&addr_remote);
+
+
 /* When acting as an MUA wrapper, we proceed only if all addresses route to a
 remote transport. The check that they all end up in one transaction happens in
 the do_remote_deliveries() function. */