[master] bfb95f831 Deprecate ban(), update documentation for std.ban()

Nils Goroll nils.goroll at uplex.de
Wed Jan 6 09:52:07 UTC 2021


commit bfb95f831a99835d65fa239196ed3371ee22ddef
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Wed Jan 6 10:44:43 2021 +0100

    Deprecate ban(), update documentation for std.ban()
    
    Ref
    https://github.com/varnishcache/varnish-cache/pull/3486#issuecomment-754019262
    
    roadmap:
    
     v/ add std.ban*(),
        add implicit import std
        vcc aliases ban() to std.ban() (some concerns raised)
     v/ deprecated ban() without std.
        remove ban() without std.

diff --git a/bin/varnishtest/tests/c00022.vtc b/bin/varnishtest/tests/c00022.vtc
index 42383a63a..2ec23c2aa 100644
--- a/bin/varnishtest/tests/c00022.vtc
+++ b/bin/varnishtest/tests/c00022.vtc
@@ -18,15 +18,27 @@ server s1 {
 	txresp -hdr "foo: bar8" -body "1111111\n"
 } -start
 
+# code from purging.rst
 varnish v1 -vcl+backend {
+	import std;
+
 	sub vcl_recv {
 		if (req.method == "BAN") {
-			ban ("req.url == " + req.url);
-			return (synth(410));
+			if (std.ban("req.http.host == " + req.http.host +
+				    " && req.url == " + req.url)) {
+				return(synth(204, "Ban added"));
+			} else {
+				# return ban error in 400 response
+				return(synth(400, std.ban_error()));
+			}
 		}
 		if (req.method == "BANSTR") {
-			ban ("" + req.http.ban);
-			return (synth(410));
+			if (std.ban(req.http.ban)) {
+				return(synth(204, "Ban added"));
+			} else {
+				# return ban error in 400 response
+				return(synth(400, std.ban_error()));
+			}
 		}
 	}
 } -start
@@ -44,7 +56,7 @@ client c1 {
 client c1 {
 	txreq -req BAN -url /foox
 	rxresp
-	expect resp.status == 410
+	expect resp.status == 204
 } -run
 varnish v1 -cliok "ban.list"
 
@@ -61,7 +73,7 @@ client c1 {
 client c1 {
 	txreq -req BAN -url /foo
 	rxresp
-	expect resp.status == 410
+	expect resp.status == 204
 } -run
 varnish v1 -cliok "ban.list"
 
@@ -78,7 +90,7 @@ client c1 {
 client c1 {
 	txreq -req BANSTR -hdr "ban: req.url != /foo"
 	rxresp
-	expect resp.status == 410
+	expect resp.status == 204
 } -run
 varnish v1 -cliok "ban.list"
 
@@ -95,7 +107,7 @@ client c1 {
 client c1 {
 	txreq -req BANSTR -hdr "Ban: obj.http.foo == bar6"
 	rxresp
-	expect resp.status == 410
+	expect resp.status == 204
 } -run
 varnish v1 -cliok "ban.list"
 
@@ -112,7 +124,7 @@ client c1 {
 client c1 {
 	txreq -req BANSTR -hdr "Ban: obj.http.foo == bar6"
 	rxresp
-	expect resp.status == 410
+	expect resp.status == 204
 } -run
 
 varnish v1 -cliok "ban.list"
@@ -130,7 +142,7 @@ client c1 {
 client c1 {
 	txreq -req BANSTR -hdr "Ban: req.http.foo == barcheck"
 	rxresp
-	expect resp.status == 410
+	expect resp.status == 204
 } -run
 varnish v1 -cliok "ban.list"
 
@@ -146,7 +158,7 @@ client c1 {
 client c1 {
 	txreq -req BANSTR -hdr "Ban: obj.http.foo == barcheck"
 	rxresp
-	expect resp.status == 410
+	expect resp.status == 204
 } -run
 varnish v1 -cliok "ban.list"
 varnish v1 -clijson "ban.list -j"
@@ -158,3 +170,11 @@ client c1 {
 	expect resp.http.foo == bar8
 	expect resp.bodylen == 8
 } -run
+
+# Error
+client c1 {
+	txreq -req BANSTR -hdr "Ban: xobj.http.foo == barcheck"
+	rxresp
+	expect resp.status == 400
+	expect resp.reason == {Unknown or unsupported field "xobj.http.foo"}
+} -run
diff --git a/bin/varnishtest/tests/r00502.vtc b/bin/varnishtest/tests/r00502.vtc
index 4b9d63905..929d3ba43 100644
--- a/bin/varnishtest/tests/r00502.vtc
+++ b/bin/varnishtest/tests/r00502.vtc
@@ -8,8 +8,10 @@ server s1 {
 } -start
 
 varnish v1 -vcl+backend {
+	import std;
+
 	sub vcl_recv {
-		ban("req.url == / && obj.http.foo ~ bar1");
+		std.ban("req.url == / && obj.http.foo ~ bar1");
 	}
 } -start
 
diff --git a/doc/sphinx/reference/vcl.rst b/doc/sphinx/reference/vcl.rst
index b442b88d3..00be68429 100644
--- a/doc/sphinx/reference/vcl.rst
+++ b/doc/sphinx/reference/vcl.rst
@@ -118,6 +118,8 @@ execution of a VCL state subroutine (``vcl_* {}``), including all
 user-defined subroutines being called, ``now`` always returns the
 same value.
 
+.. _vcl(7)_durations:
+
 Durations
 ~~~~~~~~~
 
@@ -456,76 +458,10 @@ The following built-in functions are available:
 ban(STRING)
 ~~~~~~~~~~~
 
-  Invalidates all objects in cache that match the given expression with the
-  ban mechanism.
-
-  The format of *STRING* is::
-
-	<field> <operator> <arg> [&& <field> <oper> <arg> ...]
-
-  * *<field>*:
-
-    * string fields:
-
-      * ``req.url``: The request url
-      * ``req.http.*``: Any request header
-      * ``obj.status``: The cache object status
-      * ``obj.http.*``: Any cache object header
-
-      ``obj.status`` is treated as a string despite the fact that it
-      is actually an integer.
-
-    * duration fields:
-
-      * ``obj.ttl``: Remaining ttl at the time the ban is issued
-      * ``obj.age``: Object age at the time the ban is issued
-      * ``obj.grace``: The grace time of the object
-      * ``obj.keep``: The keep time of the object
-
-
-  * *<operator>*:
-
-    * for all fields:
-
-      * ``==``: *<field>* and *<arg>* are equal
-      * ``!=``: *<field>* and *<arg>* are unequal
-
-      strings are compared case sensitively
-
-    * for string fields:
-
-      * ``~``: *<field>* matches the regular expression *<arg>*
-      * ``!~``:*<field>* does not match the regular expression *<arg>*
-
-    * for duration fields:
-
-      * ``>``: *<field>* is greater than *<arg>*
-      * ``>=``: *<field>* is greater than or equal to *<arg>*
-      * ``<``: *<field>* is less than *<arg>*
-      * ``<=``: *<field>* is less than or equal to *<arg>*
-
-
-  * *<arg>*:
-
-    * for string fields:
-
-      Either a literal string or a regular expression. Note that
-      *<arg>* does not use any of the string delimiters like ``"`` or
-      ``{"``\ *...*\ ``"}`` or ``"""``\ *...*\ ``"""`` used elsewhere
-      in varnish. To match against strings containing whitespace,
-      regular expressions containing ``\s`` can be used.
-
-    * for duration fields:
-
-      A VCL duration like ``10s``, ``5m`` or ``1h``, see `Durations`_
-
-  Expressions can be chained using the *and* operator ``&&``. For *or*
-  semantics, use several bans.
+  Deprecated. See :ref:`std.ban()`.
 
-  The unset *<field>* is not equal to any string, such that, for a
-  non-existing header, the operators ``==`` and ``~`` always evaluate
-  as false, while the operators ``!=`` and ``!~`` always evaluate as
-  true, respectively, for any value of *<arg>*.
+  The ``ban()`` function is identical to :ref:`std.ban()`, but does
+  not provide error reporting.
 
 hash_data(input)
 ~~~~~~~~~~~~~~~~
diff --git a/doc/sphinx/users-guide/purging.rst b/doc/sphinx/users-guide/purging.rst
index 7ceee5ffa..696c330cf 100644
--- a/doc/sphinx/users-guide/purging.rst
+++ b/doc/sphinx/users-guide/purging.rst
@@ -75,7 +75,7 @@ the following command from the shell::
 
   varnishadm ban req.http.host == example.com '&&' req.url '~' '\\.png$'
 
-See :ref:`vcl(7)_ban` for details on the syntax of ban expressions. In
+See :ref:`std.ban()` for details on the syntax of ban expressions. In
 particular, note that in the example given above, the quotes are
 required for execution from the shell and escaping the backslash in
 the regular expression is required by the Varnish cli interface.
@@ -99,18 +99,21 @@ impact CPU usage and thereby performance.
 
 You can also add bans to Varnish via HTTP. Doing so requires a bit of VCL::
 
+  import std;
+
   sub vcl_recv {
 	  if (req.method == "BAN") {
-                  # Same ACL check as above:
+		  # Same ACL check as above:
 		  if (!client.ip ~ purge) {
 			  return(synth(403, "Not allowed."));
 		  }
-		  ban("req.http.host == " + req.http.host +
-		        " && req.url == " + req.url);
-
-		  # Throw a synthetic page so the
-                  # request won't go to the backend.
-		  return(synth(200, "Ban added"));
+		  if (std.ban("req.http.host == " + req.http.host +
+		      " && req.url == " + req.url)) {
+			  return(synth(200, "Ban added"));
+		  } else {
+			  # return ban error in 400 response
+			  return(synth(400, std.ban_error()));
+		  }
 	  }
   }
 
@@ -123,21 +126,30 @@ object is not available in the `ban lurker` thread.
 
 You can use the following template to write `ban lurker` friendly bans::
 
+  import std;
+
   sub vcl_backend_response {
-    set beresp.http.url = bereq.url;
+	  set beresp.http.url = bereq.url;
   }
 
   sub vcl_deliver {
-    unset resp.http.url; # Optional
+	  unset resp.http.url; # Optional
   }
 
   sub vcl_recv {
-    if (req.method == "PURGE") {
-      if (client.ip !~ purge) {
-        return(synth(403, "Not allowed"));
-      }
-      ban("obj.http.url ~ " + req.url); # Assumes req.url is a regex. This might be a bit too simple
-    }
+	  if (req.method == "BAN") {
+		  # Same ACL check as above:
+		  if (!client.ip ~ purge) {
+			  return(synth(403, "Not allowed."));
+		  }
+		  # Assumes req.url is a regex. This might be a bit too simple
+		  if (std.ban("obj.http.url ~ " + req.url)) {
+			  return(synth(200, "Ban added"));
+		  } else {
+			  # return ban error in 400 response
+			  return(synth(400, std.ban_error()));
+		  }
+	  }
   }
 
 To inspect the current ban list, issue the ``ban.list`` command in the CLI. This
diff --git a/lib/libvmod_std/vmod_std.vcc b/lib/libvmod_std/vmod_std.vcc
index e0a17f019..e9812ee8e 100644
--- a/lib/libvmod_std/vmod_std.vcc
+++ b/lib/libvmod_std/vmod_std.vcc
@@ -563,8 +563,77 @@ Example::
 
 $Function BOOL ban(STRING)
 
-Same as :ref:`vcl(7)_ban`, but returns true if the ban succeeded and
-false otherwise.
+Invalidates all objects in cache that match the given expression with
+the ban mechanism. Returns ``true`` if the ban succeeded and ``false``
+otherwise. Error details are available via `std.ban_error()`_.
+
+The format of *STRING* is::
+
+	<field> <operator> <arg> [&& <field> <oper> <arg> ...]
+
+* *<field>*:
+
+  * string fields:
+
+    * ``req.url``: The request url
+    * ``req.http.*``: Any request header
+    * ``obj.status``: The cache object status
+    * ``obj.http.*``: Any cache object header
+
+    ``obj.status`` is treated as a string despite the fact that it
+    is actually an integer.
+
+  * duration fields:
+
+    * ``obj.ttl``: Remaining ttl at the time the ban is issued
+    * ``obj.age``: Object age at the time the ban is issued
+    * ``obj.grace``: The grace time of the object
+    * ``obj.keep``: The keep time of the object
+
+
+* *<operator>*:
+
+  * for all fields:
+
+    * ``==``: *<field>* and *<arg>* are equal
+    * ``!=``: *<field>* and *<arg>* are unequal
+
+    strings are compared case sensitively
+
+  * for string fields:
+
+    * ``~``: *<field>* matches the regular expression *<arg>*
+    * ``!~``:*<field>* does not match the regular expression *<arg>*
+
+  * for duration fields:
+
+    * ``>``: *<field>* is greater than *<arg>*
+    * ``>=``: *<field>* is greater than or equal to *<arg>*
+    * ``<``: *<field>* is less than *<arg>*
+    * ``<=``: *<field>* is less than or equal to *<arg>*
+
+
+* *<arg>*:
+
+  * for string fields:
+
+    Either a literal string or a regular expression. Note that
+    *<arg>* does not use any of the string delimiters like ``"`` or
+    ``{"``\ *...*\ ``"}`` or ``"""``\ *...*\ ``"""`` used elsewhere
+    in varnish. To match against strings containing whitespace,
+    regular expressions containing ``\s`` can be used.
+
+  * for duration fields:
+
+    A VCL duration like ``10s``, ``5m`` or ``1h``, see :ref:`vcl(7)_durations`
+
+Expressions can be chained using the *and* operator ``&&``. For *or*
+semantics, use several bans.
+
+The unset *<field>* is not equal to any string, such that, for a
+non-existing header, the operators ``==`` and ``~`` always evaluate as
+false, while the operators ``!=`` and ``!~`` always evaluate as true,
+respectively, for any value of *<arg>*.
 
 $Function STRING ban_error()
 


More information about the varnish-commit mailing list