Merge branch 'order_policycache_misc' into 'master'

config order, policy cache, misc

see commit messages

See merge request !1

README.rst

@@ -151,12 +151,12 @@ object::
       }
 
       # .policy() returns 1 for policy type OPEN
-      if (config.policy(req.http.Host, req.url) == 1) {
+      if (config.policy() == 1) {
       	 return(pass);
       }
   
       # .policy() returns 2 for policy type TOKEN
-      if (config.policy(req.http.Host, req.url) == 2) {
+      if (config.policy() == 2) {
       	 # Handle token authorization ...
       	 # [...]
       }
@@ -449,7 +449,7 @@ hosts.policy
 
 ::
 
-	INT hosts.policy(PRIV_TASK, STRING host, STRING path)
+	INT hosts.policy(PRIV_TASK, STRING host=0, STRING path=0)
 
 Determine the policy type that holds for ``host`` and ``path``. The
 return values are:
@@ -460,9 +460,8 @@ return values are:
 * -1 if no matching policy can be found
 * -2 if there was an internal error
 
-The ``host`` and ``path`` parameters are required, and must be
-non-empty. This method MAY NOT be called in ``vcl_init``. If it is,
-then the VCL load fails.
+This method MAY NOT be called in ``vcl_init``. If it is, then the VCL
+load fails.
 
 The method searches for host names added by the ``.add()`` method that
 match ``host``, possibly matching the suffix if the host name in
@@ -502,6 +501,13 @@ Subsequent calls to the ``.token()``, ``.secret()`` or ``.explain()``
 methods refer to the most recent invocation of ``.policy()`` in the
 same task scope, that is in the same client or backend transaction.
 
+Likewise, if both the ``host`` and ``path`` parameters are empty,
+``.policy()`` returns again the result of the most recent invocation
+with parameters.
+
+Calling ``.policy()`` with only one of the ``host`` and ``path``
+parameters empty is an error.
+
 .. _func_hosts.token:
 
 hosts.token

src/tests/explain.vtc

@@ -12,12 +12,23 @@ varnish v1 -vcl {
 		new h = hoailona.hosts();
 		h.add("example.com", "p1");
 		h.add("example.org", "p2");
+
+		// specific entry after wildcard - never matched
 		h.add("*.example.com", "p1", "/foo/...");
 		h.add("*.example.org", "p2", "/bar/...");
+		h.add("foo.example.org", "p2", "...");
+
 		h.add("example.net", "p1", description="net");
 		h.add("example.edu", "p2", description="edu");
-		h.add("*.example.net", "p1", "/baz/...", description="sub net");
-		h.add("*.example.edu", "p2", "/baz/...", description="sub edu");
+
+		// specific entry before wildcard has precendence,
+		// but lookup stops at the first host match
+		h.add("foo.example.net", "p1", "/bar/...",
+		      description="sub net");
+		h.add("*.example.net", "p1", "/baz/...",
+		      description="wild sub net");
+		h.add("*.example.edu", "p2", "/baz/...",
+		      description="wild sub edu");
 	}
 
 	sub vcl_recv {
@@ -37,12 +48,16 @@ varnish v1 -vcl {
 		set resp.http.e5 = h.explain();
 		set resp.http.p6 = h.policy("example.edu", "/foo/bar");
 		set resp.http.e6 = h.explain();
-		set resp.http.p7 = h.policy("foo.example.net", "/baz/quux");
+		set resp.http.p7 = h.policy("foo.example.net", "/bar/quux");
 		set resp.http.e7 = h.explain();
-		set resp.http.p8 = h.policy("foo.example.edu", "/baz/quux");
+		set resp.http.p8 = h.policy("foo.example.net", "/baz/quux");
 		set resp.http.e8 = h.explain();
-		set resp.http.p9 = h.policy("foo.example.com", "/baz/quux");
+		set resp.http.p9 = h.policy("bar.example.net", "/baz/quux");
 		set resp.http.e9 = h.explain();
+		set resp.http.pA = h.policy("foo.example.edu", "/baz/quux");
+		set resp.http.eA = h.explain();
+		set resp.http.pB = h.policy("foo.example.com", "/baz/quux");
+		set resp.http.eB = h.explain();
 	}
 } -start
 
@@ -63,11 +78,15 @@ client c1 {
 	expect resp.http.p6 == "1"
 	expect resp.http.e6 == "Matched host example.edu (edu) for global policy p2 (open)"
 	expect resp.http.p7 == "2"
-	expect resp.http.e7 == "Matched host *.example.net and pattern /baz/... (sub net) for policy p1"
-	expect resp.http.p8 == "1"
-	expect resp.http.e8 == "Matched host *.example.edu and pattern /baz/... (sub edu) for policy p2 (open)"
-	expect resp.http.p9 == "-1"
-	expect resp.http.e9 == "No policy was matched"
+	expect resp.http.e7 == "Matched host foo.example.net and pattern /bar/... (sub net) for policy p1"
+	expect resp.http.p8 == "-1"
+	expect resp.http.e8 == "No policy was matched"
+	expect resp.http.p9 == "2"
+	expect resp.http.e9 == "Matched host *.example.net and pattern /baz/... (wild sub net) for policy p1"
+	expect resp.http.pA == "1"
+	expect resp.http.eA == "Matched host *.example.edu and pattern /baz/... (wild sub edu) for policy p2 (open)"
+	expect resp.http.pB == "-1"
+	expect resp.http.eB == "No policy was matched"
 } -run
 
 varnish v1 -errvcl {h.explain() may not be called in vcl_init} {

src/tests/policy_method.vtc

@@ -21,9 +21,13 @@ varnish v1 -vcl {
 	}
 
 	sub vcl_synth {
-		set resp.http.p1 = h.policy("example.com", "/foo/bar");
-		set resp.http.p2 = h.policy("example.org", "/quux/4711");
-		set resp.http.p3 = h.policy("example.com", "/bar/foo");
+		set resp.http.p0  = h.policy();
+		set resp.http.p1  = h.policy("example.com", "/foo/bar");
+		set resp.http.p1c = h.policy();
+		set resp.http.p2  = h.policy("example.org", "/quux/4711");
+		set resp.http.p2c = h.policy();
+		set resp.http.p3  = h.policy("example.com", "/bar/foo");
+		set resp.http.p3c = h.policy();
 	}
 } -start
 
@@ -31,9 +35,13 @@ client c1 {
 	txreq
 	rxresp
 	expect resp.status == 200
-	expect resp.http.p1 == "2"
-	expect resp.http.p2 == "1"
-	expect resp.http.p3 == "0"
+	expect resp.http.p0  == "-2"
+	expect resp.http.p1  == "2"
+	expect resp.http.p1c == "2"
+	expect resp.http.p2  == "1"
+	expect resp.http.p2c == "1"
+	expect resp.http.p3  == "0"
+	expect resp.http.p3c == "0"
 } -run
 
 # Examples from Akamai docs
@@ -416,7 +424,7 @@ varnish v1 -vcl {
 }
 
 client c1 {
-	txreq
+	txreq -url "/emptycheck"
 	rxresp
 	expect resp.status == 200
 	expect resp.http.p1 == "-2"
@@ -426,7 +434,7 @@ client c1 {
 } -run
 
 logexpect l1 -v v1 -d 1 -g vxid -q "VCL_Error" {
-        expect 0 * Begin req
+        expect * * ReqURL ^/emptycheck
         expect * = VCL_Error "^vmod hoailona error: host is empty in h.policy..$"
         expect * = VCL_Error "^vmod hoailona error: host is empty in h.policy..$"
         expect * = VCL_Error "^vmod hoailona error: path is empty in h.policy..$"

src/vmod_hoailona.c

@@ -56,14 +56,14 @@ struct host {
 	unsigned			magic;
 #define VMOD_HOAILONA_HOST_MAGIC 0x731af58f
 	struct assign_tree		assignments;
-	VSLIST_ENTRY(host)		list;
+	VSTAILQ_ENTRY(host)		list;
 	char				*name;
 	char				*description;
 	struct vmod_hoailona_policy	*policy;
 	size_t				len;
 };
 
-typedef VSLIST_HEAD(hosthead, host) hosthead_t;
+typedef VSTAILQ_HEAD(hosthead, host) hosthead_t;
 
 struct vmod_hoailona_hosts {
 	unsigned	magic;
@@ -94,23 +94,30 @@ WS_Contains(struct ws * const restrict ws, const void * const restrict ptr,
         assert((char *)ptr >= ws->s && (char *)(ptr + len) <= ws->e);
 }
 
+static struct vmod_hoailona_policy *
+get_policy(VRT_CTX, const struct vmod_priv * restrict const,
+	   const char * restrict const,
+	   const char * restrict const);
+
 void
 errmsg(VRT_CTX, const char *fmt, ...)
 {
 	va_list args;
 
 	va_start(args, fmt);
+	if (ctx->vsl)
+		VSLbv(ctx->vsl, SLT_VCL_Error, fmt, args);
+	else
+		VSLv(SLT_VCL_Error, 0, fmt, args);
+	va_end(args);
+
 	if (ctx->method == VCL_MET_INIT) {
 		AN(ctx->msg);
+		va_start(args, fmt);
 		VSB_vprintf(ctx->msg, fmt, args);
+		va_end(args);
 		VRT_handling(ctx, VCL_RET_FAIL);
 	}
-	else if (ctx->vsl)
-		VSLbv(ctx->vsl, SLT_VCL_Error, fmt, args);
-	else
-		/* Should this ever happen in vcl_fini() ... */
-		VSL(SLT_VCL_Error, 0, fmt, args);
-	va_end(args);
 }
 
 /* Event function */
@@ -214,6 +221,8 @@ vmod_policy__fini(struct vmod_hoailona_policy **policyp)
 	struct vmod_hoailona_policy *policy;
 
 	policy = *policyp;
+	if (policy == NULL)
+		return;
 	*policyp = NULL;
 	CHECK_OBJ_NOTNULL(policy, VMOD_HOAILONA_POLICY_MAGIC);
 	if (policy->vcl_name != NULL)
@@ -246,7 +255,7 @@ vmod_hosts__init(VRT_CTX, struct vmod_hoailona_hosts **hostsp,
 
 	hosts->vcl_name = strdup(vcl_name);
 	AN(hosts->vcl_name);
-	VSLIST_INIT(&hosts->hosthead);
+	VSTAILQ_INIT(&hosts->hosthead);
 }
 
 /*
@@ -261,11 +270,13 @@ vmod_hosts__fini(struct vmod_hoailona_hosts **hostsp)
 	struct host *host = NULL;
 
 	hosts = *hostsp;
+	if (hosts == NULL)
+		return;
 	*hostsp = NULL;
 	CHECK_OBJ_NOTNULL(hosts, VMOD_HOAILONA_HOSTS_MAGIC);
 	if (hosts->vcl_name != NULL)
 		free(hosts->vcl_name);
-	host = VSLIST_FIRST(&hosts->hosthead);
+	host = VSTAILQ_FIRST(&hosts->hosthead);
 	while (host != NULL) {
 		struct assignment *a;
 		struct host *next_host = NULL;
@@ -295,7 +306,7 @@ vmod_hosts__fini(struct vmod_hoailona_hosts **hostsp)
 			FREE_OBJ(a);
 			a = next_ass;
 		}
-		next_host = VSLIST_NEXT(host, list);
+		next_host = VSTAILQ_NEXT(host, list);
 		FREE_OBJ(host);
 		host = next_host;
 	}
@@ -396,7 +407,7 @@ vmod_hosts_add(VRT_CTX, struct vmod_hoailona_hosts *hosts,
 		return;
 	}
 
-	VSLIST_FOREACH(host, &hosts->hosthead, list) {
+	VSTAILQ_FOREACH(host, &hosts->hosthead, list) {
 		CHECK_OBJ(host, VMOD_HOAILONA_HOST_MAGIC);
 		if (strcmp(hostname, host->name) == 0)
 			break;
@@ -443,7 +454,7 @@ vmod_hosts_add(VRT_CTX, struct vmod_hoailona_hosts *hosts,
 		VRB_INIT(&host->assignments);
 		AZ(host->description);
 		AZ(host->policy);
-		VSLIST_INSERT_HEAD(&hosts->hosthead, host, list);
+		VSTAILQ_INSERT_TAIL(&hosts->hosthead, host, list);
 	}
 
 	if (path == NULL) {
@@ -488,10 +499,24 @@ vmod_hosts_policy(VRT_CTX, struct vmod_hoailona_hosts *hosts,
 		return -2;
 	}
 	if (hostname == NULL || hostname[0] == '\0') {
-		VERR(ctx, "host is empty in %s.policy()", hosts->vcl_name);
-		return -2;
-	}
-	if (pathname == NULL || pathname[0] == '\0') {
+		if (! (pathname == NULL || pathname[0] == '\0')) {
+			VERR(ctx, "host is empty in %s.policy()",
+			     hosts->vcl_name);
+			return -2;
+		}
+
+		if (priv_task->priv == NULL) {
+			VERR(ctx, "%s.policy() no cached result",
+			     hosts->vcl_name);
+			return -2;
+		}
+
+		policy = get_policy(ctx, priv_task, hosts->vcl_name, "policy");
+
+		if (policy == NULL)
+			return -1;
+		return policy->type;
+	} else if (pathname == NULL || pathname[0] == '\0') {
 		VERR(ctx, "path is empty in %s.policy()", hosts->vcl_name);
 		return -2;
 	}
@@ -519,7 +544,7 @@ vmod_hosts_policy(VRT_CTX, struct vmod_hoailona_hosts *hosts,
 
 	/* XXX optimize */
 	hostlen = strlen(hostname);
-	VSLIST_FOREACH(h, &hosts->hosthead, list) {
+	VSTAILQ_FOREACH(h, &hosts->hosthead, list) {
 		const char *q, *hs;
 
 		CHECK_OBJ(h, VMOD_HOAILONA_HOST_MAGIC);

src/vmod_hoailona.vcc

@@ -134,12 +134,12 @@ object::
       }
 
       # .policy() returns 1 for policy type OPEN
-      if (config.policy(req.http.Host, req.url) == 1) {
+      if (config.policy() == 1) {
       	 return(pass);
       }
   
       # .policy() returns 2 for policy type TOKEN
-      if (config.policy(req.http.Host, req.url) == 2) {
+      if (config.policy() == 2) {
       	 # Handle token authorization ...
       	 # [...]
       }
@@ -399,7 +399,7 @@ Examples::
     h.add("evil.org", "deny", description="no access to evil.org");
   }
 
-$Method INT .policy(PRIV_TASK, STRING host, STRING path)
+$Method INT .policy(PRIV_TASK, STRING host=0, STRING path=0)
 
 Determine the policy type that holds for ``host`` and ``path``. The
 return values are:
@@ -410,9 +410,8 @@ return values are:
 * -1 if no matching policy can be found
 * -2 if there was an internal error
 
-The ``host`` and ``path`` parameters are required, and must be
-non-empty. This method MAY NOT be called in ``vcl_init``. If it is,
-then the VCL load fails.
+This method MAY NOT be called in ``vcl_init``. If it is, then the VCL
+load fails.
 
 The method searches for host names added by the ``.add()`` method that
 match ``host``, possibly matching the suffix if the host name in
@@ -452,6 +451,13 @@ Subsequent calls to the ``.token()``, ``.secret()`` or ``.explain()``
 methods refer to the most recent invocation of ``.policy()`` in the
 same task scope, that is in the same client or backend transaction.
 
+Likewise, if both the ``host`` and ``path`` parameters are empty,
+``.policy()`` returns again the result of the most recent invocation
+with parameters.
+
+Calling ``.policy()`` with only one of the ``host`` and ``path``
+parameters empty is an error.
+
 $Method STRING .token(PRIV_TASK, STRING acl=0, DURATION ttl=0, STRING data=0)
 
 If the previous invocation of ``.policy()`` determined policy type