From 9c695f6d10bd63bd44608bd01f0073fd4c7dd6e6 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 16 Nov 2014 14:14:35 +0000 Subject: [PATCH] Make the retry_include_ip_address smtp transport option expanded. Bug 1545 --- doc/doc-docbook/spec.xfpt | 7 ++- doc/doc-txt/ChangeLog | 6 +++ src/src/expand.c | 61 ++++++++++++++++++++++++++ src/src/functions.h | 3 ++ src/src/readconf.c | 2 +- src/src/route.c | 72 +++---------------------------- src/src/transports/smtp.c | 32 +++++++++++--- src/src/transports/smtp.h | 1 + test/log/0099 | 6 +++ test/msglog/0099.10HmbA-0005vi-00 | 2 + test/msglog/0099.10HmbB-0005vi-00 | 4 ++ test/scripts/0000-Basic/0099 | 12 ++++++ test/stdout/0099 | 20 +++++++++ 13 files changed, 152 insertions(+), 76 deletions(-) create mode 100644 test/msglog/0099.10HmbA-0005vi-00 create mode 100644 test/msglog/0099.10HmbB-0005vi-00 diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index c831e6cc4..c6aeea145 100644 --- a/doc/doc-docbook/spec.xfpt +++ b/doc/doc-docbook/spec.xfpt @@ -23268,7 +23268,7 @@ connecting, as an outbound SSL-on-connect, instead of using STARTTLS to upgrade. The Internet standards bodies strongly discourage use of this mode. -.option retry_include_ip_address smtp boolean true +.option retry_include_ip_address smtp boolean&!! true Exim normally includes both the host name and the IP address in the key it constructs for indexing retry data after a temporary delivery failure. This means that when one of several IP addresses for a host is failing, it gets @@ -23278,9 +23278,8 @@ addresses is not affected. However, in some dialup environments hosts are assigned a different IP address each time they connect. In this situation the use of the IP address as part of the retry key leads to undesirable behaviour. Setting this option false causes -Exim to use only the host name. This should normally be done on a separate -instance of the &(smtp)& transport, set up specially to handle the dialup -hosts. +Exim to use only the host name. +Since it is expanded it can be made to depend on the host or domain. .option serialize_hosts smtp "host list&!!" unset diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 7359f19ac..ae2b84dd0 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -2,6 +2,12 @@ Change log file for Exim from version 4.21 ------------------------------------------- +Exim version 4.86 +----------------- +JH/01 Bug 1545: The smtp transport option "retry_include_ip_address" is now + expanded. + + Exim version 4.85 ----------------- TL/01 When running the test suite, the README says that variables such as diff --git a/src/src/expand.c b/src/src/expand.c index a77363c6b..d2f215dfd 100644 --- a/src/src/expand.c +++ b/src/src/expand.c @@ -7138,6 +7138,67 @@ return -2; } +/* These values are usually fixed boolean values, but they are permitted to be +expanded strings. + +Arguments: + addr address being routed + mtype the module type + mname the module name + dbg_opt debug selectors + oname the option name + bvalue the router's boolean value + svalue the router's string value + rvalue where to put the returned value + +Returns: OK value placed in rvalue + DEFER expansion failed +*/ + +int +exp_bool(address_item *addr, + uschar *mtype, uschar *mname, unsigned dbg_opt, + uschar *oname, BOOL bvalue, + uschar *svalue, BOOL *rvalue) +{ +uschar *expanded; +if (svalue == NULL) { *rvalue = bvalue; return OK; } + +expanded = expand_string(svalue); +if (expanded == NULL) + { + if (expand_string_forcedfail) + { + DEBUG(dbg_opt) debug_printf("expansion of \"%s\" forced failure\n", oname); + *rvalue = bvalue; + return OK; + } + addr->message = string_sprintf("failed to expand \"%s\" in %s %s: %s", + oname, mname, mtype, expand_string_message); + DEBUG(dbg_opt) debug_printf("%s\n", addr->message); + return DEFER; + } + +DEBUG(dbg_opt) debug_printf("expansion of \"%s\" yields \"%s\"\n", oname, + expanded); + +if (strcmpic(expanded, US"true") == 0 || strcmpic(expanded, US"yes") == 0) + *rvalue = TRUE; +else if (strcmpic(expanded, US"false") == 0 || strcmpic(expanded, US"no") == 0) + *rvalue = FALSE; +else + { + addr->message = string_sprintf("\"%s\" is not a valid value for the " + "\"%s\" option in the %s %s", expanded, oname, mname, mtype); + return DEFER; + } + +return OK; +} + + + + /************************************************* ************************************************** * Stand-alone test program * diff --git a/src/src/functions.h b/src/src/functions.h index f7d5449bd..32d2997d5 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -165,6 +165,9 @@ extern void exim_nullstd(void); extern void exim_setugid(uid_t, gid_t, BOOL, uschar *); extern int exim_tvcmp(struct timeval *, struct timeval *); extern void exim_wait_tick(struct timeval *, int); +extern int exp_bool(address_item *addr, + uschar *mtype, uschar *mname, unsigned dgb_opt, uschar *oname, BOOL bvalue, + uschar *svalue, BOOL *rvalue); extern BOOL expand_check_condition(uschar *, uschar *, uschar *); extern uschar *expand_string(uschar *); extern uschar *expand_string_copy(uschar *); diff --git a/src/src/readconf.c b/src/src/readconf.c index 40af94024..f0d08d0e1 100644 --- a/src/src/readconf.c +++ b/src/src/readconf.c @@ -1488,7 +1488,7 @@ if (type < opt_bool || type > opt_bool_last) } /* If a boolean wasn't preceded by "no[t]_" it can be followed by = and -true/false/yes/no, or, in the case of opt_expanded_bool, a general string that +true/false/yes/no, or, in the case of opt_expand_bool, a general string that ultimately expands to one of those values. */ else if (*s != 0 && (offset != 0 || *s != '=')) diff --git a/src/src/route.c b/src/src/route.c index 3834b836a..c640e9e71 100644 --- a/src/src/route.c +++ b/src/src/route.c @@ -1305,67 +1305,6 @@ return yield; -/************************************************* -* Sort out "more" or "unseen" * -*************************************************/ - -/* These values are usually fixed boolean values, but they are permitted to be -expanded strings. - -Arguments: - addr address being routed - rname the router name - oname the option name - bvalue the router's boolean value - svalue the router's string value - rvalue where to put the returned value - -Returns: OK value placed in rvalue - DEFER expansion failed -*/ - -static int -exp_bool(address_item *addr, uschar *rname, uschar *oname, BOOL bvalue, - uschar *svalue, BOOL *rvalue) -{ -uschar *expanded; -if (svalue == NULL) { *rvalue = bvalue; return OK; } - -expanded = expand_string(svalue); -if (expanded == NULL) - { - if (expand_string_forcedfail) - { - DEBUG(D_route) debug_printf("expansion of \"%s\" forced failure\n", oname); - *rvalue = bvalue; - return OK; - } - addr->message = string_sprintf("failed to expand \"%s\" in %s router: %s", - oname, rname, expand_string_message); - DEBUG(D_route) debug_printf("%s\n", addr->message); - return DEFER; - } - -DEBUG(D_route) debug_printf("expansion of \"%s\" yields \"%s\"\n", oname, - expanded); - -if (strcmpic(expanded, US"true") == 0 || strcmpic(expanded, US"yes") == 0) - *rvalue = TRUE; -else if (strcmpic(expanded, US"false") == 0 || strcmpic(expanded, US"no") == 0) - *rvalue = FALSE; -else - { - addr->message = string_sprintf("\"%s\" is not a valid value for the " - "\"%s\" option in the %s router", expanded, oname, rname); - return DEFER; - } - -return OK; -} - - - - /************************************************* * Handle an unseen routing * *************************************************/ @@ -1689,8 +1628,8 @@ for (r = (addr->start_router == NULL)? routers : addr->start_router; /* Expand "more" if necessary; DEFER => an expansion failed */ - yield = exp_bool(addr, r->name, US"more", r->more, r->expand_more, - &more); + yield = exp_bool(addr, US"router", r->name, D_route, + US"more", r->more, r->expand_more, &more); if (yield != OK) goto ROUTE_EXIT; if (!more) @@ -1799,7 +1738,8 @@ for (r = (addr->start_router == NULL)? routers : addr->start_router; { /* Expand "more" if necessary */ - yield = exp_bool(addr, r->name, US"more", r->more, r->expand_more, &more); + yield = exp_bool(addr, US"router", r->name, D_route, + US"more", r->more, r->expand_more, &more); if (yield != OK) goto ROUTE_EXIT; if (!more) @@ -1939,8 +1879,8 @@ if (r->translate_ip_address != NULL) /* See if this is an unseen routing; first expand the option if necessary. DEFER can be given if the expansion fails */ -yield = exp_bool(addr, r->name, US"unseen", r->unseen, r->expand_unseen, - &unseen); +yield = exp_bool(addr, US"router", r->name, D_route, + US"unseen", r->unseen, r->expand_unseen, &unseen); if (yield != OK) goto ROUTE_EXIT; /* Debugging output recording a successful routing */ diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index 2fb1e599c..33d357f7a 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -19,6 +19,9 @@ before the lower case letters). Some live in the transport_instance block so as to be publicly visible; these are flagged with opt_public. */ optionlist smtp_transport_options[] = { + { "*expand_retry_include_ip_address", opt_stringptr | opt_hidden, + (void *)(offsetof(smtp_transport_options_block, expand_retry_include_ip_address)) }, + { "address_retry_include_sender", opt_bool, (void *)offsetof(smtp_transport_options_block, address_retry_include_sender) }, { "allow_localhost", opt_bool, @@ -148,7 +151,7 @@ optionlist smtp_transport_options[] = { (void *)offsetof(smtp_transport_options_block, port) }, { "protocol", opt_stringptr, (void *)offsetof(smtp_transport_options_block, protocol) }, - { "retry_include_ip_address", opt_bool, + { "retry_include_ip_address", opt_expand_bool, (void *)offsetof(smtp_transport_options_block, retry_include_ip_address) }, { "serialize_hosts", opt_stringptr, (void *)offsetof(smtp_transport_options_block, serialize_hosts) }, @@ -241,6 +244,7 @@ smtp_transport_options_block smtp_transport_option_defaults = { FALSE, /* hosts_randomize */ TRUE, /* keepalive */ FALSE, /* lmtp_ignore_quota */ + NULL, /* expand_retry_include_ip_address */ TRUE /* retry_include_ip_address */ #ifdef SUPPORT_TLS ,NULL, /* tls_certificate */ @@ -3178,14 +3182,20 @@ for (cutoff_retry = 0; expired && if (cutoff_retry == 0) { + BOOL incl_ip; /* Ensure the status of the address is set by checking retry data if - necessary. There maybe host-specific retry data (applicable to all + necessary. There may be host-specific retry data (applicable to all messages) and also data for retries of a specific message at this host. If either of these retry records are actually read, the keys used are returned to save recomputing them later. */ + if (exp_bool(addrlist, US"transport", tblock->name, D_transport, + US"retry_include_ip_address", ob->retry_include_ip_address, + ob->expand_retry_include_ip_address, &incl_ip) != OK) + continue; /* with next host */ + host_is_expired = retry_check_address(addrlist->domain, host, pistring, - ob->retry_include_ip_address, &retry_host_key, &retry_message_key); + incl_ip, &retry_host_key, &retry_message_key); DEBUG(D_transport) debug_printf("%s [%s]%s status = %s\n", host->name, (host->address == NULL)? US"" : host->address, pistring, @@ -3437,7 +3447,13 @@ for (cutoff_retry = 0; expired && int delete_flag = (rc != DEFER)? rf_delete : 0; if (retry_host_key == NULL) { - retry_host_key = ob->retry_include_ip_address? + BOOL incl_ip; + if (exp_bool(addrlist, US"transport", tblock->name, D_transport, + US"retry_include_ip_address", ob->retry_include_ip_address, + ob->expand_retry_include_ip_address, &incl_ip) != OK) + incl_ip = TRUE; /* error; use most-specific retry record */ + + retry_host_key = incl_ip ? string_sprintf("T:%S:%s%s", host->name, host->address, pistring) : string_sprintf("T:%S%s", host->name, pistring); } @@ -3479,7 +3495,13 @@ for (cutoff_retry = 0; expired && int delete_flag = message_defer? 0 : rf_delete; if (retry_message_key == NULL) { - retry_message_key = ob->retry_include_ip_address? + BOOL incl_ip; + if (exp_bool(addrlist, US"transport", tblock->name, D_transport, + US"retry_include_ip_address", ob->retry_include_ip_address, + ob->expand_retry_include_ip_address, &incl_ip) != OK) + incl_ip = TRUE; /* error; use most-specific retry record */ + + retry_message_key = incl_ip ? string_sprintf("T:%S:%s%s:%s", host->name, host->address, pistring, message_id) : string_sprintf("T:%S%s:%s", host->name, pistring, message_id); diff --git a/src/src/transports/smtp.h b/src/src/transports/smtp.h index 4494adfb2..95e9195f4 100644 --- a/src/src/transports/smtp.h +++ b/src/src/transports/smtp.h @@ -58,6 +58,7 @@ typedef struct { BOOL hosts_randomize; BOOL keepalive; BOOL lmtp_ignore_quota; + uschar *expand_retry_include_ip_address; BOOL retry_include_ip_address; #ifdef SUPPORT_TLS uschar *tls_certificate; diff --git a/test/log/0099 b/test/log/0099 index b9bf7cbda..952b24041 100644 --- a/test/log/0099 +++ b/test/log/0099 @@ -15,3 +15,9 @@ 1999-03-02 09:44:33 10HmaZ-0005vi-00 <= CALLER@test.ex U=CALLER P=local S=sss for userz@simple 1999-03-02 09:44:33 10HmaZ-0005vi-00 H=thishost.test.ex [127.0.0.1] Connection refused 1999-03-02 09:44:33 10HmaZ-0005vi-00 == userz@simple R=all T=smtp defer (dd): Connection refused +1999-03-02 09:44:33 10HmbA-0005vi-00 <= CALLER@test.ex U=CALLER P=local S=sss for with@complex +1999-03-02 09:44:33 10HmbA-0005vi-00 == with@complex R=all T=smtp defer (-53): retry time not reached for any host +1999-03-02 09:44:33 10HmbB-0005vi-00 <= CALLER@test.ex U=CALLER P=local S=sss for without@complex +1999-03-02 09:44:33 10HmbB-0005vi-00 H=thisloop.test.ex [ip4.ip4.ip4.ip4] Connection refused +1999-03-02 09:44:33 10HmbB-0005vi-00 H=thisloop.test.ex [127.0.0.1] Connection refused +1999-03-02 09:44:33 10HmbB-0005vi-00 == without@complex R=all T=smtp defer (dd): Connection refused diff --git a/test/msglog/0099.10HmbA-0005vi-00 b/test/msglog/0099.10HmbA-0005vi-00 new file mode 100644 index 000000000..d7ad301a1 --- /dev/null +++ b/test/msglog/0099.10HmbA-0005vi-00 @@ -0,0 +1,2 @@ +1999-03-02 09:44:33 Received from CALLER@test.ex U=CALLER P=local S=sss +1999-03-02 09:44:33 with@complex R=all T=smtp defer (-53): retry time not reached for any host diff --git a/test/msglog/0099.10HmbB-0005vi-00 b/test/msglog/0099.10HmbB-0005vi-00 new file mode 100644 index 000000000..d2e90a6fd --- /dev/null +++ b/test/msglog/0099.10HmbB-0005vi-00 @@ -0,0 +1,4 @@ +1999-03-02 09:44:33 Received from CALLER@test.ex U=CALLER P=local S=sss +1999-03-02 09:44:33 H=thisloop.test.ex [ip4.ip4.ip4.ip4] Connection refused +1999-03-02 09:44:33 H=thisloop.test.ex [127.0.0.1] Connection refused +1999-03-02 09:44:33 without@complex R=all T=smtp defer (dd): Connection refused diff --git a/test/scripts/0000-Basic/0099 b/test/scripts/0000-Basic/0099 index 252282112..6dfc14a04 100644 --- a/test/scripts/0000-Basic/0099 +++ b/test/scripts/0000-Basic/0099 @@ -26,6 +26,18 @@ Test message **** dump retry # +# expanded option, giving true, should leave the localhost pair unchanged +exim -odi -DRETRY='retry_include_ip_address=${if eq{with}{$local_part} {yes}{no}}' with@complex +Test message +**** +dump retry +# +# expanded option, giving false, should add another localhost entry, without IP +exim -odi -DRETRY='retry_include_ip_address=${if eq{with}{$local_part} {yes}{no}}' without@complex +Test message +**** +dump retry +# # exim -brt x@dark.star.ex **** diff --git a/test/stdout/0099 b/test/stdout/0099 index e473c2b97..35b7f54b6 100644 --- a/test/stdout/0099 +++ b/test/stdout/0099 @@ -20,6 +20,26 @@ first failed = time last try = time2 next try = time2 + 900 first failed = time last try = time2 next try = time2 + 900 T:thisloop.test.ex:ip4.ip4.ip4.ip4:999 dd 65 Connection refused first failed = time last try = time2 next try = time2 + 900 ++++++++++++++++++++++++++++ + T:thishost.test.ex:127.0.0.1:999 dd 65 Connection refused +first failed = time last try = time2 next try = time2 + 900 + T:thishost.test.ex:999 dd 65 Connection refused +first failed = time last try = time2 next try = time2 + 900 + T:thisloop.test.ex:127.0.0.1:999 dd 65 Connection refused +first failed = time last try = time2 next try = time2 + 900 + T:thisloop.test.ex:ip4.ip4.ip4.ip4:999 dd 65 Connection refused +first failed = time last try = time2 next try = time2 + 900 ++++++++++++++++++++++++++++ + T:thishost.test.ex:127.0.0.1:999 dd 65 Connection refused +first failed = time last try = time2 next try = time2 + 900 + T:thishost.test.ex:999 dd 65 Connection refused +first failed = time last try = time2 next try = time2 + 900 + T:thisloop.test.ex:127.0.0.1:999 dd 65 Connection refused +first failed = time last try = time2 next try = time2 + 900 + T:thisloop.test.ex:ip4.ip4.ip4.ip4:999 dd 65 Connection refused +first failed = time last try = time2 next try = time2 + 900 + T:thisloop.test.ex:999 dd 65 Connection refused +first failed = time last try = time2 next try = time2 + 900 Retry rule: *.star.ex * F,3d,10m; Retry rule: lsearch*@;TESTSUITE/aux-fixed/0099.rlist * F,1d,3m; Retry rule: !*.not.ex * F,2d,15m; -- 2.30.2