[core] fix conditional cache handling

- add new "skip" result to mark conditions that didn't actually get
  evaluated to false but just skipped because the preconditions failed.
- add "local_result" for each cache entry to remember whether the
  condition itself matched (not including the preconditions).
  this can be reused after a cache reset if the condition itself was not
  reset, but the preconditions were
- clear result of subtree (children and else-branches) when clearing a
  condition cache

From: Stefan Bühler <stbuehler@web.de>

git-svn-id: svn://svn.lighttpd.net/lighttpd/branches/lighttpd-1.4.x@3082 152afb58-edef-0310-8abb-c4023f1b3aa9

NEWS

@@ -14,6 +14,7 @@ NEWS
   * [mod_cgi] issue trace and exit if execve() fails (closes #2302)
   * [configparser] don't continue after parse error (fixes #2717)
   * [core] never evaluate else branches until the previous branches are ready (fixes #2598)
+  * [core] fix conditional cache handling
 
 - 1.4.39 - 2016-01-02
   * [core] fix memset_s call (fixes #2698)

src/array.h

@@ -105,8 +105,7 @@ typedef enum {
  * for compare: comp          cond  string/regex
  */
 
-typedef struct _data_config data_config;
-struct _data_config {
+typedef struct data_config {
 	DATA_UNSET;
 
 	array *value;
@@ -118,19 +117,19 @@ struct _data_config {
 	buffer *op;
 
 	int context_ndx; /* more or less like an id */
-	array *childs;
+	array *children;
 	/* nested */
-	data_config *parent;
+	struct data_config *parent;
 	/* for chaining only */
-	data_config *prev;
-	data_config *next;
+	struct data_config *prev;
+	struct data_config *next;
 
 	buffer *string;
 #ifdef HAVE_PCRE_H
 	pcre   *regex;
 	pcre_extra *regex_study;
 #endif
-};
+} data_config;
 
 data_config *data_config_init(void);
 

src/base.h

@@ -343,14 +343,30 @@ typedef enum {
 	CON_STATE_CLOSE
 } connection_state_t;
 
-typedef enum { COND_RESULT_UNSET, COND_RESULT_FALSE, COND_RESULT_TRUE } cond_result_t;
+typedef enum {
+	/* condition not active at the moment because itself or some
+	 * pre-condition depends on data not available yet
+	 */
+	COND_RESULT_UNSET,
+
+	/* special "unset" for branches not selected due to pre-conditions
+	 * not met (but pre-conditions are not "unset" anymore)
+	 */
+	COND_RESULT_SKIP,
+
+	/* actually evaluated the condition itself */
+	COND_RESULT_FALSE, /* not active */
+	COND_RESULT_TRUE, /* active */
+} cond_result_t;
+
 typedef struct {
+	/* current result (with preconditions) */
 	cond_result_t result;
+	/* result without preconditions (must never be "skip") */
+	cond_result_t local_result;
 	int patterncount;
 	int matches[3 * 10];
 	buffer *comp_value; /* just a pointer */
-
-	comp_key_t comp_type;
 } cond_cache_t;
 
 typedef struct {

src/configfile-glue.c

@@ -225,11 +225,22 @@ static unsigned short sock_addr_get_port(sock_addr *addr) {
 #endif
 }
 
+static const char* cond_result_to_string(cond_result_t cond_result) {
+	switch (cond_result) {
+	case COND_RESULT_UNSET: return "unset";
+	case COND_RESULT_SKIP: return "skipped";
+	case COND_RESULT_FALSE: return "false";
+	case COND_RESULT_TRUE: return "true";
+	default: return "invalid cond_result_t";
+	}
+}
+
 static cond_result_t config_check_cond_cached(server *srv, connection *con, data_config *dc);
 
 static cond_result_t config_check_cond_nocache(server *srv, connection *con, data_config *dc) {
 	buffer *l;
 	server_socket *srv_sock = con->srv_socket;
+	cond_cache_t *cache = &con->cond_cache[dc->context_ndx];
 
 	/* check parent first */
 	if (dc->parent && dc->parent->context_ndx) {
@@ -243,21 +254,23 @@ static cond_result_t config_check_cond_nocache(server *srv, connection *con, dat
 		}
 
 		switch (config_check_cond_cached(srv, con, dc->parent)) {
-		case COND_RESULT_FALSE:
-			return COND_RESULT_FALSE;
 		case COND_RESULT_UNSET:
+			/* decide later */
 			return COND_RESULT_UNSET;
-		default:
+		case COND_RESULT_SKIP:
+		case COND_RESULT_FALSE:
+			/* failed precondition */
+			return COND_RESULT_SKIP;
+		case COND_RESULT_TRUE:
+			/* proceed */
 			break;
 		}
 	}
 
 	if (dc->prev) {
 		/**
-		 * a else branch
-		 *
-		 * we can only be executed, if all of our previous brothers 
-		 * are false
+		 * a else branch; can only be executed if the previous branch
+		 * was evaluated as "false" (not unset/skipped/true)
 		 */
 		if (con->conf.log_condition_handling) {
 			log_error_write(srv, __FILE__, __LINE__,  "sb", "go prev", dc->prev->key);
@@ -266,18 +279,14 @@ static cond_result_t config_check_cond_nocache(server *srv, connection *con, dat
 		/* make sure prev is checked first */
 		switch (config_check_cond_cached(srv, con, dc->prev)) {
 		case COND_RESULT_UNSET:
+			/* decide later */
 			return COND_RESULT_UNSET;
-		case COND_RESULT_FALSE:
-			break;
+		case COND_RESULT_SKIP:
 		case COND_RESULT_TRUE:
-			return COND_RESULT_FALSE;
-		}
-
-		/* one of prev set me to FALSE */
-		switch (con->cond_cache[dc->context_ndx].result) {
+			/* failed precondition */
+			return COND_RESULT_SKIP;
 		case COND_RESULT_FALSE:
-			return con->cond_cache[dc->context_ndx].result;
-		default:
+			/* proceed */
 			break;
 		}
 	}
@@ -287,12 +296,21 @@ static cond_result_t config_check_cond_nocache(server *srv, connection *con, dat
 			log_error_write(srv, __FILE__, __LINE__,  "dss", 
 				dc->comp,
 				dc->key->ptr,
-				con->conditional_is_valid[dc->comp] ? "yeah" : "nej");
+				"not available yet");
 		}
 
 		return COND_RESULT_UNSET;
 	}
 
+	/* if we had a real result before and weren't cleared just return it */
+	switch (cache->local_result) {
+	case COND_RESULT_TRUE:
+	case COND_RESULT_FALSE:
+		return cache->local_result;
+	default:
+		break;
+	}
+
 	/* pass the rules */
 
 	switch (dc->comp) {
@@ -499,7 +517,6 @@ static cond_result_t config_check_cond_nocache(server *srv, connection *con, dat
 #ifdef HAVE_PCRE_H
 	case CONFIG_COND_NOMATCH:
 	case CONFIG_COND_MATCH: {
-		cond_cache_t *cache = &con->cond_cache[dc->context_ndx];
 		int n;
 
 #ifndef elementsof
@@ -511,7 +528,6 @@ static cond_result_t config_check_cond_nocache(server *srv, connection *con, dat
 		cache->patterncount = n;
 		if (n > 0) {
 			cache->comp_value = l;
-			cache->comp_type  = dc->comp;
 			return (dc->cond == CONFIG_COND_MATCH) ? COND_RESULT_TRUE : COND_RESULT_FALSE;
 		} else {
 			/* cache is already cleared */
@@ -532,37 +548,56 @@ static cond_result_t config_check_cond_cached(server *srv, connection *con, data
 	cond_cache_t *caches = con->cond_cache;
 
 	if (COND_RESULT_UNSET == caches[dc->context_ndx].result) {
-		if (COND_RESULT_TRUE == (caches[dc->context_ndx].result = config_check_cond_nocache(srv, con, dc))) {
-			if (dc->next) {
-				data_config *c;
-				if (con->conf.log_condition_handling) {
-					log_error_write(srv, __FILE__, __LINE__, "s",
-							"setting remains of chaining to false");
-				}
-				for (c = dc->next; c; c = c->next) {
-					caches[c->context_ndx].result = COND_RESULT_FALSE;
-				}
-			}
+		caches[dc->context_ndx].result = config_check_cond_nocache(srv, con, dc);
+		switch (caches[dc->context_ndx].result) {
+		case COND_RESULT_FALSE:
+		case COND_RESULT_TRUE:
+			/* remember result of local condition for a partial reset */
+			caches[dc->context_ndx].local_result = caches[dc->context_ndx].result;
+			break;
+		default:
+			break;
 		}
-		caches[dc->context_ndx].comp_type = dc->comp;
 
 		if (con->conf.log_condition_handling) {
-			log_error_write(srv, __FILE__, __LINE__, "dss", dc->context_ndx,
-					"(uncached) result:",
-					caches[dc->context_ndx].result == COND_RESULT_UNSET ? "unknown" :
-						(caches[dc->context_ndx].result == COND_RESULT_TRUE ? "true" : "false"));
+			log_error_write(srv, __FILE__, __LINE__, "dss",
+				dc->context_ndx,
+				"(uncached) result:",
+				cond_result_to_string(caches[dc->context_ndx].result));
 		}
 	} else {
 		if (con->conf.log_condition_handling) {
-			log_error_write(srv, __FILE__, __LINE__, "dss", dc->context_ndx,
-					"(cached) result:",
-					caches[dc->context_ndx].result == COND_RESULT_UNSET ? "unknown" : 
-						(caches[dc->context_ndx].result == COND_RESULT_TRUE ? "true" : "false"));
+			log_error_write(srv, __FILE__, __LINE__, "dss",
+				dc->context_ndx,
+				"(cached) result:",
+				cond_result_to_string(caches[dc->context_ndx].result));
 		}
 	}
 	return caches[dc->context_ndx].result;
 }
 
+/* if we reset the cache result for a node, we also need to clear all
+ * child nodes and else-branches*/
+static void config_cond_clear_node(server *srv, connection *con, data_config *dc) {
+	/* if a node is "unset" all children are unset too */
+	if (con->cond_cache[dc->context_ndx].result != COND_RESULT_UNSET) {
+		size_t i;
+
+		con->cond_cache[dc->context_ndx].patterncount = 0;
+		con->cond_cache[dc->context_ndx].comp_value = NULL;
+		con->cond_cache[dc->context_ndx].result = COND_RESULT_UNSET;
+
+		for (i = 0; i < dc->children->used; ++i) {
+			data_config *dc_child = (data_config*) dc->children->data[i];
+			if (NULL == dc_child->prev) {
+				/* only call for first node in if-else chain */
+				config_cond_clear_node(srv, con, dc_child);
+			}
+		}
+		if (NULL != dc->next) config_cond_clear_node(srv, con, dc->next);
+	}
+}
+
 /**
  * reset the config-cache for a named item
  *
@@ -572,11 +607,13 @@ void config_cond_cache_reset_item(server *srv, connection *con, comp_key_t item)
 	size_t i;
 
 	for (i = 0; i < srv->config_context->used; i++) {
-		if (item == COMP_LAST_ELEMENT || 
-		    con->cond_cache[i].comp_type == item) {
-			con->cond_cache[i].result = COND_RESULT_UNSET;
-			con->cond_cache[i].patterncount = 0;
-			con->cond_cache[i].comp_value = NULL;
+		data_config *dc = (data_config *)srv->config_context->data[i];
+
+		if (item == dc->comp) {
+			/* clear local_result */
+			con->cond_cache[i].local_result = COND_RESULT_UNSET;
+			/* clear result in subtree (including the node itself) */
+			config_cond_clear_node(srv, con, dc);
 		}
 	}
 }
@@ -587,7 +624,13 @@ void config_cond_cache_reset_item(server *srv, connection *con, comp_key_t item)
 void config_cond_cache_reset(server *srv, connection *con) {
 	size_t i;
 
-	config_cond_cache_reset_all_items(srv, con);
+	/* resetting all entries; no need to follow children as in config_cond_cache_reset_item */
+	for (i = 0; i < srv->config_context->used; i++) {
+		con->cond_cache[i].result = COND_RESULT_UNSET;
+		con->cond_cache[i].local_result = COND_RESULT_UNSET;
+		con->cond_cache[i].patterncount = 0;
+		con->cond_cache[i].comp_value = NULL;
+	}
 
 	for (i = 0; i < COMP_LAST_ELEMENT; i++) {
 		con->conditional_is_valid[i] = 0;
@@ -614,4 +657,3 @@ int config_append_cond_match_buffer(connection *con, data_config *dc, buffer *bu
 			cache->matches[n + 1] - cache->matches[n]);
 	return 1;
 }
-

src/configfile.h

@@ -24,7 +24,4 @@ data_unset *configparser_merge_data(data_unset *op1, const data_unset *op2);
 void config_cond_cache_reset(server *srv, connection *con);
 void config_cond_cache_reset_item(server *srv, connection *con, comp_key_t item);
 
-#define config_cond_cache_reset_all_items(srv, con) \
-	config_cond_cache_reset_item(srv, con, COMP_LAST_ELEMENT);
-
 #endif

src/configparser.y

@@ -17,7 +17,7 @@ static void configparser_push(config_t *ctx, data_config *dc, int isnew) {
     force_assert(dc->context_ndx > ctx->current->context_ndx);
     array_insert_unique(ctx->all_configs, (data_unset *)dc);
     dc->parent = ctx->current;
-    array_insert_unique(dc->parent->childs, (data_unset *)dc);
+    array_insert_unique(dc->parent->children, (data_unset *)dc);
   }
   if (ctx->configs_stack->used > 0 && ctx->current->context_ndx == 0) {
     fprintf(stderr, "Cannot use conditionals inside a global { ... } block\n");

src/data_config.c

@@ -23,7 +23,7 @@ static void data_config_free(data_unset *d) {
 	buffer_free(ds->comp_key);
 
 	array_free(ds->value);
-	array_free(ds->childs);
+	array_free(ds->children);
 
 	if (ds->string) buffer_free(ds->string);
 #ifdef HAVE_PCRE_H
@@ -84,10 +84,10 @@ static void data_config_print(const data_unset *d, int depth) {
 		fprintf(stdout, "\n");
 	}
 
-	if (ds->childs) {
+	if (ds->children) {
 		fprintf(stdout, "\n");
-		for (i = 0; i < ds->childs->used; i ++) {
-			data_unset *du = ds->childs->data[i];
+		for (i = 0; i < ds->children->used; i ++) {
+			data_unset *du = ds->children->data[i];
 
 			/* only the 1st block of chaining */
 			if (NULL == ((data_config *)du)->prev) {
@@ -124,8 +124,8 @@ data_config *data_config_init(void) {
 	ds->op = buffer_init();
 	ds->comp_key = buffer_init();
 	ds->value = array_init();
-	ds->childs = array_init();
-	ds->childs->is_weakref = 1;
+	ds->children = array_init();
+	ds->children->is_weakref = 1;
 
 	ds->copy = data_config_copy;
 	ds->free = data_config_free;