add error handling, and test it for decodes of illegal encodings

in the process, simplify identity decoding

src/id.c

@@ -27,6 +27,7 @@
  */
 
 #include <string.h>
+#include <errno.h>
 
 #include "vmod_convert.h"
 
@@ -54,35 +55,23 @@ ssize_t
 id_decode(const enum encoding enc, char *restrict const buf,
 	  const size_t maxlen, const char *restrict const p, va_list ap)
 {
-	const char *next;
 	char *dest = buf;
 	size_t len, outlen = 0;
 
 	(void) enc;
 	AN(buf);
 
-	if (p == vrt_magic_string_end)
-		return 0;
-
-	SKIP_EMPTY(next, ap);
-	if (next == vrt_magic_string_end && (p == NULL || *p == '\0'))
-		return 0;
-
-	if (p != NULL && *p != '\0') {
-		if ((len = strlen(p)) > maxlen)
-			return -1;
-		memcpy(buf, p, len);
-		outlen = len;
-		dest += len;
-	}
-
-	while (next != vrt_magic_string_end) {
-		len = strlen(next);
-		if ((outlen += len) > maxlen)
+	for (const char *s = p; s != vrt_magic_string_end;
+	     s = va_arg(ap, const char *)) {
+		if (s == NULL || *s == '\0')
+			continue;
+		len = strlen(s);
+		if ((outlen += len) > maxlen) {
+			errno = ENOMEM;
 			return -1;
-		memcpy(dest, next, len);
+		}
+		memcpy(dest, s, len);
 		dest += len;
-		SKIP_EMPTY(next, ap);
 	}
 
 	return outlen;

src/tests/base64.vtc

@@ -112,3 +112,44 @@ client c1 {
 }
 
 client c1 -run
+
+varnish v1 -vcl+backend {
+	import convert from "${vmod_topbuild}/src/.libs/libvmod_convert.so";
+
+	sub vcl_recv {
+	  return(synth(200));
+	}
+
+	sub vcl_synth {
+	  set req.http.foo = "AAA=";
+	  set resp.http.bad64 = convert.encode(IDENTITY,
+				convert.decode(BASE64, "-_-_" + req.http.foo));
+	  set resp.http.badurl = convert.encode(IDENTITY,
+				 convert.decode(BASE64URL,
+					        "+/+/" + req.http.foo));
+	  set resp.http.badpad = convert.encode(IDENTITY,
+				 convert.decode(BASE64URLNOPAD,
+					        "TWFu" + req.http.foo));
+	}
+}
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.bad64 == ""
+	expect resp.http.badurl == ""
+	expect resp.http.badpad == ""
+} -run
+
+logexpect l1 -v v1 -d 1 -g vxid -q "VCL_Error" {
+        expect 0 * Begin req
+        expect * = VCL_Error "^vmod convert error: cannot decode, illegal encoding beginning with \"-_-_\"$"
+#"
+        expect * = VCL_Error "^vmod convert error: cannot decode, illegal encoding beginning with \"\+/\+/\"$"
+#"
+        expect * = VCL_Error "^vmod convert error: cannot decode, illegal encoding beginning with \"TWFu\"$"
+#"
+	expect * = End
+} -start
+
+logexpect l1 -wait

src/tests/blob.vtc

@@ -54,3 +54,35 @@ client c1 {
 }
 
 client c1 -run
+
+varnish v1 -errvcl {vmod convert error: cannot create blob err, illegal encoding beginning with "g"} {
+	import convert from "${vmod_topbuild}/src/.libs/libvmod_convert.so";
+	backend b { .host="${bad_ip}"; }
+	sub vcl_init {
+	  new err = convert.blob(HEX, "g");
+	}
+}
+
+varnish v1 -errvcl {vmod convert error: cannot create blob bad64, illegal encoding beginning with "-_-_"} {
+	import convert from "${vmod_topbuild}/src/.libs/libvmod_convert.so";
+	backend b { .host="${bad_ip}"; }
+	sub vcl_init {
+	  new bad64 = convert.blob(BASE64, "-_-_");
+	}
+}
+
+varnish v1 -errvcl {vmod convert error: cannot create blob badurl, illegal encoding beginning with "+/+/"} {
+	import convert from "${vmod_topbuild}/src/.libs/libvmod_convert.so";
+	backend b { .host="${bad_ip}"; }
+	sub vcl_init {
+	  new badurl = convert.blob(BASE64URL, "+/+/");
+	}
+}
+
+varnish v1 -errvcl {vmod convert error: cannot create blob badpad, illegal encoding beginning with "YWI="} {
+	import convert from "${vmod_topbuild}/src/.libs/libvmod_convert.so";
+	backend b { .host="${bad_ip}"; }
+	sub vcl_init {
+	  new badpad = convert.blob(BASE64URLNOPAD, "YWI=");
+	}
+}

src/tests/hex.vtc

@@ -81,6 +81,33 @@ client c1 {
 	expect resp.http.decmanypieces == "foo bar baz quux"
 	expect resp.http.hexlcxcode == "2f48656c6c6f20776f726c642f"
 	expect resp.http.hexucxcode == "2F48656C6C6F20776F726C642F"
+} -run
+
+varnish v1 -vcl+backend {
+	import convert from "${vmod_topbuild}/src/.libs/libvmod_convert.so";
+
+	sub vcl_recv {
+	  return(synth(200));
+	}
+
+	sub vcl_synth {
+	  set req.http.foo = "123";
+	  set resp.http.badhex = convert.encode(HEX,
+				 convert.decode(HEX, "g" + req.http.foo));
+	}
 }
 
-client c1 -run
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.badhex == ""
+} -run
+
+logexpect l1 -v v1 -d 1 -g vxid -q "VCL_Error" {
+        expect 0 * Begin req
+        expect * = VCL_Error "^vmod convert error: cannot decode, illegal encoding beginning with \"g\"$"
+#"
+	expect * = End
+} -start
+
+logexpect l1 -wait

src/tests/xcode.vtc

@@ -15,11 +15,23 @@ varnish v1 -vcl+backend {
 	  set resp.http.id2id
 	    = convert.transcode(IDENTITY, IDENTITY, "Hello world");
 
-	  set resp.http.hex2hex
+	  set resp.http.hexuc2hexuc
 	    = convert.transcode(HEX, HEX, "0123456789ABCDEF");
 
-#	  set resp.http.hexlc2hexlc
-#	    = convert.transcode(HEXLC, HEXLC, "0123456789abcdef");
+	  set resp.http.hexlc2hexlc
+	    = convert.transcode(HEX, HEXLC, "0123456789abcdef");
+
+	  set resp.http.hexuc2hexlc
+	    = convert.transcode(HEX, HEXLC, "0123456789ABCDEF");
+
+	  set resp.http.hexlc2hexuc
+	    = convert.transcode(HEX, HEX, "0123456789abcdef");
+
+	  set resp.http.hexmix2hexuc
+	    = convert.transcode(HEX, HEX, "0123456789ABCdef");
+
+	  set resp.http.hexmix2hexlc
+	    = convert.transcode(HEX, HEXLC, "0123456789abcDEF");
 
 	  set resp.http.b642b64
 	    = convert.transcode(BASE64, BASE64,
@@ -35,6 +47,15 @@ varnish v1 -vcl+backend {
 	    convert.transcode(BASE64URLNOPAD, BASE64URLNOPAD,
 			      "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdef"
 			      + "ghijklmnopqrstuvwxyz0123456789-_");
+
+	  set resp.http.badhex = convert.transcode(HEX, IDENTITY, "0x123456");
+	  set req.http.foo = "AAA=";
+	  set resp.http.bad64 = convert.transcode(BASE64, IDENTITY,
+						  "-_-_" + req.http.foo);
+	  set resp.http.badurl = convert.transcode(BASE64URL, IDENTITY,
+						   "+/+/" + req.http.foo);
+	  set resp.http.badpad = convert.transcode(BASE64URLNOPAD, IDENTITY,
+						   "TWFu" + req.http.foo);
 	}
 } -start
 
@@ -42,11 +63,34 @@ client c1 {
 	txreq -url "/"
 	rxresp
 	expect resp.http.id2id == "Hello world"
-	expect resp.http.hex2hex == "0123456789ABCDEF"
-#	expect resp.http.hexlc2hexlc == "0123456789abcdef"
+	expect resp.http.hexuc2hexuc == "0123456789ABCDEF"
+	expect resp.http.hexlc2hexlc == "0123456789abcdef"
+	expect resp.http.hexuc2hexlc == "0123456789abcdef"
+#	expect resp.http.hexlc2hexuc == "0123456789ABCDEF"
+#	expect resp.http.hexmix2hexuc == "0123456789ABCDEF"
+	expect resp.http.hexmix2hexlc == "0123456789abcdef"
 	expect resp.http.b642b64 == "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"
 	expect resp.http.b64url2b64url == "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_"
 	expect resp.http.b64urlnopad2b64urlnopad == "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_"
+	expect resp.http.badhex == ""
+	expect resp.http.bad64 == ""
+	expect resp.http.badurl == ""
+	expect resp.http.badpad == ""
 }
 
 client c1 -run
+
+logexpect l1 -v v1 -d 1 -g vxid -q "VCL_Error" {
+        expect 0 * Begin req
+        expect * = VCL_Error "^vmod convert error: cannot decode, illegal encoding beginning with \"0x123456\"$"
+#"
+        expect * = VCL_Error "^vmod convert error: cannot decode, illegal encoding beginning with \"-_-_\"$"
+#"
+        expect * = VCL_Error "^vmod convert error: cannot decode, illegal encoding beginning with \"\+/\+/\"$"
+#"
+        expect * = VCL_Error "^vmod convert error: cannot decode, illegal encoding beginning with \"TWFu\"$"
+#"
+	expect * = End
+} -start
+
+logexpect l1 -wait

src/vmod_convert.c

@@ -26,6 +26,8 @@
  *
  */
 
+#include <errno.h>
+
 #include "vcl.h"
 #include "vrt.h"
 
@@ -39,6 +41,22 @@ struct vmod_convert_blob {
 	struct vmod_priv blob;
 };
 
+#define ERR(ctx, msg) \
+	errmsg((ctx), "vmod convert error: " msg)
+
+#define VERR(ctx, fmt, ...) \
+	errmsg((ctx), "vmod convert error: " fmt, __VA_ARGS__)
+
+#define ERRINVAL(ctx, enc) \
+	VERR((ctx), "cannot decode, illegal encoding beginning with \"%s\"", \
+	     (enc))
+
+#define VERRNOMEM(ctx, fmt, ...) \
+	VERR((ctx), fmt ", out of space", __VA_ARGS__)
+
+#define ERRNOMEM(ctx, msg) \
+	ERR((ctx), msg ", out of space")
+
 static const struct vmod_priv const null_blob[1] =
 {
 	{
@@ -119,6 +137,7 @@ static inline ssize_t __match_proto__(decode_f)
 decode(const enum encoding dec, char *restrict const buf,
        const size_t maxlen, const char *restrict const p, va_list ap)
 {
+	errno = 0;
 	switch(dec) {
 	case IDENTITY:
 		return id_decode(dec, buf, maxlen, p, ap);
@@ -152,6 +171,39 @@ encode(const enum encoding enc, char *restrict const buf, const size_t maxlen,
 	}
 }
 
+static void
+errmsg(VRT_CTX, const char *fmt, ...)
+{
+	va_list args;
+
+	va_start(args, fmt);
+	if (ctx->method == VCL_MET_INIT) {
+		AN(ctx->msg);
+		VSB_vprintf(ctx->msg, fmt, args);
+		VRT_handling(ctx, VCL_RET_FAIL);
+	}
+	else {
+		AN(ctx->vsl);
+		VSLbv(ctx->vsl, SLT_VCL_Error, fmt, args);
+	}
+	va_end(args);
+}
+
+static void
+err_decode(VRT_CTX, const char *enc)
+{
+	switch(errno) {
+	case EINVAL:
+		ERRINVAL(ctx, enc);
+		break;
+	case ENOMEM:
+		ERRNOMEM(ctx, "cannot decode");
+		break;
+	default:
+		WRONG("invalid errno");
+	}
+}
+
 /* init / event handler */
 int __match_proto__(vmod_event_f)
 event(VRT_CTX, struct vmod_priv *priv, enum vcl_event_e event)
@@ -196,16 +248,21 @@ vmod_blob__init(VRT_CTX, struct vmod_convert_blob **blobp, const char *vcl_name,
 	assert(len > 0);
 
 	b->blob.priv = malloc(len);
-	AN(b->blob.priv);
+	if (b->blob.priv == NULL) {
+		VERRNOMEM(ctx, "cannot create blob %s", vcl_name);
+		return;
+	}
+
 	va_start(ap, p);
 	len = decode(dec, b->blob.priv, len, p, ap);
 	va_end(ap);
 
 	if (len == -1) {
-		/* XXX: error msg in ctx->msg and fail */
+		assert(errno == EINVAL);
 		free(b->blob.priv);
-		FREE_OBJ(b);
-		*blobp = NULL;
+		b->blob.priv = NULL;
+		VERR(ctx, "cannot create blob %s, illegal encoding beginning "
+		    "with \"%s\"", vcl_name, p);
 		return;
 	}
 	if (len == 0) {
@@ -253,10 +310,13 @@ vmod_decode(VRT_CTX, VCL_ENUM decs, const char *p, ...) {
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 
 	snap = WS_Snapshot(ctx->ws);
-	if ((b = WS_Alloc(ctx->ws, sizeof(struct vmod_priv))) == NULL)
+	if ((b = WS_Alloc(ctx->ws, sizeof(struct vmod_priv))) == NULL) {
+		ERRNOMEM(ctx, "cannot decode");
 		return NULL;
+	}
 	if (wb_create(ctx->ws, &wb) == NULL) {
 		WS_Reset(ctx->ws, snap);
+		ERRNOMEM(ctx, "cannot decode");
 		return NULL;
 	}
 	buf = wb_buf(&wb);
@@ -266,6 +326,7 @@ vmod_decode(VRT_CTX, VCL_ENUM decs, const char *p, ...) {
 	va_end(ap);
 
 	if (len == -1) {
+		err_decode(ctx, p);
 		wb_reset(&wb);
 		WS_Reset(ctx->ws, snap);
 		return NULL;
@@ -293,12 +354,15 @@ vmod_encode(VRT_CTX, VCL_ENUM encs, VCL_BLOB b) {
 	if (b == NULL)
 		return NULL;
 
-	if (wb_create(ctx->ws, &wb) == NULL)
+	if (wb_create(ctx->ws, &wb) == NULL) {
+		ERRNOMEM(ctx, "cannot encode");
 		return NULL;
+	}
 
 	len = encode(enc, wb_buf(&wb), wb_space(&wb), b->priv, b->len);
 
 	if (len == -1) {
+		ERRNOMEM(ctx, "cannot encode");
 		wb_reset(&wb);
 		return NULL;
 	}
@@ -338,21 +402,26 @@ vmod_transcode(VRT_CTX, VCL_ENUM decs, VCL_ENUM encs, const char *p, ...) {
 	va_end(ap);
 	if (l == 0)
 		return "";
-	assert(l > 0);
+	/* XXX: handle stack overflow? */
 	char buf[l];
 
 	va_start(ap, p);
 	len = decode(dec, buf, l, p, ap);
 	va_end(ap);
 
-	if (len == -1)
+	if (len == -1) {
+		err_decode(ctx, p);
 		return NULL;
+	}
 
-	if (wb_create(ctx->ws, &wb) == NULL)
+	if (wb_create(ctx->ws, &wb) == NULL) {
+		ERRNOMEM(ctx, "cannot encode after decode");
 		return NULL;
+	}
 
 	len = encode(enc, wb_buf(&wb), wb_space(&wb), buf, len);
 	if (len == -1) {
+		ERRNOMEM(ctx, "cannot encode after decode");
 		wb_reset(&wb);
 		return NULL;
 	}