Merge pull request #5536 from larsewi/expand

ENT-9491: Fixed bug in double expansion of foreign list variables

libpromises/iteration.c

@@ -310,12 +310,10 @@ static bool IsMangled(const char *s)
  * whichever comes first.
  *
  * "this" scope is never mangled, no need to VariablePut() a mangled reference
- * in THIS scope, since the non-manled one already exists.
+ * in THIS scope, since the non-mangled one already exists.
  */
 static void MangleVarRefString(char *ref_str, size_t len)
 {
-    //    printf("MangleVarRefString: %.*s\n", (int) len, ref_str);
-
     size_t dollar_paren = FindDollarParen(ref_str, len);
     size_t upto         = MIN(len, dollar_paren);
     char *bracket       = memchr(ref_str, '[', upto);
@@ -799,6 +797,11 @@ static void ExpandAndPutWheelVariablesAfter(
     EvalContext *evalctx,
     size_t wheel_idx)
 {
+    assert(iterctx != NULL);
+    assert(iterctx->wheels != NULL);
+    assert(iterctx->pp != NULL);
+    assert(iterctx->pp->promiser != NULL);
+
     /* Buffer to store the expanded wheel variable name, for each wheel. */
     Buffer *tmpbuf = BufferNew();
 
@@ -859,6 +862,56 @@ static void ExpandAndPutWheelVariablesAfter(
                     assert(SeqLength(wheel->values)     > 0);
                     assert(    SeqAt(wheel->values, 0) != NULL);
 
+                    /* During initialization of the iteration engine, lists from
+                     * foreign bundles are mangled in order to avoid overwriting
+                     * the values in that bundle. However, the iteration engine
+                     * has no way of knowing that some variables like
+                     * $($(parent_bundle).lst) is a list during initialization.
+                     * Hence, while crunching the wheels, this hack makes sure
+                     * foreign variables are mangled when ever they expand into
+                     * a list.
+                     *
+                     * How does it work? It looks for a '.' in both
+                     * #iterctx->pp->promiser and #varname, where the left side
+                     * of #varname is a bundle name and the right side of
+                     * #iterctx->pp->promiser begins with the right side of
+                     * #varname. If all of the constraints are fulfilled it
+                     * swaps the '.' with '#' in both variables.
+                     *
+                     * See ticket ENT-9491 for more info.
+                     */
+                    if (!IsMangled(varname))
+                    {
+                        const StringSet *scopes = EvalContextGetBundleNames(evalctx);
+
+                        bool done = false;
+                        char *unexp_pos = iterctx->pp->promiser;
+                        while (!done && (unexp_pos = strchr(unexp_pos, '.')) != NULL)
+                        {
+                            char *exp_pos = (char *)varname;
+                            while (!done && (exp_pos = strchr(exp_pos, '.')) != NULL)
+                            {
+                                if (StringStartsWith(unexp_pos + 1, exp_pos + 1))
+                                {
+                                    char tmp = *exp_pos;
+                                    *exp_pos = '\0';
+                                    if (StringSetContains(scopes, varname))
+                                    {
+                                        *exp_pos = CF_MANGLED_SCOPE;
+                                        *unexp_pos = CF_MANGLED_SCOPE;
+                                        done = true;
+                                    }
+                                    else
+                                    {
+                                        *exp_pos = tmp;
+                                    }
+                                }
+                                exp_pos += 1;
+                            }
+                            unexp_pos += 1;
+                        }
+                    }
+
                     /* Put the first value of the iterable. */
                     IterListElementVariablePut(evalctx, varname, value_type,
                                                SeqAt(wheel->values, 0));
@@ -1041,7 +1094,7 @@ bool PromiseIteratorNext(PromiseIterator *iterctx, EvalContext *evalctx)
                  " count=%zu wheels_num=%zu current_wheel=%zd",
                  iterctx->count, wheels_num, (ssize_t) i);
 
-    /* TODO if not done, then we are re-Put()ing variables in the EvalContect,
+    /* TODO if not done, then we are re-Put()ing variables in the EvalContext,
      *      hopefully overwriting the previous values, but possibly not! */
     }
 

tests/acceptance/01_vars/01_basic/double_expansion_list.cf

@@ -0,0 +1,68 @@
+##############################################################################
+#
+# Test double expansion of list from remote bundle (ENT-9491)
+#
+##############################################################################
+
+body common control
+{
+  bundlesequence => { "check" };
+}
+
+bundle agent test(parent_bundle)
+{
+  meta:
+    "description" -> { "ENT-9491", "CFE-1644" }
+      string => "Test double expension of list from remote bundle";
+
+  reports:
+    "$($(parent_bundle).str)"
+      comment => "Double expansion of remote string works prior to fix",
+      bundle_return_value_index => "foo";
+
+    "$($(parent_bundle).lst)"
+      comment => "But double expansion of remote list does not work prior to fix",
+      bundle_return_value_index => "bar";
+
+    "$(check.lst)"
+      comment => "Single expansion of remote list works prior to fix",
+      bundle_return_value_index => "baz";
+
+    "$($(parent_bundle)#lst)"
+      comment => "We force mangle the variable, it works, but should this be possible?",
+      bundle_return_value_index => "qux";
+
+}
+
+bundle agent check
+{
+  vars:
+    "str"
+      string => "EXPANDED";
+    "lst"
+      slist => { "EXPANDED" };
+
+  methods:
+    "holder"
+      usebundle => test("$(this.bundle)"),
+      useresult => "ret";
+
+  reports:
+      "$(this.promise_filename) Pass"
+        if => and(strcmp("$(ret[foo])", "EXPANDED"),
+                  strcmp("$(ret[bar])", "EXPANDED"),
+                  strcmp("$(ret[baz])", "EXPANDED"),
+                  strcmp("$(ret[qux])", "EXPANDED"));
+
+      "$(this.promise_filename) FAIL"
+        unless => and(strcmp("$(ret[foo])", "EXPANDED"),
+                      strcmp("$(ret[bar])", "EXPANDED"),
+                      strcmp("$(ret[qux])", "EXPANDED"),
+                      strcmp("$(ret[baz])", "EXPANDED"));
+
+    DEBUG::
+      "$(const.dollar)($(const.dollar)(parent_bundle).str) => $(ret[foo])";
+      "$(const.dollar)($(const.dollar)(parent_bundle).lst) => $(ret[bar])";
+      "$(const.dollar)(check.lst)            => $(ret[baz])";
+      "$(const.dollar)($(const.dollar)(parent_bundle)#lst) => $(ret[qux])";
+}