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

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index eb359d0882783ac5e0937c870e652be0f99ed7ec..8c738c0edecb12c461bab7fc178fdf93277e99c0 100644 (file)
--- 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 a9c9abed8c4c66873267ede00eb0751cb27414b1..be52bf98fd3bf9b657148656eebcbaa8e26bba7b 100644 (file)
--- 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 53d533dea2e658e6233616ecf8347babec05ca77..e684344c909d5ca0d0b66de5b93138843f9fbd2b 100644 (file)
--- 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 d25071aae1c8dd6ca827d5747dba01910c17a2bf..3ec94bc6da50920871a5005f87fbb15ebae49673 100644 (file)
--- 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 087ab5b9b0a01706437ea3d3905d5ac990e3213b..ddd81d1d33f54d0c8b369a265057e040c614b474 100644 (file)
--- 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 43ffc25a99c7339603902fc941285cdefe3ffe66..32dbd60abd04ea207d7f988f56f61dd21bf207d5 100644 (file)
--- 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 9e533f63e77378ec0c0216e0f2aa814c48b0bf40..7c79bb0095b802fa845b2c82b041c846f50f7ff8 100644 (file)
--- 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 be3f923ddd7d616e61deeba129a5d4c0af2fc001..212af518dffff0cd285bc5eaa7833b94ead6262d 100644 (file)
--- 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 3c0b2ab01f9b7836929e1a7d72483e837284c6de..b95b0950af62bc63bee2f48d1087a9ccbde4656c 100644 (file)
--- 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 a9f142c3547bf3ffbdce5fe8b12be9e37e791add..07700dd925e8e8376e41c4700de7f1ade527b653 100644 (file)
--- 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 3f308f992dd6d2096e33db16464a68aef81470d1..d1a9a4a70ad7cfa0b2c388ea42ef663d796821e3 100644 (file)
--- 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!
+****