Retry: always use interface, if set, for retry DB key. Bug 1678

Even constant values must be used, as multiple transports with
different values may be in play and should be kept distinct.

11 files changed, 151 insertions, 16 deletions

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 37d66617e..14f0dc737 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -49,6 +49,11 @@ HS/02 Add the Exim version string to the process info.  This way exiwhat
 JH/06 Bug 1395: time-limit cacheing of DNS lookups, to the TTL value.  This may
       matter for fast-change records such as DNSBLs.
 
+JH/07 Bug 1678: Always record an interface option value, if set,  as part of a
+      retry record, even if constant.  There may be multiple transports with
+      different interface settings and the retry behaviour needs to be kept
+      distinct.
+
 
 Exim version 4.86
 -----------------
diff --git a/src/src/functions.h b/src/src/functions.h
index 4af0017c1..fcfe74675 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -377,7 +377,7 @@ extern int     smtp_sock_connect(host_item *, int, int, uschar *,
 extern int     smtp_feof(void);
 extern int     smtp_ferror(void);
 extern uschar *smtp_get_connection_info(void);
-extern BOOL    smtp_get_interface(uschar *, int, address_item *, BOOL *,
+extern BOOL    smtp_get_interface(uschar *, int, address_item *,
                  uschar **, uschar *);
 extern BOOL    smtp_get_port(uschar *, address_item *, int *, uschar *);
 extern int     smtp_getc(void);
diff --git a/src/src/smtp_out.c b/src/src/smtp_out.c
index 88dde8301..d32ef89b5 100644
--- a/src/src/smtp_out.c
+++ b/src/src/smtp_out.c
@@ -26,7 +26,6 @@ Arguments:
                which case the function does nothing
   host_af    AF_INET or AF_INET6 for the outgoing IP address
   addr       the mail address being handled (for setting errors)
-  changed    if not NULL, set TRUE if expansion actually changed istring
   interface  point this to the interface
   msg        to add to any error message
 
@@ -36,7 +35,7 @@ Returns:     TRUE on success, FALSE on failure, with error message
 
 BOOL
 smtp_get_interface(uschar *istring, int host_af, address_item *addr,
-  BOOL *changed, uschar **interface, uschar *msg)
+  uschar **interface, uschar *msg)
 {
 const uschar * expint;
 uschar *iface;
@@ -54,8 +53,6 @@ if (expint == NULL)
   return FALSE;
   }
 
-if (changed != NULL) *changed = expint != istring;
-
 while (isspace(*expint)) expint++;
 if (*expint == 0) return TRUE;
 
diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c
index ac40460f1..f129cce9b 100644
--- a/src/src/transports/smtp.c
+++ b/src/src/transports/smtp.c
@@ -3259,7 +3259,6 @@ for (cutoff_retry = 0; expired &&
     BOOL serialized = FALSE;
     BOOL host_is_expired = FALSE;
     BOOL message_defer = FALSE;
-    BOOL ifchanges = FALSE;
     BOOL some_deferred = FALSE;
     address_item *first_addr = NULL;
     uschar *interface = NULL;
@@ -3435,15 +3434,18 @@ for (cutoff_retry = 0; expired &&
     if (Ustrcmp(pistring, ":25") == 0) pistring = US"";
 
     /* Select IPv4 or IPv6, and choose an outgoing interface. If the interface
-    string changes upon expansion, we must add it to the key that is used for
-    retries, because connections to the same host from a different interface
-    should be treated separately. */
+    string is set, even if constant (as different transports can have different
+    constant settings), we must add it to the key that is used for retries,
+    because connections to the same host from a different interface should be
+    treated separately. */
 
     host_af = (Ustrchr(host->address, ':') == NULL)? AF_INET : AF_INET6;
-    if (!smtp_get_interface(ob->interface, host_af, addrlist, &ifchanges,
-         &interface, tid))
-      return FALSE;
-    if (ifchanges) pistring = string_sprintf("%s/%s", pistring, interface);
+    if ((rs = ob->interface) && *rs)
+      {
+      if (!smtp_get_interface(rs, host_af, addrlist, &interface, tid))
+	return FALSE;
+      pistring = string_sprintf("%s/%s", pistring, interface);
+      }
 
     /* The first time round the outer loop, check the status of the host by
     inspecting the retry data. The second time round, we are interested only
diff --git a/src/src/verify.c b/src/src/verify.c
index dc9c58224..cb88f28a9 100644
--- a/src/src/verify.c
+++ b/src/src/verify.c
@@ -444,7 +444,7 @@ can do it there for the non-rcpt-verify case.  For this we keep an addresscount.
 
 	  host_af = (Ustrchr(host->address, ':') == NULL)? AF_INET:AF_INET6;
 
-	  if (!smtp_get_interface(tf->interface, host_af, addr, NULL, &interface,
+	  if (!smtp_get_interface(tf->interface, host_af, addr, &interface,
 		  US"callout") ||
 	      !smtp_get_port(tf->port, addr, &port, US"callout"))
 	    log_write(0, LOG_MAIN|LOG_PANIC, "<%s>: %s", addr->address,
@@ -579,7 +579,7 @@ can do it there for the non-rcpt-verify case.  For this we keep an addresscount.
     deliver_domain = addr->domain;
     transport_name = addr->transport->name;
 
-    if (  !smtp_get_interface(tf->interface, host_af, addr, NULL, &interface,
+    if (  !smtp_get_interface(tf->interface, host_af, addr, &interface,
             US"callout")
        || !smtp_get_port(tf->port, addr, &port, US"callout")
        )
diff --git a/test/confs/0610 b/test/confs/0610
new file mode 100644
index 000000000..f805c086c
--- /dev/null
+++ b/test/confs/0610
@@ -0,0 +1,65 @@
+# Exim test configuration 0610
+
+SERVER =
+
+exim_path = EXIM_PATH
+host_lookup_order = bydns
+#primary_hostname = myhost.test.ex
+rfc1413_query_timeout = 0s
+spool_directory = DIR/spool
+log_file_path = DIR/spool/log/SERVER%slog
+log_selector = +sender_on_delivery
+gecos_pattern = ""
+gecos_name = CALLER_NAME
+
+# ----- Main settings -----
+
+acl_smtp_connect = conn_chk
+acl_smtp_rcpt = accept
+
+untrusted_set_sender = *
+queue_only
+queue_run_in_order
+
+# ----- ACL -----
+
+begin acl
+
+conn_chk:
+  defer	condition = ${if eq {SERVER}{server}}
+  accept
+
+# ----- Routers -----
+
+begin routers
+
+client:
+  driver = accept
+  transport = $sender_address_local_part
+
+# ----- Transports -----
+
+begin transports
+
+t1:
+  driver = smtp
+  allow_localhost
+  hosts = 127.0.0.1
+  port = PORT_D
+  interface = 127.0.0.1
+
+t2:
+  driver = smtp
+  allow_localhost
+  hosts = 127.0.0.1
+  port = PORT_D
+  interface = 127.0.0.2
+
+# ----- Retry -----
+
+begin retry
+
+* * F,5d,10s
+
+# End
+
diff --git a/test/log/0610 b/test/log/0610
new file mode 100644
index 000000000..d9e584b6d
--- /dev/null
+++ b/test/log/0610
@@ -0,0 +1,13 @@
+1999-03-02 09:44:33 10HmaX-0005vi-00 <= t1@dustyshoes.tld U=CALLER P=local-smtp S=sss
+1999-03-02 09:44:33 10HmaY-0005vi-00 <= t2@dustybelt.tld U=CALLER P=local-smtp S=sss
+1999-03-02 09:44:33 Start queue run: pid=pppp -qq
+1999-03-02 09:44:33 10HmaX-0005vi-00 H=127.0.0.1 [127.0.0.1]: SMTP error from remote mail server after initial connection: 451 Temporary local problem - please try later
+1999-03-02 09:44:33 10HmaX-0005vi-00 == fred@anotherone.tld R=client T=t1 defer (0) H=127.0.0.1 [127.0.0.1]: SMTP error from remote mail server after initial connection: 451 Temporary local problem - please try later
+1999-03-02 09:44:33 10HmaY-0005vi-00 H=127.0.0.1 [127.0.0.1]: SMTP error from remote mail server after initial connection: 451 Temporary local problem - please try later
+1999-03-02 09:44:33 10HmaY-0005vi-00 == fred@anotherone.tld R=client T=t2 defer (0) H=127.0.0.1 [127.0.0.1]: SMTP error from remote mail server after initial connection: 451 Temporary local problem - please try later
+1999-03-02 09:44:33 End queue run: pid=pppp -qq
+
+******** SERVER ********
+1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
+1999-03-02 09:44:33 H=[127.0.0.1] temporarily rejected connection in "connect" ACL
+1999-03-02 09:44:33 H=[127.0.0.2] temporarily rejected connection in "connect" ACL
diff --git a/test/rejectlog/0610 b/test/rejectlog/0610
new file mode 100644
index 000000000..c1384b3de
--- /dev/null
+++ b/test/rejectlog/0610
@@ -0,0 +1,4 @@
+
+******** SERVER ********
+1999-03-02 09:44:33 H=[127.0.0.1] temporarily rejected connection in "connect" ACL
+1999-03-02 09:44:33 H=[127.0.0.2] temporarily rejected connection in "connect" ACL
diff --git a/test/scripts/0000-Basic/0610 b/test/scripts/0000-Basic/0610
new file mode 100644
index 000000000..ff690f63f
--- /dev/null
+++ b/test/scripts/0000-Basic/0610
@@ -0,0 +1,33 @@
+# retry: transport with fixed interface
+# Exim test configuration 0610
+#
+exim -DSERVER=server -bd -oX PORT_D
+****
+#
+exim -bs
+MAIL FROM:<t1@dustyshoes.tld>
+RCPT TO:<fred@anotherone.tld>
+DATA
+This is a test message.
+.
+RSET
+MAIL FROM:<t2@dustybelt.tld>
+RCPT TO:<fred@anotherone.tld>
+DATA
+Subject: second
+
+This is a second test message.
+.
+QUIT
+****
+#
+#
+exim -qq
+****
+#
+# Should get two separate retry records.
+dump retry
+#
+#
+killdaemon
+no_msglog_check
diff --git a/test/stderr/0143 b/test/stderr/0143
index 054d65d28..cff499045 100644
--- a/test/stderr/0143
+++ b/test/stderr/0143
@@ -15,7 +15,7 @@ transport_name <my_smtp>
 my_smtp transport entered
   userx@domain.com
 checking status of 127.0.0.1
-127.0.0.1 [127.0.0.1]:1111 status = usable
+127.0.0.1 [127.0.0.1]:1111/ip4.ip4.ip4.ip4 status = usable
 delivering 10HmaX-0005vi-00 to 127.0.0.1 [127.0.0.1] (userx@domain.com)
 Connecting to 127.0.0.1 [127.0.0.1]:1224 from ip4.ip4.ip4.ip4 ... connected
   SMTP<< 220 ESMTP
diff --git a/test/stdout/0610 b/test/stdout/0610
new file mode 100644
index 000000000..24291b53f
--- /dev/null
+++ b/test/stdout/0610
@@ -0,0 +1,16 @@
+220 the.local.host.name ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000

+250 OK

+250 Accepted

+354 Enter message, ending with "." on a line by itself

+250 OK id=10HmaX-0005vi-00

+250 Reset OK

+250 OK

+250 Accepted

+354 Enter message, ending with "." on a line by itself

+250 OK id=10HmaY-0005vi-00

+221 the.local.host.name closing connection

++++++++++++++++++++++++++++
+  T:127.0.0.1:127.0.0.1:1225/127.0.0.1 0 65 H=127.0.0.1 [127.0.0.1]: SMTP error from remote mail server after initial connection: 451 Temporary local problem - please try later
+first failed = time last try = time2 next try = time2 + 10
+  T:127.0.0.1:127.0.0.1:1225/127.0.0.2 0 65 H=127.0.0.1 [127.0.0.1]: SMTP error from remote mail server after initial connection: 451 Temporary local problem - please try later
+first failed = time last try = time2 next try = time2 + 10