Fix ${filter } for conditions modifying $value

5 files changed, 24 insertions, 9 deletions

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 4e41acf42..8c54931fe 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -9907,7 +9907,11 @@ After expansion, <&'string'&> is interpreted as a list, colon-separated by
 default, but the separator can be changed in the usual way (&<<SECTlistsepchange>>&).
 For each item
 in this list, its value is place in &$item$&, and then the condition is
-evaluated. If the condition is true, &$item$& is added to the output as an
+evaluated.
+.new
+Any modification of &$value$& by this evaluation is discarded.
+.wen
+If the condition is true, &$item$& is added to the output as an
 item in a new list; if the condition is false, the item is discarded. The
 separator used for the output list is the same as the one used for the
 input, but a separator setting is not included in the output. For example:
@@ -9915,7 +9919,8 @@ input, but a separator setting is not included in the output. For example:
 ${filter{a:b:c}{!eq{$item}{b}}}
 .endd
 yields &`a:c`&. At the end of the expansion, the value of &$item$& is restored
-to what it was before. See also the &%map%& and &%reduce%& expansion items.
+to what it was before.
+See also the &%map%& and &%reduce%& expansion items.
 
 
 .vitem &*${hash{*&<&'string1'&>&*}{*&<&'string2'&>&*}{*&<&'string3'&>&*}}*&
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index ba5bd23d9..2720fb819 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -23,10 +23,10 @@ JH/05 Follow symlinks for placing a watch on TLS creds files.  This means
       it would be the dir with the first symlink.  We still do not monitor
       the entire path.
 
-JH/06 Check for bad chars in rDNS for sender_host_name. The OpenBSD (at least)
+JH/06 Check for bad chars in rDNS for sender_host_name.  The OpenBSD (at least)
       dn_expand() is happy to pass them through.
 
-JH/07 OpenSSL Fix auto-reload of changed server OCSP proof. Previously, if
+JH/07 OpenSSL Fix auto-reload of changed server OCSP proof.  Previously, if
       the file with the proof had an unchanged name, the new proof(s) were
       loaded on top of the old ones (and nover used; the old ones were stapled).
 
@@ -36,6 +36,10 @@ JH/08 Bug 2915: Fix use-after-free for $regex<n> variables. Previously when
       These variables were introduced in Exim 4.87.
       Debug help from Graeme Fowler.
 
+JH/09 Fix ${filter } for conditions that modify $value.  Previously the
+      modified version would be used in construction the result, and a memory
+      error would occur.
+
 
 Exim version 4.96
 -----------------
diff --git a/src/src/expand.c b/src/src/expand.c
index 831ca2b75..ff0b4d600 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -6544,6 +6544,9 @@ while (*s)
         if (item_type == EITEM_FILTER)
           {
           BOOL condresult;
+	  /* the condition could modify $value, as a side-effect */
+	  uschar * save_value = lookup_value;
+
           if (!eval_condition(expr, &resetok, &condresult))
             {
             iterate_item = save_iterate_item;
@@ -6552,6 +6555,7 @@ while (*s)
               expand_string_message, name);
             goto EXPAND_FAILED;
             }
+	  lookup_value = save_value;
           DEBUG(D_expand) debug_printf_indent("%s: condition is %s\n", name,
             condresult? "true":"false");
           if (condresult)
@@ -6560,14 +6564,12 @@ while (*s)
             continue;               /* FALSE => skip this item */
           }
 
-        /* EITEM_MAP and EITEM_REDUCE */
-
-        else
+        else			/* EITEM_MAP and EITEM_REDUCE */
           {
+	  /* the expansion could modify $value, as a side-effect */
 	  uschar * t = expand_string_internal(expr,
 	    ESI_BRACE_ENDS | ESI_HONOR_DOLLAR | flags, NULL, &resetok, NULL);
-          temp = t;
-          if (!temp)
+          if (!(temp = t))
             {
             iterate_item = save_iterate_item;
             expand_string_message = string_sprintf("%s inside \"%s\" item",
diff --git a/test/scripts/0000-Basic/0002 b/test/scripts/0000-Basic/0002
index d169518b6..898f3b6bc 100644
--- a/test/scripts/0000-Basic/0002
+++ b/test/scripts/0000-Basic/0002
@@ -90,6 +90,8 @@ filter: ${filter{a:b:c}{!eq{$item}{b}}}
 filter: ${filter{<' a'b'c}{!eq{$item}{b}}}
 filter: ${filter{<' ''a'b' ''c}{!eq{$item}{b}}}
 filter: "${filter{}{!eq{$item}{b}}}"
+# check operation when the condition modifies the 'value' variable
+${filter {E} {inlisti{$item}{ e }}}
 
 map: "${map{}{$item}}"
 map: ${map{a:b:c}{$item}}
diff --git a/test/stdout/0002 b/test/stdout/0002
index 74219a6f3..9232089f6 100644
--- a/test/stdout/0002
+++ b/test/stdout/0002
@@ -79,6 +79,8 @@
 > filter: a'c
 > filter: ''a' ''c
 > filter: ""
+> # check operation when the condition modifies the 'value' variable
+> E
 > 
 > map: ""
 > map: a:b:c