[master] 11d55148a Put some serious red tape on VCL_STRANDS

[Dridi Boukelmoune]

commit 11d55148abcaa64e843df0dceca6c7371c0a018a
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Nov 19 18:09:27 2019 +0100

    Put some serious red tape on VCL_STRANDS
    
    I started suspecting that we need this clarification during the review
    of #3123 [1] and was able to verify it with a simple test case. First I
    needed a function I put in vmod_debug:
    
        $Function STRANDS hoard_strands(PRIV_TASK, STRANDS s)
    
        Return the first value of s for the rest of the task.
    
    The implementation is very straightforward:
    
        struct hoarder {
               VCL_STRANDS     s;
        };
    
        VCL_STRANDS
        xyzzy_hoard_strands(VRT_CTX, struct vmod_priv *priv, VCL_STRANDS s)
        {
               struct hoarder *h;
    
               CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
               AN(priv);
    
               if (priv->priv == NULL) {
                       h = malloc(sizeof *h);
                       AN(h);
                       h->s = s;
                       priv->priv = h;
                       priv->free = free;
               }
    
               return (priv->priv);
        }
    
    And then the following test case results in a panic on my system, but I
    suspect this is generally undefined behavior and other nasty results may
    occur under different circumstances:
    
        varnishtest "Beware of STRANDS"
    
        varnish v1 -vcl {
                import debug;
                backend be none;
                sub vcl_recv {
                        debug.hoard_strands("recv: " + req.url);
                }
                sub vcl_miss {
                        debug.hoard_strands("miss: " + req.url);
                        return (synth(200));
                }
                sub vcl_synth {
                        set resp.body = debug.hoard_strands("synth: " + req.url);
                        return (deliver);
                }
        } -start
    
        client c1 {
                txreq
                rxresp
                expect resp.body ~ recv
        } -run
    
    This also begs the following question: can it ever be safe to let a VMOD
    function return a STRANDS? Maybe it should be banned from return types.
    
    [1] https://github.com/varnishcache/varnish-cache/pull/3123#discussion_r345617108

diff --git a/include/vrt.h b/include/vrt.h
index 70a153bf3..863e60f65 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -180,6 +180,22 @@ struct vsl_log;
 struct ws;
 struct VSC_main;
 
+/*
+ * VCL_STRANDS:
+ *
+ * An argc+argv type of data structure where n indicates the number of strings
+ * in the p array. Individual components of a strands may be null.
+ *
+ * A STRANDS allows you to work on a strings concatenation with the option to
+ * collect it into a single STRING, or if possible work directly on individual
+ * parts.
+ *
+ * The memory management is very strict: a VMOD function receiving a STRANDS
+ * argument should keep no reference after the function returns. Retention of
+ * a STRANDS further in the ongoing task is undefined behavior and may result
+ * in a panic or data corruption.
+ */
+
 struct strands {
 	int		n;
 	const char	**p;