VSB_quote has gotten messy

Dridi Boukelmoune dridi at varni.sh
Mon Apr 20 12:12:15 UTC 2020


On Mon, Apr 20, 2020 at 9:45 AM Poul-Henning Kamp <phk at phk.freebsd.dk> wrote:
>
> I finally got around to look at VSB_QUOTE_GLOB feature Guillaume committed
> by accident some time ago, and it doesn't work correctly as far as I
> can tell, for instance, the difference between inputs:
>         []
>     and
>         ["]
> makes no sense to me.
>
> However, I can hardly blame Guillaume, because it is not very
> consistent or clear how VSB_QUOTE is supposed to work in the first
> place, I just spent 4 hours trying to find out, because we sort of
> made it up as we went.

As discussed previously, it was also on my review queue, and still is.

> I propose that b0d1a40f326f... gets backed out before it has any
> use in the tree, and put an errata on the 6.4 release page to the
> effect of "do not use VSB_QUOTE_GLOB".

Agreed.

> I also propose that we should deprecate VSB_quote*() in its current
> form, ie: leave around for the benefit of VMODers for 7.x, remove
> in 8.x.

No opinion, but if we replace the old API with a new one, maybe we can
manage to translate old API calls to new ones?

> Finally, I propose a new and more well thought, and better documented
> replacement, VSB_encode(), to be added shortly.
>
> Comments ?

One annoying limitation with the current quote logic is that calls
must be self-contained. If I want to quote a string that will come in
multiple parts, I have to assemble it (eg. with a VSB!) prior to
feeding it to VSB_quote().

If we land a new API, it would be nice to be able to delimit begin and
end steps, either with flags or more functions:

AZ(VSB_encode_begin(vsb, VSB_ENCODE_JSON)); /* adds the leading quote */
/* encode a VCL_STRANDS in a loop */
AZ(VSB_encode_end(vsb); /* adds the trailing quote */

If we move struct strands to libvarnish, we can also have
VSB_encode_strands(), but being able to encode multiple string
components independently of struct strands would be appreciated.

Dridi


More information about the varnish-dev mailing list