From dbbf21a75d225871cb7a44878ece42c5d79a1a2c Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 1 Aug 2019 19:31:36 +0100 Subject: [PATCH] Routers: make retry_use_local_part default true when any non-domain condition is present. Bug 2408 --- doc/doc-docbook/spec.xfpt | 15 ++++- doc/doc-txt/ChangeLog | 6 ++ src/src/deliver.c | 3 +- src/src/route.c | 3 +- test/confs/0264 | 6 ++ test/log/0264 | 16 +++++- test/scripts/0000-Basic/0264 | 15 +++++ test/stderr/0264 | 105 +++++++++++++++++++++++++++++++++++ test/stderr/0375 | 2 - test/stdout/0264 | 9 +++ 10 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 test/stderr/0264 diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index 736ac0fe4..aa3996505 100644 --- a/doc/doc-docbook/spec.xfpt +++ b/doc/doc-docbook/spec.xfpt @@ -18884,11 +18884,24 @@ latter kind. This option controls whether the local part is used to form the key for retry hints for addresses that suffer temporary errors while being handled by this -router. The default value is true for any router that has &%check_local_user%& +.new +router. The default value is true for any router that has any of +&%check_local_user%&, +&%local_parts%&, +&%condition%&, +&%local_part_prefix%&, +&%local_part_suffix%&, +&%senders%& or +&%require_files%& +.wen set, and false otherwise. Note that this option does not apply to hints keys for transport delays; they are controlled by a generic transport option of the same name. +Failing to set this option when it is needed +(because a remote router handles only some of the local-parts for a domain) +can result in incorrect error messages being generated. + The setting of &%retry_use_local_part%& applies only to the router on which it appears. If the router generates child addresses, they are routed independently; this setting does not become attached to them. diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 7fca99b62..6d1b2631b 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -164,6 +164,12 @@ JH/34 Fix crash after TLS shutdown. When the TCP/SMTP channel was left open, JH/35 Bug 2409: filter out-of-spec chars from callout response before using them in our smtp response. +JH/35 Have the general router option retry_use_local_part default to true when + any of the restrictive preconditions are set (to anything). Previously it + was only for check_local user. The change removes one item of manual + configuration which is required for proper retries when a remote router + handles a subset of addresses for a domain. + Exim version 4.92 ----------------- diff --git a/src/src/deliver.c b/src/src/deliver.c index 66e49d371..a82a04f42 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -4794,7 +4794,6 @@ all pipes, so I do not see a reason to use non-blocking IO here for(; addr; addr = addr->next) { uschar *ptr; - retry_item *r; /* The certificate verification status goes into the flags */ if (tls_out.certificate_verified) setflag(addr, af_cert_verified); @@ -4894,7 +4893,7 @@ all pipes, so I do not see a reason to use non-blocking IO here /* Retry information: for most success cases this will be null. */ - for (r = addr->retries; r; r = r->next) + for (retry_item * r = addr->retries; r; r = r->next) { sprintf(CS big_buffer, "%c%.500s", r->flags, r->key); ptr = big_buffer + Ustrlen(big_buffer+2) + 3; diff --git a/src/src/route.c b/src/src/route.c index 41716bc0e..c6119eed0 100644 --- a/src/src/route.c +++ b/src/src/route.c @@ -281,7 +281,8 @@ for (router_instance * r = routers; r; r = r->next) TRUE; otherwise its default is FALSE. */ if (r->retry_use_local_part == TRUE_UNSET) - r->retry_use_local_part = r->check_local_user; + r->retry_use_local_part = + r->check_local_user || r->local_parts || r->condition || r->prefix || r->suffix || r->senders || r->require_files; /* Build a host list if fallback hosts is set. */ diff --git a/test/confs/0264 b/test/confs/0264 index 792b304dd..7c0a066e9 100644 --- a/test/confs/0264 +++ b/test/confs/0264 @@ -63,6 +63,12 @@ r5: allow_defer data = :defer: not just now +r_remain: + driver = redirect + allow_defer + data = :defer: not just now + + # ----- Retry ----- begin retry diff --git a/test/log/0264 b/test/log/0264 index d26865be8..599ea4716 100644 --- a/test/log/0264 +++ b/test/log/0264 @@ -44,7 +44,21 @@ 1999-03-02 09:44:33 10HmbG-0005vi-00 <= CALLER@test.ex U=CALLER P=local S=sss 1999-03-02 09:44:33 Start queue run: pid=pppp 1999-03-02 09:44:33 10HmbF-0005vi-00 == r4.a@outside routing defer (-51): retry time not reached -1999-03-02 09:44:33 10HmbG-0005vi-00 == r4.b@outside routing defer (-51): retry time not reached +1999-03-02 09:44:33 10HmbG-0005vi-00 == r4.b@outside R=r4 defer (-1): not just now 1999-03-02 09:44:33 End queue run: pid=pppp 1999-03-02 09:44:33 10HmbH-0005vi-00 <= CALLER@test.ex U=CALLER P=local S=sss 1999-03-02 09:44:33 10HmbH-0005vi-00 == r5.a@r5domain.ex R=r5 defer (-1): not just now +1999-03-02 09:44:33 10HmbF-0005vi-00 removed by CALLER +1999-03-02 09:44:33 10HmbF-0005vi-00 Completed +1999-03-02 09:44:33 10HmbG-0005vi-00 removed by CALLER +1999-03-02 09:44:33 10HmbG-0005vi-00 Completed +1999-03-02 09:44:33 10HmbH-0005vi-00 removed by CALLER +1999-03-02 09:44:33 10HmbH-0005vi-00 Completed +1999-03-02 09:44:33 10HmbI-0005vi-00 <= CALLER@test.ex U=CALLER P=local S=sss +1999-03-02 09:44:33 10HmbI-0005vi-00 == rz.a@outside R=r_remain defer (-1): not just now +1999-03-02 09:44:33 10HmbJ-0005vi-00 <= CALLER@test.ex U=CALLER P=local S=sss +1999-03-02 09:44:33 10HmbJ-0005vi-00 == rz.b@outside R=r_remain defer (-1): not just now +1999-03-02 09:44:33 Start queue run: pid=pppp +1999-03-02 09:44:33 10HmbI-0005vi-00 == rz.a@outside routing defer (-51): retry time not reached +1999-03-02 09:44:33 10HmbJ-0005vi-00 == rz.b@outside routing defer (-51): retry time not reached +1999-03-02 09:44:33 End queue run: pid=pppp diff --git a/test/scripts/0000-Basic/0264 b/test/scripts/0000-Basic/0264 index 5d6bcfb29..2b97ad4f6 100644 --- a/test/scripts/0000-Basic/0264 +++ b/test/scripts/0000-Basic/0264 @@ -27,13 +27,28 @@ exim -q **** exim -Mrm $msg1 $msg2 **** +# Using a router with preconditions (local_parts, here) should get an address-retry record +sudo rm DIR/spool/db/retry exim -odi r4.a@outside **** +dump retry exim -odq r4.b@outside **** exim -q **** exim -odi r5.a@r5domain.ex **** +exim -Mrm $msg1 $msg2 $msg3 +**** +# Using a router with no non-domain preconditions, first should write a domain-retry record. +sudo rm DIR/spool/db/retry +exim -odi rz.a@outside +**** +dump retry +# Second will find it - this is visible in debug output +exim -d-all+route+deliver+retry -odi rz.b@outside +**** +exim -q +**** no_msglog_check no_message_check diff --git a/test/stderr/0264 b/test/stderr/0264 new file mode 100644 index 000000000..7e86817b4 --- /dev/null +++ b/test/stderr/0264 @@ -0,0 +1,105 @@ +Exim version x.yz .... +configuration file is TESTSUITE/test-config +admin user +Writing spool header file: TESTSUITE/spool//input//hdr.10HmbJ-0005vi-00 +DSN: Write SPOOL: -dsn_envid NULL +DSN: Write SPOOL :-dsn_ret 0 +DSN: Flags: 0x0 +DSN: **** SPOOL_OUT - address: errorsto: orcpt: dsn_flags: 0x0 +Renaming spool header file: TESTSUITE/spool//input//10HmbJ-0005vi-00-H +LOG: MAIN + <= CALLER@test.ex U=CALLER P=local S=sss +Exim version x.yz .... +configuration file is TESTSUITE/test-config +trusted user +admin user +dropping to exim gid; retaining priv uid +delivering 10HmbJ-0005vi-00 +Trying spool file TESTSUITE/spool//input//10HmbJ-0005vi-00-D +reading spool file 10HmbJ-0005vi-00-H +user=CALLER uid=CALLER_UID gid=CALLER_GID sender=CALLER@test.ex +sender_local=1 ident=CALLER +Non-recipients: +Empty Tree +---- End of tree ---- +recipients_count=1 +**** SPOOL_IN - No additional fields +body_linecount=0 message_linecount=7 +DSN: set orcpt: flags: 0x0 +Delivery address list: + rz.b@outside +locking TESTSUITE/spool/db/retry.lockfile +>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> +Considering: rz.b@outside +unique = rz.b@outside +have domain retry record; next_try = now+0 +no address retry record +rz.b@outside: queued for routing +>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> +routing rz.b@outside +--------> r1 router <-------- +local_part=rz.b domain=outside +checking domains +r1 router skipped: domains mismatch +--------> r2 router <-------- +local_part=rz.b domain=outside +checking domains +r2 router skipped: domains mismatch +--------> r3 router <-------- +local_part=rz.b domain=outside +checking local_parts +r3 router skipped: local_parts mismatch +--------> r4 router <-------- +local_part=rz.b domain=outside +checking local_parts +r4 router skipped: local_parts mismatch +--------> r5 router <-------- +local_part=rz.b domain=outside +checking local_parts +r5 router skipped: local_parts mismatch +--------> r_remain router <-------- +local_part=rz.b domain=outside +calling r_remain router +rda_interpret (string): ':defer: not just now' +expanded: ':defer: not just now' +file is not a filter file +parse_forward_list: :defer: not just now +extract item: :defer: not just now +r_remain router: defer for rz.b@outside + message: not just now +added retry item for R:outside: errno=-1 more_errno=dd flags=0 +post-process rz.b@outside (1) +LOG: MAIN + == rz.b@outside R=r_remain defer (-1): not just now +>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> +After routing: + Local deliveries: + Remote deliveries: + Failed addresses: + Deferred addresses: + rz.b@outside +>>>>>>>>>>>>>>>> deliveries are done >>>>>>>>>>>>>>>> +Processing retry items +Succeeded addresses: +Failed addresses: +Deferred addresses: + rz.b@outside +locking TESTSUITE/spool/db/retry.lockfile +retry for R:outside = * 0 0 +failing_interval=ttt message_age=ttt +Writing retry data for R:outside + first failed=dddd last try=dddd next try=+300 expired=0 + errno=-1 more_errno=dd not just now +end of retry processing +time on queue = 0s id 10HmbJ-0005vi-00 addr rz.b@outside +warning counts: required 0 done 0 +delivery deferred: update_spool=1 header_rewritten=0 +Writing spool header file: TESTSUITE/spool//input//hdr.10HmbJ-0005vi-00 +DSN: Write SPOOL: -dsn_envid NULL +DSN: Write SPOOL :-dsn_ret 0 +DSN: Flags: 0x0 +DSN: **** SPOOL_OUT - address: errorsto: orcpt: dsn_flags: 0x0 +Renaming spool header file: TESTSUITE/spool//input//10HmbJ-0005vi-00-H +end delivery of 10HmbJ-0005vi-00 +>>>>>>>>>>>>>>>> Exim pid=pppp (main) terminating with rc=0 >>>>>>>>>>>>>>>> +>>>>>>>>>>>>>>>> Exim pid=pppp (main) terminating with rc=0 >>>>>>>>>>>>>>>> diff --git a/test/stderr/0375 b/test/stderr/0375 index 2c8bffcc7..cc30e5668 100644 --- a/test/stderr/0375 +++ b/test/stderr/0375 @@ -910,7 +910,6 @@ After routing: locking TESTSUITE/spool/db/retry.lockfile LOG: MAIN => CALLER P=<> R=real T=real -locking TESTSUITE/spool/db/retry.lockfile LOG: MAIN Completed >>>>>>>>>>>>>>>> Exim pid=pppp (main) terminating with rc=0 >>>>>>>>>>>>>>>> @@ -1019,7 +1018,6 @@ locking TESTSUITE/spool/db/retry.lockfile LOG: MAIN => h1 P= R=ut8 T=ut1 log writing disabled -locking TESTSUITE/spool/db/retry.lockfile LOG: MAIN Completed >>>>>>>>>>>>>>>> Exim pid=pppp (main) terminating with rc=0 >>>>>>>>>>>>>>>> diff --git a/test/stdout/0264 b/test/stdout/0264 index 48b789e6a..4fbb7da87 100644 --- a/test/stdout/0264 +++ b/test/stdout/0264 @@ -12,3 +12,12 @@ Message 10HmbB-0005vi-00 has been removed Message 10HmbC-0005vi-00 has been removed Message 10HmbD-0005vi-00 has been removed Message 10HmbE-0005vi-00 has been removed ++++++++++++++++++++++++++++ + R:r4.a@outside -1 0 not just now +first failed = time last try = time2 next try = time2 + 300 +Message 10HmbF-0005vi-00 has been removed +Message 10HmbG-0005vi-00 has been removed +Message 10HmbH-0005vi-00 has been removed ++++++++++++++++++++++++++++ + R:outside -1 0 not just now +first failed = time last try = time2 next try = time2 + 300 -- 2.30.2