Make the retry_include_ip_address smtp transport option expanded. Bug 1545
authorJeremy Harris <jgh146exb@wizmail.org>
Sun, 16 Nov 2014 14:14:35 +0000 (14:14 +0000)
committerJeremy Harris <jgh146exb@wizmail.org>
Mon, 12 Jan 2015 18:58:33 +0000 (18:58 +0000)
13 files changed:
doc/doc-docbook/spec.xfpt
doc/doc-txt/ChangeLog
src/src/expand.c
src/src/functions.h
src/src/readconf.c
src/src/route.c
src/src/transports/smtp.c
src/src/transports/smtp.h
test/log/0099
test/msglog/0099.10HmbA-0005vi-00 [new file with mode: 0644]
test/msglog/0099.10HmbB-0005vi-00 [new file with mode: 0644]
test/scripts/0000-Basic/0099
test/stdout/0099

index c831e6cc46322f046fc1382c68a7c2c63b30770b..c6aeea145db4f67e292299c92129bca5cbdf751c 100644 (file)
@@ -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
index 7359f19ac52ef06928592f77b844d4b76897b8f3..ae2b84dd079c6c576256c34d4636f9b14aab18b6 100644 (file)
@@ -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
index a77363c6b75feab5958963343d1ef9a7c0e9dba8..d2f215dfd0a150bec644ffc6d8445dbd85b3e0c4 100644 (file)
@@ -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           *
index f7d5449bd1490f1663fb9aad2e5d044e55d2c01d..32d2997d5372c913cbd3ebe9a80fc2bcc9ebca27 100644 (file)
@@ -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 *);
index 40af94024d7a71677e3a01f516fcab8613519863..f0d08d0e1d851d77da9b884ffe16211d43f3abbc 100644 (file)
@@ -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 != '='))
index 3834b836a2ca907e4324013250704c4839770de1..c640e9e7188b8de5fdc21d39bdc151dffdd3f2be 100644 (file)
@@ -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 */
index 2fb1e599c178b1a862dddd2fdb01ebfbf39cf10a..33d357f7a17a3c0294e365bb795c2001fe2d6196 100644 (file)
@@ -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);
index 4494adfb2aa07beeab5edee4dc68730af8d2fc19..95e9195f4d536c621711466364b9c23c7ac0433e 100644 (file)
@@ -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;
index b9bf7cbda732dc54ad640da617302c1d6fb37083..952b24041d0d2da7f47b2a186d7e582a8f762c77 100644 (file)
@@ -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 (file)
index 0000000..d7ad301
--- /dev/null
@@ -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 (file)
index 0000000..d2e90a6
--- /dev/null
@@ -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
index 25228211201f04731908860f9606b09f56f02a8a..6dfc14a04eeb742534ce4ec2b139fb0565e44c66 100644 (file)
@@ -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
 ****
index e473c2b97ff717887c79f39144d01cd571319400..35b7f54b69d36ba7d1902060a29e18acaa6df4b5 100644 (file)
@@ -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;