On Mon, Apr 20, 2020 at 9:45 AM Poul-Henning Kamp <p...@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 _______________________________________________ varnish-dev mailing list varnish-dev@varnish-cache.org https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
