Sort out VSB quoting, add testcases and asserts on usage.

include/vsb.h

@@ -79,12 +79,52 @@ ssize_t		 VSB_len(const struct vsb *);
 void		 VSB_delete(struct vsb *) v_deprecated_;
 void		 VSB_fini(struct vsb *);
 void		 VSB_destroy(struct vsb **);
-#define VSB_QUOTE_NONL		1
+
+/*
+ * VSB_quote[_pfx] has four major modes, and two modifiers
+ */
+
+#define VSB_QUOTE_PLAIN		0
+	/*
+	 * Basic "show me the string" mode
+	 * All output is a single line
+	 */
 #define VSB_QUOTE_JSON		2
+	/*
+	 * Output suitable for inclusion between "..." in JSON
+	 * Uses JSON \u%04x quoting.
+	 * Anything above 0x7e had better be UTF-8
+	 */
 #define VSB_QUOTE_HEX		4
+	/*
+	 * Hex dump, single line.
+	 * All zero data is compressed to "0x0...0"
+	 */
 #define VSB_QUOTE_CSTR		8
+	/*
+	 * C lanuage source code literal string(s)
+	 * Breaks strings at \n (expecting string concatenation)
+	 */
 #define VSB_QUOTE_UNSAFE	16
+	/*
+	 * For general display applications
+	 * " and \ are not quoted
+	 * Splits output to new line at '\n'
+	 * Implies VSB_QUOTE_NONL
+	 */
+
+#define VSB_QUOTE_NONL		1
+	/*
+	 * If the output does not end in \n, append \n
+	 * Can be combined with all other modes.
+	 */
+
 #define VSB_QUOTE_ESCHEX	32
+	/*
+	 * Use \x%02x instead of \%03o
+	 * Not valid with VSB_QUOTE_JSON and VSB_QUOTE_HEX
+	 */
+
 void		 VSB_quote_pfx(struct vsb *, const char*, const void *,
 		     int len, int how);
 void		 VSB_quote(struct vsb *, const void *, int len, int how);

lib/libvarnish/vsb.c

@@ -555,73 +555,102 @@ VSB_destroy(struct vsb **s)
 /*
  * Quote a string
  */
+
+static void
+vsb_quote_hex(struct vsb *s, const uint8_t *u, size_t len)
+{
+	const uint8_t *w;
+
+	VSB_cat(s, "0x");
+	for (w = u; w < u + len; w++)
+		if (*w != 0x00)
+			break;
+	if (w == u + len && len > 4) {
+		VSB_cat(s, "0...0");
+	} else {
+		for (w = u; w < u + len; w++)
+			VSB_printf(s, "%02x", *w);
+	}
+}
+
 void
 VSB_quote_pfx(struct vsb *s, const char *pfx, const void *v, int len, int how)
 {
-	const char *p;
-	const char *q;
+	const uint8_t *p = v;
+	const uint8_t *q;
 	int quote = 0;
-	int nl = 0;
-	const unsigned char *u, *w;
+	int nl;
+
+	nl = how &
+	    (VSB_QUOTE_JSON|VSB_QUOTE_HEX|VSB_QUOTE_CSTR|VSB_QUOTE_UNSAFE);
+	AZ(nl & (nl - 1)); // Only one bit can be set
+
+	if (how & VSB_QUOTE_ESCHEX)
+		AZ(how & (VSB_QUOTE_JSON|VSB_QUOTE_HEX));
+
+	if (how & VSB_QUOTE_UNSAFE)
+		how |= VSB_QUOTE_NONL;
 
-	assert(v != NULL);
+	assert(p != NULL);
 	if (len == -1)
 		len = strlen(v);
 
 	if (len == 0 && (how & VSB_QUOTE_CSTR)) {
 		VSB_printf(s, "%s\"\"", pfx);
-		return;
-	} else if (len == 0)
+		if ((how & VSB_QUOTE_NONL))
+			VSB_putc(s, '\n');
+	}
+
+	if (len == 0)
 		return;
 
 	VSB_cat(s, pfx);
 
 	if (how & VSB_QUOTE_HEX) {
-		u = v;
-		for (w = u; w < u + len; w++)
-			if (*w != 0x00)
-				break;
-		VSB_cat(s, "0x");
-		if (w == u + len && len > 4) {
-			VSB_cat(s, "0...0");
-		} else {
-			for (w = u; w < u + len; w++)
-				VSB_printf(s, "%02x", *w);
-		}
+		vsb_quote_hex(s, v, len);
+		if (how & VSB_QUOTE_NONL)
+			VSB_putc(s, '\n');
 		return;
 	}
-	p = v;
+
+	if (how & VSB_QUOTE_CSTR)
+		VSB_putc(s, '"');
 
 	for (q = p; q < p + len; q++) {
-		if (!isgraph(*q) || *q == '"' || *q == '\\') {
+		if (
+		    *q < 0x20 ||
+		    *q == '"' ||
+		    *q == '\\' ||
+		    (*q == '?' && (how & VSB_QUOTE_CSTR)) ||
+		    (*q > 0x7e && !(how & VSB_QUOTE_JSON))
+		) {
 			quote++;
 			break;
 		}
 	}
-	if (!quote && !(how & (VSB_QUOTE_JSON|VSB_QUOTE_CSTR))) {
-		(void)VSB_bcat(s, p, len);
-		if ((how & (VSB_QUOTE_UNSAFE|VSB_QUOTE_NONL)) &&
+
+	if (!quote) {
+		VSB_bcat(s, p, len);
+		if ((how & VSB_QUOTE_NONL) &&
 		    p[len-1] != '\n')
 			(void)VSB_putc(s, '\n');
+		if (how & VSB_QUOTE_CSTR)
+			VSB_putc(s, '"');
 		return;
 	}
 
-	if (how & VSB_QUOTE_CSTR)
-		(void)VSB_putc(s, '"');
-
+	nl = 0;
 	for (q = p; q < p + len; q++) {
 		if (nl)
 			VSB_cat(s, pfx);
 		nl = 0;
 		switch (*q) {
 		case '?':
-			if (how & VSB_QUOTE_CSTR)
+			/* Avoid C Trigraph insanity */
+			if (how & VSB_QUOTE_CSTR && !(how & VSB_QUOTE_JSON))
 				(void)VSB_putc(s, '\\');
 			(void)VSB_putc(s, *q);
 			break;
-		case ' ':
-			(void)VSB_putc(s, *q);
-			break;
 		case '\\':
 		case '"':
 			if (!(how & VSB_QUOTE_UNSAFE))
@@ -630,38 +659,43 @@ VSB_quote_pfx(struct vsb *s, const char *pfx, const void *v, int len, int how)
 			break;
 		case '\n':
 			if (how & VSB_QUOTE_CSTR) {
-				(void)VSB_printf(s, "\\n\"\n%s\"", pfx);
-			} else if (how & (VSB_QUOTE_NONL|VSB_QUOTE_UNSAFE)) {
-				(void)VSB_printf(s, "\n");
+				VSB_printf(s, "\\n\"\n%s\"", pfx);
+			} else if (how & VSB_QUOTE_JSON) {
+				VSB_printf(s, "\\n");
+			} else if (how & VSB_QUOTE_NONL) {
+				VSB_putc(s, *q);
 				nl = 1;
 			} else {
-				(void)VSB_printf(s, "\\n");
+				VSB_printf(s, "\\n");
 			}
 			break;
 		case '\r':
-			(void)VSB_cat(s, "\\r");
+			VSB_cat(s, "\\r");
 			break;
 		case '\t':
-			(void)VSB_cat(s, "\\t");
+			VSB_cat(s, "\\t");
 			break;
 		case '\v':
-			(void)VSB_cat(s, "\\v");
+			VSB_cat(s, "\\v");
 			break;
 		default:
-			/* XXX: Implement VSB_QUOTE_JSON */
-			if (isgraph(*q))
-				(void)VSB_putc(s, *q);
+			if (0x20 <= *q && *q <= 0x7e)
+				VSB_putc(s, *q);
+			else if (*q > 0x7e && (how & VSB_QUOTE_JSON))
+				VSB_putc(s, *q);
+			else if (how & VSB_QUOTE_JSON)
+				VSB_printf(s, "\\u%04x", *q);
 			else if (how & VSB_QUOTE_ESCHEX)
-				(void)VSB_printf(s, "\\x%02x", *q & 0xff);
+				VSB_printf(s, "\\x%02x", *q);
 			else
-				(void)VSB_printf(s, "\\%03o", *q & 0xff);
+				VSB_printf(s, "\\%03o", *q);
 			break;
 		}
 	}
 	if (how & VSB_QUOTE_CSTR)
-		(void)VSB_putc(s, '"');
-	if ((how & (VSB_QUOTE_NONL|VSB_QUOTE_UNSAFE)) && !nl)
-		(void)VSB_putc(s, '\n');
+		VSB_putc(s, '"');
+	if ((how & VSB_QUOTE_NONL) && !nl)
+		VSB_putc(s, '\n');
 }
 
 void

lib/libvarnish/vsb_test.c

@@ -10,13 +10,79 @@
 
 struct tc {
 	int		how;
+	int		inlen;
 	const char	*in;
 	const char	*out;
 };
 
 static struct tc tcs[] = {
 	{
-	    0, NULL, NULL
+		VSB_QUOTE_HEX,
+		5, "\x00\n\x7e\x7f\xff",
+		"PFX0x000a7e7fff"
+	},
+	{
+		VSB_QUOTE_HEX,
+		5, "\0\0\0\0\0",
+		"PFX0x0...0"
+	},
+	{
+		VSB_QUOTE_HEX | VSB_QUOTE_NONL,
+		5, "\x00\n\x7e\x7f\xff",
+		"PFX0x000a7e7fff\n"
+	},
+	{
+		VSB_QUOTE_ESCHEX,
+		5, "\x00\n\x7e\x7f\xff",
+		"PFX\\x00\\n~\\x7f\\xff",
+	},
+	{
+		0,
+		5, "\x00\n\x7e\x7f\xff",
+		"PFX\\000\\n~\\177\\377",
+	},
+	{
+		VSB_QUOTE_UNSAFE,
+		5, "\x00\n\x7e\x7f\xff",
+		"PFX\\000\nPFX~\\177\\377\n",
+	},
+	{
+		VSB_QUOTE_UNSAFE,
+		-1, "\n\"\\\t",
+		"PFX\nPFX\"\\\\t\n"
+	},
+	{
+		VSB_QUOTE_CSTR | VSB_QUOTE_ESCHEX,
+		5, "\x00\n\x7e\x7f\xff",
+		"PFX\"\\x00\\n\"\nPFX\"~\\x7f\\xff\"",
+	},
+	{
+		VSB_QUOTE_JSON,
+		5, "\x00\n\x7e\x7f\xff",
+		"PFX\\u0000\\n~\x7f\xff",
+	},
+	{
+		VSB_QUOTE_JSON | VSB_QUOTE_NONL,
+		5, "\x00\n\x7e\x7f\xff",
+		"PFX\\u0000\\n~\x7f\xff\n",
+	},
+	{
+		VSB_QUOTE_CSTR,
+		-1, "",
+		"PFX\"\""
+	},
+	{
+		VSB_QUOTE_CSTR,
+		-1, "?",
+		"PFX\"\\?\""
+	},
+	{
+		VSB_QUOTE_NONL,
+		-1, "\n\t",
+		"PFX\nPFX\\t\n"
+	},
+	{
+		0, -1, NULL, NULL
 	}
 };
 
@@ -26,25 +92,58 @@ main(int argc, char *argv[])
 	int err = 0;
 	struct tc *tc;
 	struct vsb *vsb;
+	struct vsb *vsbo;
 
 	(void)argc;
 	(void)argv;
 	vsb = VSB_new_auto();
 	AN(vsb);
+	vsbo = VSB_new_auto();
+	AN(vsbo);
 
 	for (tc = tcs; tc->in; tc++) {
-		VSB_quote(vsb, tc->in, -1, tc->how);
+		VSB_quote_pfx(vsb, "PFX", tc->in, tc->inlen, tc->how);
 		assert(VSB_finish(vsb) == 0);
 
-		printf("%s -> %s", tc->in, VSB_data(vsb));
+		VSB_clear(vsbo);
+		VSB_printf(vsbo, "0x%02x: ", tc->how);
+		VSB_quote(vsbo, tc->in, tc->inlen, VSB_QUOTE_HEX);
+		VSB_printf(vsbo, " -> ");
+		VSB_quote(vsbo, VSB_data(vsb), -1, VSB_QUOTE_HEX);
+		VSB_printf(vsbo, " (");
+		VSB_quote(vsbo, tc->out, -1, VSB_QUOTE_ESCHEX);
+		VSB_printf(vsbo, ")");
 		if (strcmp(VSB_data(vsb), tc->out)) {
-			printf(", but should have been %s",  tc->out);
+			VSB_printf(vsbo, "\nShould have been:\n\t");
+			VSB_quote(vsbo, tc->out, -1, VSB_QUOTE_HEX);
+			VSB_printf(vsbo, "\nThat's:\n\t");
+			VSB_quote(vsbo, VSB_data(vsb), -1, VSB_QUOTE_ESCHEX);
+			VSB_printf(vsbo, "\nvs:\n\t");
+			VSB_quote(vsbo, tc->out, -1, VSB_QUOTE_ESCHEX);
+			VSB_printf(vsbo, "\nFlags 0x%02x = ", tc->how);
+			if (!tc->how)
+				VSB_printf(vsbo, "\n\t0");
+			if (tc->how & VSB_QUOTE_NONL)
+				VSB_printf(vsbo, "\n\tVSB_QUOTE_NONL");
+			if (tc->how & VSB_QUOTE_JSON)
+				VSB_printf(vsbo, "\n\tVSB_QUOTE_JSON");
+			if (tc->how & VSB_QUOTE_HEX)
+				VSB_printf(vsbo, "\n\tVSB_QUOTE_HEX");
+			if (tc->how & VSB_QUOTE_CSTR)
+				VSB_printf(vsbo, "\n\tVSB_QUOTE_CSTR");
+			if (tc->how & VSB_QUOTE_UNSAFE)
+				VSB_printf(vsbo, "\n\tVSB_QUOTE_UNSAFE");
+			if (tc->how & VSB_QUOTE_ESCHEX)
+				VSB_printf(vsbo, "\n\tVSB_QUOTE_ESCHEX");
+			VSB_printf(vsbo, "\n\n");
 			err = 1;
 		}
-		printf("\n");
+		AZ(VSB_finish(vsbo));
+		printf("%s\n", VSB_data(vsbo));
 		VSB_clear(vsb);
 	}
 	VSB_destroy(&vsb);
+	VSB_destroy(&vsbo);
 	printf("error is %i\n", err);
 	return (err);
 }