From 846726c5a374d833fb5211dde62ae6bceb6841c7 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 8 Jul 2012 22:49:18 +0100 Subject: [PATCH] Multiple headers_add/remove options per router/transport - fixes bug 337 --- doc/doc-docbook/spec.xfpt | 23 ++++++++++++++++++++++- doc/doc-txt/ChangeLog | 2 ++ doc/doc-txt/NewStuff | 3 +++ src/src/macros.h | 4 +++- src/src/readconf.c | 32 +++++++++++++++++--------------- src/src/route.c | 6 +++--- src/src/transport.c | 4 ++-- test/confs/0481 | 2 ++ test/log/0481 | 3 +++ test/mail/0481.userx | 13 +++++++++++++ test/scripts/0000-Basic/0481 | 7 ++++++- 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 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 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 ) + id 10HmaY-0005vi-00 + for userx@myhost.test.ex; Tue, 2 Mar 1999 09:44:33 +0000 +Another: This is another header +Message-Id: +From: CALLER_NAME +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! +**** -- 2.25.1