Multiple headers_add/remove options per router/transport - fixes bug 337

11 files changed, 76 insertions, 23 deletions

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index eb359d088..8c738c0ed 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -16333,7 +16333,7 @@ router is skipped, and the address is offered to the next one.
 If the result is any other value, the router is run (as this is the last
 precondition to be evaluated, all the other preconditions must be true).
 
-This option is unique in that multiple &%condition%& options may be present.
+This option is unusual in that multiple &%condition%& options may be present.
 All &%condition%& options must succeed.
 
 The &%condition%& option provides a means of applying custom conditions to the
@@ -16532,6 +16532,9 @@ The &%headers_add%& option is expanded after &%errors_to%&, but before
 the expansion is forced to fail, the option has no effect. Other expansion
 failures are treated as configuration errors.
 
+Unlike most options, &%headers_add%& can be specified multiple times
+for a router; all listed headers are added.
+
 &*Warning 1*&: The &%headers_add%& option cannot be used for a &(redirect)&
 router that has the &%one_time%& option set.
 
@@ -16565,6 +16568,9 @@ The &%headers_remove%& option is expanded after &%errors_to%& and
 the option has no effect. Other expansion failures are treated as configuration
 errors.
 
+Unlike most options, &%headers_remove%& can be specified multiple times
+for a router; all listed headers are removed.
+
 &*Warning 1*&: The &%headers_remove%& option cannot be used for a &(redirect)&
 router that has the &%one_time%& option set.
 
@@ -19568,6 +19574,9 @@ routers. If the result of the expansion is an empty string, or if the expansion
 is forced to fail, no action is taken. Other expansion failures are treated as
 errors and cause the delivery to be deferred.
 
+Unlike most options, &%headers_add%& can be specified multiple times
+for a transport; all listed headers are added.
+
 
 
 .option headers_only transports boolean false
@@ -19590,6 +19599,9 @@ routers. If the result of the expansion is an empty string, or if the expansion
 is forced to fail, no action is taken. Other expansion failures are treated as
 errors and cause the delivery to be deferred.
 
+Unlike most options, &%headers_remove%& can be specified multiple times
+for a router; all listed headers are added.
+
 
 
 .option headers_rewrite transports string unset
@@ -31481,6 +31493,10 @@ headers_add = X-added-header: added by $primary_hostname\n\
 .endd
 Exim does not check the syntax of these added header lines.
 
+Multiple &%headers_add%& options for a single router or transport can be
+specified; the values will be concatenated (with a separating newline
+added) before expansion.
+
 The result of expanding &%headers_remove%& must consist of a colon-separated
 list of header names. This is confusing, because header names themselves are
 often terminated by colons. In this case, the colons are the list separators,
@@ -31488,6 +31504,11 @@ not part of the names. For example:
 .code
 headers_remove = return-receipt-to:acknowledge-to
 .endd
+
+Multiple &%headers_remove%& options for a single router or transport can be
+specified; the values will be concatenated (with a separating colon
+added) before expansion.
+
 When &%headers_add%& or &%headers_remove%& is specified on a router, its value
 is expanded at routing time, and then associated with all addresses that are
 accepted by that router, and also with any new addresses that it generates. If
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index a9c9abed8..be52bf98f 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -53,6 +53,8 @@ JH/04 Add expansion item ${acl {name}{arg}...}, expansion condition
       "acl {{name}{arg}...}", and optional args on acl condition
       "acl = name arg..."
 
+JH/05 Permit multiple router/transport headers_add/remove lines.
+
 Exim version 4.80
 -----------------
 
diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff
index 53d533dea..e684344c9 100644
--- a/doc/doc-txt/NewStuff
+++ b/doc/doc-txt/NewStuff
@@ -106,6 +106,9 @@ Version 4.81
     accept the expansion condition is true; if reject, false.  A defer
     return results in a forced fail.
 
+11. Routers and transports can now have multiple headers_add and headers_remove
+    option lines.  The concatenated list is used.
+
 Version 4.80
 ------------
 
diff --git a/src/src/macros.h b/src/src/macros.h
index d25071aae..3ec94bc6d 100644
--- a/src/src/macros.h
+++ b/src/src/macros.h
@@ -624,7 +624,9 @@ for booleans that are kept in one bit. */
 #define opt_public  0x200      /* Stored in the main instance block */
 #define opt_set     0x400      /* Option is set */
 #define opt_secure  0x800      /* "hide" prefix used */
-#define opt_mask    0x0ff
+#define opt_rep_con 0x1000     /* Can be appended to by a repeated line (condition) */
+#define opt_rep_str 0x2000     /* Can be appended to by a repeated line (string) */
+#define opt_mask    0x00ff
 
 /* Verify types when directing and routing */
 
diff --git a/src/src/readconf.c b/src/src/readconf.c
index 087ab5b9b..ddd81d1d3 100644
--- a/src/src/readconf.c
+++ b/src/src/readconf.c
@@ -1374,7 +1374,6 @@ uid_t uid;
 gid_t gid;
 BOOL boolvalue = TRUE;
 BOOL freesptr = TRUE;
-BOOL extra_condition = FALSE;
 optionlist *ol, *ol2;
 struct passwd *pw;
 void *reset_point;
@@ -1437,16 +1436,9 @@ if (ol == NULL)
   log_write(0, LOG_PANIC_DIE|LOG_CONFIG_IN, CS unknown_txt, name);
   }
 
-if ((ol->type & opt_set) != 0)
-  {
-  uschar *mname = name;
-  if (Ustrncmp(mname, "no_", 3) == 0) mname += 3;
-  if (Ustrcmp(mname, "condition") == 0)
-    extra_condition = TRUE;
-  else
-    log_write(0, LOG_PANIC_DIE|LOG_CONFIG_IN,
-      "\"%s\" option set for the second time", mname);
-  }
+if ((ol->type & opt_set)  && !(ol->type & (opt_rep_con | opt_rep_str)))
+  log_write(0, LOG_PANIC_DIE|LOG_CONFIG_IN,
+    "\"%s\" option set for the second time", name);
 
 ol->type |= opt_set | issecure;
 type = ol->type & opt_mask;
@@ -1530,7 +1522,7 @@ switch (type)
       str_target = (uschar **)(ol->value);
     else
       str_target = (uschar **)((uschar *)data_block + (long int)(ol->value));
-    if (extra_condition)
+    if (ol->type & opt_rep_con)
       {
       /* We already have a condition, we're conducting a crude hack to let
       multiple condition rules be chained together, despite storing them in
@@ -1539,9 +1531,10 @@ switch (type)
       strtemp = string_sprintf("${if and{{bool_lax{%s}}{bool_lax{%s}}}}",
           saved_condition, sptr);
       *str_target = string_copy_malloc(strtemp);
-      /* TODO(pdp): there is a memory leak here when we set 3 or more
-      conditions; I still don't understand the store mechanism enough
-      to know what's the safe way to free content from an earlier store.
+      /* TODO(pdp): there is a memory leak here and just below
+      when we set 3 or more conditions; I still don't
+      understand the store mechanism enough to know
+      what's the safe way to free content from an earlier store.
       AFAICT, stores stack, so freeing an early stored item also stores
       all data alloc'd after it.  If we knew conditions were adjacent,
       we could survive that, but we don't.  So I *think* we need to take
@@ -1552,6 +1545,15 @@ switch (type)
       Because we only do this once, near process start-up, I'm prepared to
       let this slide for the time being, even though it rankles.  */
       }
+    else if (*str_target && (ol->type & opt_rep_str))
+     {
+      uschar sep = Ustrncmp(name, "headers_add", 11)==0 ? '\n' : ':';
+      saved_condition = *str_target;
+      strtemp = saved_condition + strlen(saved_condition)-1;
+      if (*strtemp == sep) *strtemp = 0;	/* eliminate trailing list-sep */
+      strtemp = string_sprintf("%s%c%s", saved_condition, sep, sptr);
+      *str_target = string_copy_malloc(strtemp);
+     }
     else
       {
       *str_target = sptr;
diff --git a/src/src/route.c b/src/src/route.c
index 43ffc25a9..32dbd60ab 100644
--- a/src/src/route.c
+++ b/src/src/route.c
@@ -48,7 +48,7 @@ optionlist optionlist_routers[] = {
                  (void *)(offsetof(router_instance, caseful_local_part)) },
   { "check_local_user",   opt_bool | opt_public,
                  (void *)(offsetof(router_instance, check_local_user)) },
-  { "condition",          opt_stringptr|opt_public,
+  { "condition",          opt_stringptr|opt_public|opt_rep_con,
                  (void *)offsetof(router_instance, condition) },
   { "debug_print",        opt_stringptr | opt_public,
                  (void *)offsetof(router_instance, debug_string) },
@@ -72,9 +72,9 @@ optionlist optionlist_routers[] = {
                  (void *)offsetof(router_instance, fallback_hosts) },
   { "group",              opt_expand_gid | opt_public,
                  (void *)(offsetof(router_instance, gid)) },
-  { "headers_add",        opt_stringptr|opt_public,
+  { "headers_add",        opt_stringptr|opt_public|opt_rep_str,
                  (void *)offsetof(router_instance, extra_headers) },
-  { "headers_remove",     opt_stringptr|opt_public,
+  { "headers_remove",     opt_stringptr|opt_public|opt_rep_str,
                  (void *)offsetof(router_instance, remove_headers) },
   { "ignore_target_hosts",opt_stringptr|opt_public,
                  (void *)offsetof(router_instance, ignore_target_hosts) },
diff --git a/src/src/transport.c b/src/src/transport.c
index 9e533f63e..7c79bb009 100644
--- a/src/src/transport.c
+++ b/src/src/transport.c
@@ -68,11 +68,11 @@ optionlist optionlist_transports[] = {
                  (void *)(offsetof(transport_instance, envelope_to_add)) },
   { "group",             opt_expand_gid|opt_public,
                  (void *)offsetof(transport_instance, gid) },
-  { "headers_add",      opt_stringptr|opt_public,
+  { "headers_add",      opt_stringptr|opt_public|opt_rep_str,
                  (void *)offsetof(transport_instance, add_headers) },
   { "headers_only",     opt_bool|opt_public,
                  (void *)offsetof(transport_instance, headers_only) },
-  { "headers_remove",   opt_stringptr|opt_public,
+  { "headers_remove",   opt_stringptr|opt_public|opt_rep_str,
                  (void *)offsetof(transport_instance, remove_headers) },
   { "headers_rewrite",  opt_rewrite|opt_public,
                  (void *)offsetof(transport_instance, headers_rewrite) },
diff --git a/test/confs/0481 b/test/confs/0481
index be3f923dd..212af518d 100644
--- a/test/confs/0481
+++ b/test/confs/0481
@@ -23,6 +23,7 @@ r1:
 
 r2:
   driver = redirect
+  headers_remove = Remove-Me-Also:
   headers_remove = Remove-Me:
   data = $local_part@domain
 
@@ -30,6 +31,7 @@ r3:
   driver = accept
   headers_remove = Remove-Me:
   headers_add = X-Was-Remove-Me: >$h_remove-me:<
+  headers_add = ${if def:h_remove-me-also {X-Was-Remove-Me-Also: >$h_remove-me-also:<}}
   transport = t1
 
 
diff --git a/test/log/0481 b/test/log/0481
index 3c0b2ab01..b95b0950a 100644
--- a/test/log/0481
+++ b/test/log/0481
@@ -1,3 +1,6 @@
 1999-03-02 09:44:33 10HmaX-0005vi-00 <= CALLER@myhost.test.ex U=CALLER P=local S=sss
 1999-03-02 09:44:33 10HmaX-0005vi-00 => userx <userx@myhost.test.ex> R=r3 T=t1
 1999-03-02 09:44:33 10HmaX-0005vi-00 Completed
+1999-03-02 09:44:33 10HmaY-0005vi-00 <= CALLER@myhost.test.ex U=CALLER P=local S=sss
+1999-03-02 09:44:33 10HmaY-0005vi-00 => userx <userx@myhost.test.ex> R=r3 T=t1
+1999-03-02 09:44:33 10HmaY-0005vi-00 Completed
diff --git a/test/mail/0481.userx b/test/mail/0481.userx
index a9f142c35..07700dd92 100644
--- a/test/mail/0481.userx
+++ b/test/mail/0481.userx
@@ -10,3 +10,16 @@ Date: Tue, 2 Mar 1999 09:44:33 +0000
 X-Was-Remove-Me: >this header is to be removed<
 
 
+From CALLER@myhost.test.ex Tue Mar 02 09:44:33 1999
+Received: from CALLER by myhost.test.ex with local (Exim x.yz)
+	(envelope-from <CALLER@myhost.test.ex>)
+	id 10HmaY-0005vi-00
+	for userx@myhost.test.ex; Tue, 2 Mar 1999 09:44:33 +0000
+Another: This is another header
+Message-Id: <E10HmaY-0005vi-00@myhost.test.ex>
+From: CALLER_NAME <CALLER@myhost.test.ex>
+Date: Tue, 2 Mar 1999 09:44:33 +0000
+X-Was-Remove-Me: >this header is to be removed<
+X-Was-Remove-Me-Also: >me too!<
+
+
diff --git a/test/scripts/0000-Basic/0481 b/test/scripts/0000-Basic/0481
index 3f308f992..d1a9a4a70 100644
--- a/test/scripts/0000-Basic/0481
+++ b/test/scripts/0000-Basic/0481
@@ -1,5 +1,10 @@
-# remove_headers and trailing colons
+# multiple remove_headers and trailing colons
 exim -odi userx
 Remove-Me: this header is to be removed
 Another: This is another header
 ****
+exim -odi userx
+Remove-Me: this header is to be removed
+Another: This is another header
+Remove-Me-Also: me too!
+****