From ad65603ec07499e5eb4e5b8b7b09d5d3937f1396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20B=C3=BChler?= Date: Sun, 21 Feb 2016 18:32:14 +0000 Subject: [PATCH] [core] fix conditional cache handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 git-svn-id: svn://svn.lighttpd.net/lighttpd/branches/lighttpd-1.4.x@3082 152afb58-edef-0310-8abb-c4023f1b3aa9 --- NEWS | 1 + src/array.h | 13 ++-- src/base.h | 22 ++++++- src/configfile-glue.c | 134 +++++++++++++++++++++++++++--------------- src/configfile.h | 3 - src/configparser.y | 2 +- src/data_config.c | 12 ++-- 7 files changed, 121 insertions(+), 66 deletions(-) diff --git a/NEWS b/NEWS index f2af2d64..79b93866 100644 --- a/NEWS +++ b/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) diff --git a/src/array.h b/src/array.h index 84245c73..13751222 100644 --- a/src/array.h +++ b/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); diff --git a/src/base.h b/src/base.h index 4c748a57..b26cc791 100644 --- a/src/base.h +++ b/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 { diff --git a/src/configfile-glue.c b/src/configfile-glue.c index b1d9b6b9..e49aaee9 100644 --- a/src/configfile-glue.c +++ b/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; } - diff --git a/src/configfile.h b/src/configfile.h index f46e869f..17743a07 100644 --- a/src/configfile.h +++ b/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 diff --git a/src/configparser.y b/src/configparser.y index 7f33f05a..b22dac1e 100644 --- a/src/configparser.y +++ b/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"); diff --git a/src/data_config.c b/src/data_config.c index b05ba202..fa666d60 100644 --- a/src/data_config.c +++ b/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;