From: Philip Hazel Date: Thu, 7 Apr 2005 15:37:13 +0000 (+0000) Subject: Move duplicate checking till after routing. Should fix several subtle X-Git-Tag: exim-4_51~15 X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=d71748467d1443be8c3b84a23839ea975c125b0d;p=exim.git Move duplicate checking till after routing. Should fix several subtle bugs. --- diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index cb661b943..758317801 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -1,4 +1,4 @@ -$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.122 2005/04/07 10:54:54 ph10 Exp $ +$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.123 2005/04/07 15:37:13 ph10 Exp $ Change log file for Exim from version 4.21 ------------------------------------------- @@ -212,6 +212,24 @@ PH/35 ACL_WHERE_MIME is now declared unconditionally, to avoid too much code PH/36 The change PH/12 above was broken. Fixed it. +PH/37 Exim used to check for duplicate addresses in the middle of routing, on + the grounds that routing the same address twice would always produce the + same answer. This might have been true once, but it is certainly no + longer true now. Routing a child address may depend on the previous + routing that produced that child. Some complicated redirection strategies + went wrong when messages had multiple recipients, and made Exim's + behaviour dependent on the order in which the addresses were given. + + I have moved the duplicate checking until after the routing is complete. + Exim scans the addresses that are assigned to local and remote + transports, and removes any duplicates. This means that more work will be + done, as duplicates will always all be routed, but duplicates are + presumably rare, so I don't expect this is of any significance. + + For deliveries to pipes, files, and autoreplies, the duplicate checking + still happens during the routing process, since they are not going to be + routed further. + A note about Exim versions 4.44 and 4.50 ---------------------------------------- diff --git a/src/src/deliver.c b/src/src/deliver.c index ac23a33aa..d610b38ca 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/deliver.c,v 1.11 2005/04/06 14:40:24 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 * @@ -4274,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 * @@ -5290,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) @@ -5305,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 @@ -5617,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. */