[master] 6eee0074f add std.ban() and std.ban_error() for vcl access to ban errors
Nils Goroll
nils.goroll at uplex.de
Wed Jan 6 09:03:07 UTC 2021
commit 6eee0074ffd3e3b87c994bcec8a9ec28993000c9
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Mon Dec 21 16:29:07 2020 +0100
add std.ban() and std.ban_error() for vcl access to ban errors
The ban() vcl action adds bans like the ban CLI command, but has no
facility for in-band error reporting. Errors are only reported to VSL.
We add std.ban() as a replacement for ban() with identical semantics,
but returning if the ban was successfully added.
std.ban_error() is added to report the error, if any.
We add v00009.vtc mirroring v00011.vtc, but with std.ban() replacing
ban(). The test number was chosen to fill a gap close to v00011.vtc.
Implementation:
We change VRT_ban_string() to return any error or NULL for
success. Where the error is not a constant string, we need to format
it on the workspace. For workspace overflows, we need to fall back to
canned constant error strings to ensure that the error case never
appears as success.
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 14a10c9a3..072f1e745 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -829,7 +829,7 @@ VRT_synth_page(VRT_CTX, VCL_STRANDS s)
/*--------------------------------------------------------------------*/
-static VCL_VOID
+static VCL_STRING
vrt_ban_error(VRT_CTX, VCL_STRING err)
{
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
@@ -837,77 +837,83 @@ vrt_ban_error(VRT_CTX, VCL_STRING err)
AN(err);
VSLb(ctx->vsl, SLT_VCL_Error, "ban(): %s", err);
+ return (err);
}
-VCL_VOID
+VCL_STRING
VRT_ban_string(VRT_CTX, VCL_STRING str)
{
char *a1, *a2, *a3;
char **av;
struct ban_proto *bp;
- const char *err;
+ const char *err = NULL, *berr = NULL;
int i;
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
- if (str == NULL) {
- vrt_ban_error(ctx, "Null argument");
- return;
- }
+ if (str == NULL)
+ return (vrt_ban_error(ctx, "Null argument"));
bp = BAN_Build();
- if (bp == NULL) {
- vrt_ban_error(ctx, "Out of Memory");
- return;
- }
+ if (bp == NULL)
+ return (vrt_ban_error(ctx, "Out of Memory"));
+
av = VAV_Parse(str, NULL, ARGV_NOESC);
AN(av);
if (av[0] != NULL) {
- vrt_ban_error(ctx, av[0]);
+ err = av[0];
VAV_Free(av);
BAN_Abandon(bp);
- return;
+ return (vrt_ban_error(ctx, err));
}
for (i = 0; ;) {
a1 = av[++i];
if (a1 == NULL) {
- vrt_ban_error(ctx, "No ban conditions found.");
+ err = "No ban conditions found.";
break;
}
a2 = av[++i];
if (a2 == NULL) {
- vrt_ban_error(ctx, "Expected comparison operator.");
+ err = "Expected comparison operator.";
break;
}
a3 = av[++i];
if (a3 == NULL) {
- vrt_ban_error(ctx, "Expected second operand.");
+ err = "Expected second operand.";
break;
}
- err = BAN_AddTest(bp, a1, a2, a3);
- if (err != NULL) {
- vrt_ban_error(ctx, err);
+ berr = BAN_AddTest(bp, a1, a2, a3);
+ if (berr != NULL)
break;
- }
+
if (av[++i] == NULL) {
- err = BAN_Commit(bp);
- if (err == NULL)
+ berr = BAN_Commit(bp);
+ if (berr == NULL)
bp = NULL;
- else
- vrt_ban_error(ctx, err);
break;
}
if (strcmp(av[i], "&&")) {
- // XXX refactoring pending via PR
- VSLb(ctx->vsl, SLT_VCL_Error,
- "ban(): Expected && between conditions,"
- " found \"%s\"", av[i]);
+ err = WS_Printf(ctx->ws, "Expected && between "
+ "conditions, found \"%s\"", av[i]);
+ if (err == NULL)
+ err = "Expected && between conditions "
+ "(workspace overflow)";
break;
}
}
+ if (berr != NULL) {
+ AZ(err);
+ err = WS_Copy(ctx->ws, berr, -1);
+ if (err == NULL)
+ err = "Unknown error (workspace overflow)";
+ berr = NULL;
+ }
if (bp != NULL)
BAN_Abandon(bp);
VAV_Free(av);
+ if (err == NULL)
+ return (NULL);
+ return (vrt_ban_error(ctx, err));
}
VCL_BYTES
diff --git a/bin/varnishtest/tests/v00009.vtc b/bin/varnishtest/tests/v00009.vtc
new file mode 100644
index 000000000..c08ff1dec
--- /dev/null
+++ b/bin/varnishtest/tests/v00009.vtc
@@ -0,0 +1,104 @@
+varnishtest "Test std.ban()"
+
+# see also v00011.vtc
+
+server s1 {
+ rxreq
+ txresp -body "foo"
+
+ rxreq
+ txresp -body "foo"
+} -start
+
+varnish v1 -vcl+backend {
+ import std;
+
+ sub vcl_synth {
+ set resp.http.b1 = std.ban(req.http.doesntexist);
+ set resp.http.b1e = std.ban_error();
+ set resp.http.b2 = std.ban("");
+ set resp.http.b2e = std.ban_error();
+ set resp.http.b3 = std.ban("req.url");
+ set resp.http.b3e = std.ban_error();
+ set resp.http.b4 = std.ban("req.url //");
+ set resp.http.b4e = std.ban_error();
+ set resp.http.b5 = std.ban("req.url // bar");
+ set resp.http.b5e = std.ban_error();
+ set resp.http.b6 = std.ban("req.url == bar //");
+ set resp.http.b6e = std.ban_error();
+ set resp.http.b7 = std.ban("foo == bar //");
+ set resp.http.b7e = std.ban_error();
+ set resp.http.b8 = std.ban("obj.age == 4");
+ set resp.http.b8e = std.ban_error();
+ set resp.http.b9 = std.ban("obj.age // 4d");
+ set resp.http.b9e = std.ban_error();
+ set resp.http.b10 = std.ban("obj.http.foo > 4d");
+ set resp.http.b10e = std.ban_error();
+ set resp.http.b11 = std.ban("req.url ~ ^/$");
+ set resp.http.b11e = std.ban_error();
+ }
+ sub vcl_recv {
+ if (req.method == "BAN") {
+ return (synth(209,"foo"));
+ }
+ }
+
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.http.X-Varnish == "1001"
+} -run
+
+logexpect l1 -v v1 -d 1 -g vxid {
+ expect * 1004 VCL_Error {ban[(][)]: Null argument}
+ expect * 1004 VCL_Error {ban[(][)]: No ban conditions found[.]}
+ expect * 1004 VCL_Error {ban[(][)]: Expected comparison operator[.]}
+ expect * 1004 VCL_Error {ban[(][)]: Expected second operand[.]}
+ expect * 1004 VCL_Error {ban[(][)]: expected conditional [(]==, !=, ~ or !~[)] got "//"}
+ expect * 1004 VCL_Error {ban[(][)]: Expected && between conditions, found "//"}
+ expect * 1004 VCL_Error {ban[(][)]: Unknown or unsupported field "foo"}
+ expect * 1004 VCL_Error {ban[(][)]: expected duration <n.nn>.ms|s|m|h|d|w|y. got "4"}
+ expect * 1004 VCL_Error {ban[(][)]: expected conditional [(]==, !=, >, >=, < or <=[)] got "//"}
+ expect * 1004 VCL_Error {ban[(][)]: expected conditional [(]==, !=, ~ or !~[)] got ">"}
+
+
+} -start
+
+client c1 {
+ txreq -req "BAN"
+ rxresp
+ expect resp.http.X-Varnish == "1004"
+ expect resp.status == 209
+ expect resp.http.b1 == false
+ expect resp.http.b1e == {Null argument}
+ expect resp.http.b2 == false
+ expect resp.http.b2e == {No ban conditions found.}
+ expect resp.http.b3 == false
+ expect resp.http.b3e == {Expected comparison operator.}
+ expect resp.http.b4 == false
+ expect resp.http.b4e == {Expected second operand.}
+ expect resp.http.b5 == false
+ expect resp.http.b5e == {expected conditional (==, !=, ~ or !~) got "//"}
+ expect resp.http.b6 == false
+ expect resp.http.b6e == {Expected && between conditions, found "//"}
+ expect resp.http.b7 == false
+ expect resp.http.b7e == {Unknown or unsupported field "foo"}
+ expect resp.http.b8 == false
+ expect resp.http.b8e == {expected duration <n.nn>[ms|s|m|h|d|w|y] got "4"}
+ expect resp.http.b9 == false
+ expect resp.http.b9e == {expected conditional (==, !=, >, >=, < or <=) got "//"}
+ expect resp.http.b10 == false
+ expect resp.http.b10e == {expected conditional (==, !=, ~ or !~) got ">"}
+ expect resp.http.b11 == true
+ expect resp.http.b11e == {}
+} -run
+
+logexpect l1 -wait
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.http.X-Varnish == "1006"
+} -run
diff --git a/bin/varnishtest/tests/v00011.vtc b/bin/varnishtest/tests/v00011.vtc
index 8a613be33..6e75b88da 100644
--- a/bin/varnishtest/tests/v00011.vtc
+++ b/bin/varnishtest/tests/v00011.vtc
@@ -1,5 +1,7 @@
varnishtest "Test vcl ban()"
+# see also v00009.vtc
+
server s1 {
rxreq
txresp -body "foo"
diff --git a/include/vrt.h b/include/vrt.h
index 8f17e2014..96bd70df6 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -58,6 +58,7 @@
* struct vmod_priv_methods added
* struct vmod_priv free member replaced with methods
* VRT_CTX_Assert() added
+ * VRT_ban_string() signature changed
* 12.0 (2020-09-15)
* Added VRT_DirectorResolve()
* Added VCL_STRING VRT_BLOB_string(VRT_CTX, VCL_BLOB)
@@ -494,7 +495,7 @@ VCL_BYTES VRT_CacheReqBody(VRT_CTX, VCL_BYTES maxsize);
/* Regexp related */
const char *VRT_regsub(VRT_CTX, int all, const char *, void *, const char *);
-VCL_VOID VRT_ban_string(VRT_CTX, VCL_STRING);
+VCL_STRING VRT_ban_string(VRT_CTX, VCL_STRING);
VCL_INT VRT_purge(VRT_CTX, VCL_DURATION, VCL_DURATION, VCL_DURATION);
VCL_VOID VRT_synth(VRT_CTX, VCL_INT, VCL_STRING);
VCL_VOID VRT_hit_for_pass(VRT_CTX, VCL_DURATION);
diff --git a/lib/libvcc/vcc_action.c b/lib/libvcc/vcc_action.c
index 1ed104af5..eed78708a 100644
--- a/lib/libvcc/vcc_action.c
+++ b/lib/libvcc/vcc_action.c
@@ -187,7 +187,7 @@ vcc_act_ban(struct vcc *tl, struct token *t, struct symbol *sym)
ExpectErr(tl, '(');
vcc_NextToken(tl);
- Fb(tl, 1, "VRT_ban_string(ctx, \n");
+ Fb(tl, 1, "(void) VRT_ban_string(ctx, \n");
tl->indent += INDENT;
vcc_Expr(tl, STRING);
tl->indent -= INDENT;
diff --git a/lib/libvmod_std/vmod_std.c b/lib/libvmod_std/vmod_std.c
index 57b12c6a6..203a621b1 100644
--- a/lib/libvmod_std/vmod_std.c
+++ b/lib/libvmod_std/vmod_std.c
@@ -311,3 +311,51 @@ vmod_fnmatch(VRT_CTX, VCL_STRING pattern, VCL_STRING subject,
flags |= FNM_PERIOD;
return (fnmatch(pattern, subject, flags) != FNM_NOMATCH);
}
+
+static const void * const priv_task_id_ban = &priv_task_id_ban;
+
+VCL_BOOL v_matchproto_(td_std_ban)
+vmod_ban(VRT_CTX, VCL_STRING s)
+{
+ struct vmod_priv *priv_task;
+ VCL_STRING r;
+
+ CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+ priv_task = VRT_priv_task(ctx, priv_task_id_ban);
+ if (priv_task == NULL) {
+ VRT_fail(ctx, "std.ban(): no priv_task (out of workspace?)");
+ return (0);
+ }
+
+ r = VRT_ban_string(ctx, s);
+
+ /*
+ * TRUST_ME: the ban error is const. We save it in the un-const priv
+ * pointer, but promise to only ever return it as a (const) VCL_STRING
+ */
+ priv_task->priv = TRUST_ME(r);
+
+ return (r == NULL);
+}
+
+VCL_STRING v_matchproto_(td_std_ban_error)
+vmod_ban_error(VRT_CTX)
+{
+ struct vmod_priv *priv_task;
+ VCL_STRING r;
+
+ CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+ priv_task = VRT_priv_task(ctx, priv_task_id_ban);
+ if (priv_task == NULL) {
+ VRT_fail(ctx, "std.ban_error():"
+ " no priv_task (out of workspace?)");
+ return ("no priv_task");
+ }
+
+ r = priv_task->priv;
+ if (r == NULL)
+ r = "";
+ return (r);
+}
diff --git a/lib/libvmod_std/vmod_std.vcc b/lib/libvmod_std/vmod_std.vcc
index d0aea5707..e0a17f019 100644
--- a/lib/libvmod_std/vmod_std.vcc
+++ b/lib/libvmod_std/vmod_std.vcc
@@ -561,6 +561,16 @@ Example::
std.rollback(bereq);
+$Function BOOL ban(STRING)
+
+Same as :ref:`vcl(7)_ban`, but returns true if the ban succeeded and
+false otherwise.
+
+$Function STRING ban_error()
+
+Returns a textual error description of the last `std.ban()`_ call from
+the same task or the empty string if there either was no error or no
+`std.ban()`_ call.
DEPRECATED functions
====================
@@ -630,8 +640,6 @@ Example::
set req.http.real = std.time2real(now, 1.0);
-
-
SEE ALSO
========
More information about the varnish-commit
mailing list