fixed aggressive caching of conditionals (#41)

$HTTP["url"] =~ "" { cgi.assign = ... } fails if there is a module
loaded which is called before uri_clean is set (mod_exforward,
mod_rewrite, ...) 

- merged [1792], [1798], [1807], [1810], [1811] from trunk


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

NEWS

@@ -16,7 +16,9 @@ NEWS
   * fixed invalid "304 Not Modified" on broken timestamps
   * fixed endless loop on shrinked files with sendfile() on BSD (#1289)
   * fixed counter overrun in ?auto in mod_status (#909)
+  * fixed too aggresive caching of nested conditionals 
   * removed config-check if passwd files exist (#1188)
+  
 
 - 1.4.16 - 
 

src/array.h

@@ -89,7 +89,9 @@ typedef enum {
 	COMP_HTTP_USERAGENT,
 	COMP_HTTP_COOKIE,
 	COMP_HTTP_REMOTEIP,
-	COMP_HTTP_QUERYSTRING
+	COMP_HTTP_QUERYSTRING,
+
+	COMP_LAST_ELEMENT
 } comp_key_t;
 
 /* $HTTP["host"] ==    "incremental.home.kneschke.de" { ... }

src/base.h

@@ -324,6 +324,8 @@ typedef struct {
 	int patterncount;
 	int matches[3 * 10];
 	buffer *comp_value; /* just a pointer */
+	
+	comp_key_t comp_type;
 } cond_cache_t;
 
 typedef struct {
@@ -416,6 +418,8 @@ typedef struct {
 #endif
 	/* etag handling */
 	etag_flags_t etag_flags;
+
+	int conditional_is_valid[COMP_LAST_ELEMENT]; 
 } connection;
 
 typedef struct {

src/configfile-glue.c

@@ -6,6 +6,8 @@
 #include "log.h"
 #include "plugin.h"
 
+#include "configfile.h"
+
 /**
  * like all glue code this file contains functions which
  * are the external interface of lighttpd. The functions
@@ -133,6 +135,7 @@ int config_insert_values_internal(server *srv, array *ca, const config_values_t
 			break;
 		}
 	}
+
 	return 0;
 }
 
@@ -174,28 +177,62 @@ static cond_result_t config_check_cond_cached(server *srv, connection *con, data
 static cond_result_t config_check_cond_nocache(server *srv, connection *con, data_config *dc) {
 	buffer *l;
 	server_socket *srv_sock = con->srv_socket;
+
 	/* check parent first */
 	if (dc->parent && dc->parent->context_ndx) {
+		/**
+		 * a nested conditional 
+		 *
+		 * if the parent is not decided yet or false, we can't be true either 
+		 */
 		if (con->conf.log_condition_handling) {
 			log_error_write(srv, __FILE__, __LINE__,  "sb", "go parent", dc->parent->key);
 		}
-		if (config_check_cond_cached(srv, con, dc->parent) == COND_RESULT_FALSE) {
+
+		switch (config_check_cond_cached(srv, con, dc->parent)) {
+		case COND_RESULT_FALSE:
 			return COND_RESULT_FALSE;
+		case COND_RESULT_UNSET:
+			return COND_RESULT_UNSET;
+		default:
+			break;
 		}
 	}
 
 	if (dc->prev) {
+		/**
+		 * a else branch
+		 *
+		 * we can only be executed, if all of our previous brothers 
+		 * are false
+		 */
 		if (con->conf.log_condition_handling) {
 			log_error_write(srv, __FILE__, __LINE__,  "sb", "go prev", dc->prev->key);
 		}
+
 		/* make sure prev is checked first */
 		config_check_cond_cached(srv, con, dc->prev);
+
 		/* one of prev set me to FALSE */
-		if (COND_RESULT_FALSE == con->cond_cache[dc->context_ndx].result) {
-			return COND_RESULT_FALSE;
+		switch (con->cond_cache[dc->context_ndx].result) {
+		case COND_RESULT_FALSE:
+			return con->cond_cache[dc->context_ndx].result;
+		default:
+			break;
 		}
 	}
 
+	if (!con->conditional_is_valid[dc->comp]) {
+		if (con->conf.log_condition_handling) {
+			log_error_write(srv, __FILE__, __LINE__,  "dss", 
+				dc->comp,
+				dc->key->ptr,
+				con->conditional_is_valid[dc->comp] ? "yeah" : "nej");
+		}
+
+		return COND_RESULT_UNSET;
+	}
+
 	/* pass the rules */
 
 	switch (dc->comp) {
@@ -385,6 +422,7 @@ 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 */
@@ -417,32 +455,54 @@ static cond_result_t config_check_cond_cached(server *srv, connection *con, data
 				}
 			}
 		}
+		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_TRUE ? "true" : "false");
+					caches[dc->context_ndx].result == COND_RESULT_UNSET ? "unknown" :
+						(caches[dc->context_ndx].result == COND_RESULT_TRUE ? "true" : "false"));
 		}
 	} 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_TRUE ? "true" : "false");
+					caches[dc->context_ndx].result == COND_RESULT_UNSET ? "unknown" : 
+						(caches[dc->context_ndx].result == COND_RESULT_TRUE ? "true" : "false"));
 		}
 	}
 	return caches[dc->context_ndx].result;
 }
 
+/**
+ * reset the config-cache for a named item
+ *
+ * if the item is COND_LAST_ELEMENT we reset all items
+ */
+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;
+		}
+	}
+}
+
+/**
+ * reset the config cache to its initial state at connection start
+ */
 void config_cond_cache_reset(server *srv, connection *con) {
-#if COND_RESULT_UNSET
 	size_t i;
 
-	for (i = srv->config_context->used - 1; i >= 0; i --) {
-		con->cond_cache[i].result = COND_RESULT_UNSET;
-		con->cond_cache[i].patterncount = 0;
+	config_cond_cache_reset_all_items(srv, con);
+
+	for (i = 0; i < COMP_LAST_ELEMENT; i++) {
+		con->conditional_is_valid[i] = 0;
 	}
-#else
-	memset(con->cond_cache, 0, sizeof(cond_cache_t) * srv->config_context->used);
-#endif
 }
 
 int config_check_cond(server *srv, connection *con, data_config *dc) {

src/configfile.c

@@ -298,6 +298,8 @@ int config_setup_connection(server *srv, connection *con) {
 int config_patch_connection(server *srv, connection *con, comp_key_t comp) {
 	size_t i, j;
 
+	con->conditional_is_valid[comp] = 1;
+
 	/* skip the first, the global context */
 	for (i = 1; i < srv->config_context->used; i++) {
 		data_config *dc = (data_config *)srv->config_context->data[i];

src/configfile.h

@@ -21,4 +21,10 @@ int config_parse_file(server *srv, config_t *context, const char *fn);
 int config_parse_cmd(server *srv, config_t *context, const char *cmd);
 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/mod_extforward.c

@@ -181,17 +181,6 @@ static int mod_extforward_patch_connection(server *srv, connection *con, plugin_
 
 	PATCH(forwarder);
 
-	/* LEM: The purpose of this seems to match extforward configuration
-	        stanzas that are not in the global context, but in some sub-context.
-                I fear this will break contexts of the form HTTP['remote'] = .
-		(in the form that they do not work with the real remote, but matching on
-		the proxy instead).
-
-		I'm not sure this this is all thread-safe. Is the p we are passed different
-		for each connection or is it global?
-
-		mod_fastcgi does the same, so it must be safe.
-	 */
 	/* skip the first, the global context */
 	for (i = 1; i < srv->config_context->used; i++) {
 		data_config *dc = (data_config *)srv->config_context->data[i];
@@ -316,6 +305,8 @@ static void clean_cond_cache(server *srv, connection *con)
 {
 	size_t i;
 
+	log_error_write(srv, __FILE__, __LINE__, "s", "");
+
 	for (i = 0; i < srv->config_context->used; i++) {
 		data_config *dc = (data_config *)srv->config_context->data[i];
 
@@ -457,28 +448,24 @@ URIHANDLER_FUNC(mod_extforward_uri_handler) {
 
 CONNECTION_FUNC(mod_extforward_restore) {
 	plugin_data *p = p_d;
-	UNUSED(srv);
+	handler_ctx *hctx = con->plugin_ctx[p->id];
+
+	if (!hctx) return HANDLER_GO_ON;
+	
+	log_error_write(srv, __FILE__, __LINE__, "s", "");
+
+	con->dst_addr = hctx->saved_remote_addr;
+	buffer_free(con->dst_addr_buf);
+
+	con->dst_addr_buf = hctx->saved_remote_addr_buf;
+	
+	handler_ctx_free(hctx);
+
+	con->plugin_ctx[p->id] = NULL;
+
+	/* Now, clean the conf_cond cache, because we may have changed the results of tests */
+	clean_cond_cache(srv, con);
 
-	/* LEM: This seems completely unuseful, as we are not using
-	        p->conf in this function. Furthermore, it brings a
-	        segfault if one of the conditional configuration
-	        blocks is "SERVER['socket'] == foo", because the
-	        socket is not known yet in the srv/con structure.
-	 */
-	/* mod_extforward_patch_connection(srv, con, p); */
-
-	/* restore this connection's remote ip */
-	if (con->plugin_ctx[p->id]) {
-		handler_ctx *hctx = con->plugin_ctx[p->id];
-		con->dst_addr = hctx->saved_remote_addr;
-		buffer_free(con->dst_addr_buf);
-		con->dst_addr_buf = hctx->saved_remote_addr_buf;
-/* 		log_error_write(srv, __FILE__, __LINE__,"s","LEM: Reset dst_addr_buf"); */
-		handler_ctx_free(hctx);
-		con->plugin_ctx[p->id] = NULL;
-		/* Now, clean the conf_cond cache, because we may have changed the results of tests */
-		clean_cond_cache(srv, con);
-	}
 	return HANDLER_GO_ON;
 }