From 5ca23c802b939827b429a7fe63578d898ba15d95 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 21 Apr 2018 01:04:36 -0400 Subject: [PATCH] Refactor TR_FILTER using a GPtrArray of filter lines --- common/tr_config_filters.c | 40 +++++++------ common/tr_config_rp_clients.c | 35 ++++++----- common/tr_filter.c | 131 ++++++++++++++++++++++++++++++++---------- common/tr_filter_encoders.c | 31 +++++++++- include/tr_filter.h | 16 ++++-- 5 files changed, 184 insertions(+), 69 deletions(-) diff --git a/common/tr_config_filters.c b/common/tr_config_filters.c index 4035e68..5be737f 100644 --- a/common/tr_config_filters.c +++ b/common/tr_config_filters.c @@ -98,6 +98,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR { TALLOC_CTX *tmp_ctx = talloc_new(NULL); TR_FILTER *filt = NULL; + TR_FLINE *fline = NULL; json_t *jfaction = NULL; json_t *jfline = NULL; json_t *jfspecs = NULL; @@ -125,13 +126,6 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR } tr_filter_set_type(filt, ftype); - /* make sure we have space to represent the filter */ - if (json_array_size(jfilt) > TR_MAX_FILTER_LINES) { - tr_err("tr_cfg_parse_one_filter: Filter has too many lines, maximum of %d.", TR_MAX_FILTER_LINES); - *rc = TR_CFG_NOPARSE; - goto cleanup; - } - /* For each entry in the filter... */ json_array_foreach(jfilt, i, jfline) { if ((NULL == (jfaction = json_object_get(jfline, "action"))) || @@ -155,16 +149,17 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR goto cleanup; } - if (NULL == (filt->lines[i] = tr_fline_new(filt))) { + fline = tr_fline_new(tmp_ctx); + if (fline == NULL) { tr_debug("tr_cfg_parse_one_filter: Out of memory allocating filter line %d.", i + 1); *rc = TR_CFG_NOMEM; goto cleanup; } if (!strcmp(json_string_value(jfaction), "accept")) { - filt->lines[i]->action = TR_FILTER_ACTION_ACCEPT; + fline->action = TR_FILTER_ACTION_ACCEPT; } else if (!strcmp(json_string_value(jfaction), "reject")) { - filt->lines[i]->action = TR_FILTER_ACTION_REJECT; + fline->action = TR_FILTER_ACTION_REJECT; } else { tr_debug("tr_cfg_parse_one_filter: Error parsing filter action, unknown action' %s'.", json_string_value(jfaction)); @@ -184,7 +179,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR goto cleanup; } else if (json_array_size(jrc) > 0) { /* ok we actually have entries to process */ - if (NULL == (filt->lines[i]->realm_cons = tr_cfg_parse_one_constraint(filt->lines[i], "realm", jrc, rc))) { + if (NULL == (fline->realm_cons = tr_cfg_parse_one_constraint(fline, "realm", jrc, rc))) { tr_debug("tr_cfg_parse_one_filter: Error parsing realm constraint"); *rc = TR_CFG_NOPARSE; goto cleanup; @@ -203,7 +198,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR *rc = TR_CFG_NOPARSE; goto cleanup; } else if (json_array_size(jdc) > 0) { - if (NULL == (filt->lines[i]->domain_cons = tr_cfg_parse_one_constraint(filt->lines[i], "domain", jdc, rc))) { + if (NULL == (fline->domain_cons = tr_cfg_parse_one_constraint(fline, "domain", jdc, rc))) { tr_debug("tr_cfg_parse_one_filter: Error parsing domain constraint"); *rc = TR_CFG_NOPARSE; goto cleanup; @@ -239,14 +234,14 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR } /* allocate the filter spec */ - if (NULL == (filt->lines[i]->specs[j] = tr_fspec_new(filt->lines[i]))) { + if (NULL == (fline->specs[j] = tr_fspec_new(fline))) { tr_debug("tr_cfg_parse_one_filter: Out of memory."); *rc = TR_CFG_NOMEM; goto cleanup; } /* fill in the field */ - if (NULL == (filt->lines[i]->specs[j]->field = tr_new_name(json_string_value(jfield)))) { + if (NULL == (fline->specs[j]->field = tr_new_name(json_string_value(jfield)))) { tr_debug("tr_cfg_parse_one_filter: Out of memory."); *rc = TR_CFG_NOMEM; goto cleanup; @@ -259,7 +254,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR *rc = TR_CFG_NOMEM; goto cleanup; } - tr_fspec_add_match(filt->lines[i]->specs[j], name); + tr_fspec_add_match(fline->specs[j], name); } else { /* jmatch is an array (we checked earlier) */ json_array_foreach(jmatch, k, this_jmatch) { @@ -268,19 +263,26 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR *rc = TR_CFG_NOMEM; goto cleanup; } - tr_fspec_add_match(filt->lines[i]->specs[j], name); + tr_fspec_add_match(fline->specs[j], name); } } - if (!tr_filter_validate_spec_field(ftype, filt->lines[i]->specs[j])){ + if (!tr_filter_validate_spec_field(ftype, fline->specs[j])){ tr_debug("tr_cfg_parse_one_filter: Invalid filter field \"%.*s\" for %s filter, spec %d, filter %d.", - filt->lines[i]->specs[j]->field->len, - filt->lines[i]->specs[j]->field->buf, + fline->specs[j]->field->len, + fline->specs[j]->field->buf, tr_filter_type_to_string(filt->type), i, j); *rc = TR_CFG_ERROR; goto cleanup; } } + + if (NULL == tr_filter_add_line(filt, fline)) { + tr_debug("tr_cfg_parse_one_filter: Error adding line %d for %s filter", + i+1, tr_filter_type_to_string(filt->type)); + *rc = TR_CFG_NOMEM; + goto cleanup; + } } /* check that the filter is valid */ diff --git a/common/tr_config_rp_clients.c b/common/tr_config_rp_clients.c index 960edce..3487cb7 100644 --- a/common/tr_config_rp_clients.c +++ b/common/tr_config_rp_clients.c @@ -113,6 +113,7 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm { TALLOC_CTX *tmp_ctx=talloc_new(NULL); TR_FILTER *filt=NULL; + TR_FLINE *fline = NULL; TR_FILTER_SET *filt_set=NULL; TR_CONSTRAINT *cons=NULL; TR_NAME *name=NULL; @@ -147,16 +148,17 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm goto cleanup; } tr_filter_set_type(filt, TR_FILTER_TYPE_TID_INBOUND); - filt->lines[0]=tr_fline_new(filt); - if (filt->lines[0]==NULL) { + + fline = tr_fline_new(tmp_ctx); + if (fline==NULL) { tr_debug("tr_cfg_default_filters: could not allocate filter line."); *rc=TR_CFG_NOMEM; goto cleanup; } - filt->lines[0]->action=TR_FILTER_ACTION_ACCEPT; - filt->lines[0]->specs[0]=tr_fspec_new(filt->lines[0]); - filt->lines[0]->specs[0]->field=n_rp_realm_1; + fline->action=TR_FILTER_ACTION_ACCEPT; + fline->specs[0]=tr_fspec_new(fline); + fline->specs[0]->field=n_rp_realm_1; n_rp_realm_1=NULL; /* we don't own this name any more */ name=tr_dup_name(realm); @@ -165,12 +167,12 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm *rc=TR_CFG_NOMEM; goto cleanup; } - tr_fspec_add_match(filt->lines[0]->specs[0], name); + tr_fspec_add_match(fline->specs[0], name); name=NULL; /* we no longer own the name */ /* now do the wildcard name */ - filt->lines[0]->specs[1]=tr_fspec_new(filt->lines[0]); - filt->lines[0]->specs[1]->field=n_rp_realm_2; + fline->specs[1]=tr_fspec_new(fline); + fline->specs[1]->field=n_rp_realm_2; n_rp_realm_2=NULL; /* we don't own this name any more */ if (NULL==(name=tr_name_cat(n_prefix, realm))) { @@ -179,11 +181,11 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm goto cleanup; } - tr_fspec_add_match(filt->lines[0]->specs[1], name); + tr_fspec_add_match(fline->specs[1], name); name=NULL; /* we no longer own the name */ /* domain constraint */ - if (NULL==(cons=tr_constraint_new(filt->lines[0]))) { + if (NULL==(cons=tr_constraint_new(fline))) { tr_debug("tr_cfg_default_filters: could not allocate domain constraint."); *rc=TR_CFG_NOMEM; goto cleanup; @@ -206,11 +208,11 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm } cons->matches[1]=name; name=NULL; - filt->lines[0]->domain_cons=cons; + fline->domain_cons=cons; /* realm constraint */ - if (NULL==(cons=tr_constraint_new(filt->lines[0]))) { + if (NULL==(cons=tr_constraint_new(fline))) { tr_debug("tr_cfg_default_filters: could not allocate realm constraint."); *rc=TR_CFG_NOMEM; goto cleanup; @@ -233,7 +235,14 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm } cons->matches[1]=name; name=NULL; - filt->lines[0]->realm_cons=cons; + fline->realm_cons=cons; + + /* put the fline in the filter */ + if (NULL == tr_filter_add_line(filt, fline)) { + tr_debug("tr_cfg_default_filters: could not add line to filter."); + *rc = TR_CFG_NOMEM; + goto cleanup; + } /* put the filter in a set */ filt_set=tr_filter_set_new(tmp_ctx); diff --git a/common/tr_filter.c b/common/tr_filter.c index 52dffb4..529be85 100644 --- a/common/tr_filter.c +++ b/common/tr_filter.c @@ -408,34 +408,34 @@ int tr_filter_apply(TR_FILTER_TARGET *target, TR_CONSTRAINT_SET **constraints, TR_FILTER_ACTION *out_action) { - unsigned int ii=0, jj=0; + TALLOC_CTX *tmp_ctx = talloc_new(NULL); + TR_FILTER_ITER *filt_iter = tr_filter_iter_new(tmp_ctx); + TR_FLINE *this_fline = NULL; + unsigned int jj=0; int retval=TR_FILTER_NO_MATCH; /* Default action is reject */ *out_action = TR_FILTER_ACTION_REJECT; /* Validate filter */ - if ((filt==NULL) || (filt->type==TR_FILTER_TYPE_UNKNOWN)) + if ((filt_iter == NULL) || (filt==NULL) || (filt->type==TR_FILTER_TYPE_UNKNOWN)) { + talloc_free(tmp_ctx); return TR_FILTER_NO_MATCH; + } /* Step through filter lines looking for a match. If a line matches, retval * will be set to TR_FILTER_MATCH, so stop then. */ - for (ii=0, retval=TR_FILTER_NO_MATCH; - iilines[ii]==NULL) - continue; - + this_fline = tr_filter_iter_first(filt_iter, filt); + while(this_fline) { /* Assume we are going to succeed. If any specs fail to match, we'll set * this to TR_FILTER_NO_MATCH. */ retval=TR_FILTER_MATCH; for (jj=0; jjlines[ii]->specs[jj]==NULL) + if (this_fline->specs[jj]==NULL) continue; - if (!tr_fspec_matches(filt->lines[ii]->specs[jj], filt->type, target)) { + if (!tr_fspec_matches(this_fline->specs[jj], filt->type, target)) { retval=TR_FILTER_NO_MATCH; /* set this in case this is the last filter line */ break; /* give up on this filter line */ } @@ -447,11 +447,11 @@ int tr_filter_apply(TR_FILTER_TARGET *target, if (retval==TR_FILTER_MATCH) { /* Matched line ii. Grab its action and constraints. */ - *out_action = filt->lines[ii]->action; + *out_action = this_fline->action; if (constraints!=NULL) { /* if either constraint is missing, these are no-ops */ - tr_constraint_add_to_set(constraints, filt->lines[ii]->realm_cons); - tr_constraint_add_to_set(constraints, filt->lines[ii]->domain_cons); + tr_constraint_add_to_set(constraints, this_fline->realm_cons); + tr_constraint_add_to_set(constraints, this_fline->domain_cons); } } @@ -576,12 +576,14 @@ TR_FLINE *tr_fline_new(TALLOC_CTX *mem_ctx) TR_FILTER *tr_filter_new(TALLOC_CTX *mem_ctx) { TR_FILTER *f = talloc(mem_ctx, TR_FILTER); - int ii = 0; if (f != NULL) { f->type = TR_FILTER_TYPE_UNKNOWN; - for (ii = 0; ii < TR_MAX_FILTER_LINES; ii++) - f->lines[ii] = NULL; + f->lines = g_ptr_array_new(); + if (f->lines == NULL) { + talloc_free(f); + return NULL; + } } return f; } @@ -602,6 +604,65 @@ TR_FILTER_TYPE tr_filter_get_type(TR_FILTER *filt) } /** + * Add a TR_FLINE to a filter + * + * Steals the line into its context on success + * + * @param filt + * @param line + * @return line, or null on failure + */ +TR_FLINE *tr_filter_add_line(TR_FILTER *filt, TR_FLINE *line) +{ + guint old_len = filt->lines->len; + g_ptr_array_add(filt->lines, line); + talloc_steal(filt, line); /* take this no matter what */ + if (old_len == filt->lines->len) + return NULL; /* failed to add */ + return line; +} + +/** + * Iterator for TR_FLINES in a TR_FILTER + * + * @param mem_ctx + * @return + */ +TR_FILTER_ITER *tr_filter_iter_new(TALLOC_CTX *mem_ctx) +{ + TR_FILTER_ITER *iter = talloc(mem_ctx, TR_FILTER_ITER); + if (iter) { + iter->filter = NULL; + } + return iter; +} + +void tr_filter_iter_free(TR_FILTER_ITER *iter) +{ + talloc_free(iter); +} + +TR_FLINE *tr_filter_iter_next(TR_FILTER_ITER *iter) +{ + if (!iter) + return NULL; + + if (iter->ii < iter->filter->lines->len) + return g_ptr_array_index(iter->filter->lines, iter->ii++); + return NULL; +} + +TR_FLINE *tr_filter_iter_first(TR_FILTER_ITER *iter, TR_FILTER *filter) +{ + if (!iter || !filter) + return NULL; + + iter->filter = filter; + iter->ii = 0; + return tr_filter_iter_next(iter); +} + +/** * Check that a filter is valid, i.e., can be processed. * * @param filt Filter to verify @@ -609,10 +670,15 @@ TR_FILTER_TYPE tr_filter_get_type(TR_FILTER *filt) */ int tr_filter_validate(TR_FILTER *filt) { - size_t ii=0, jj=0, kk=0; - - if (!filt) + TALLOC_CTX *tmp_ctx = talloc_new(NULL); + size_t jj=0, kk=0; + TR_FILTER_ITER *filt_iter = tr_filter_iter_new(tmp_ctx); + TR_FLINE *this_fline = NULL; + + if (!filt) { + talloc_free(tmp_ctx); return 0; + } /* check that we recognize the type */ switch(filt->type) { @@ -622,41 +688,48 @@ int tr_filter_validate(TR_FILTER *filt) break; default: + talloc_free(tmp_ctx); return 0; /* if we get here, either TR_FILTER_TYPE_UNKNOWN or an invalid value was found */ } - for (ii=0; iilines[ii]==NULL) - continue; /* an empty filter line is valid */ - + + this_fline = tr_filter_iter_first(filt_iter, filt); + while(this_fline) { /* check that we recognize the action */ - switch(filt->lines[ii]->action) { + switch(this_fline->action) { case TR_FILTER_ACTION_ACCEPT: case TR_FILTER_ACTION_REJECT: break; default: /* if we get here, either TR_FILTER_ACTION_UNKNOWN or an invalid value was found */ + talloc_free(tmp_ctx); return 0; } for (jj=0; jjlines[ii]->specs[jj]==NULL) + if (this_fline->specs[jj]==NULL) continue; /* an empty filter spec is valid */ - if (!tr_filter_validate_spec_field(filt->type, filt->lines[ii]->specs[jj])) + if (!tr_filter_validate_spec_field(filt->type, this_fline->specs[jj])) { + talloc_free(tmp_ctx); return 0; + } /* check that at least one match is non-null */ for (kk=0; kklines[ii]->specs[jj]->match[kk]!=NULL) + if (this_fline->specs[jj]->match[kk]!=NULL) break; } - if (kk==TR_MAX_FILTER_SPEC_MATCHES) + if (kk==TR_MAX_FILTER_SPEC_MATCHES) { + talloc_free(tmp_ctx); return 0; + } } + this_fline = tr_filter_iter_next(filt_iter); } /* We ran the gauntlet. Success! */ + talloc_free(tmp_ctx); return 1; } diff --git a/common/tr_filter_encoders.c b/common/tr_filter_encoders.c index cc22b02..68da1b1 100644 --- a/common/tr_filter_encoders.c +++ b/common/tr_filter_encoders.c @@ -145,6 +145,33 @@ cleanup: return retval; } +static json_t *tr_flines_to_json_array(TR_FILTER *filt) +{ + json_t *jarray = json_array(); + json_t *retval = NULL; + TR_FILTER_ITER *iter = tr_filter_iter_new(NULL); + TR_FLINE *this_fline = NULL; + + if ((jarray == NULL) || (iter == NULL)) + goto cleanup; + + this_fline = tr_filter_iter_first(iter, filt); + while(this_fline) { + ARRAY_APPEND_OR_FAIL(jarray, tr_fline_to_json(this_fline)); + this_fline = tr_filter_iter_next(iter); + } + /* success */ + retval = jarray; + json_incref(retval); + +cleanup: + if (jarray) + json_decref(jarray); + if (iter) + tr_filter_iter_free(iter); + + return retval; +} json_t *tr_filter_set_to_json(TR_FILTER_SET *filter_set) { json_t *fset_json = NULL; @@ -166,9 +193,7 @@ json_t *tr_filter_set_to_json(TR_FILTER_SET *filter_set) filt = tr_filter_set_get(filter_set, *filt_type); if (filt) { OBJECT_SET_OR_FAIL(fset_json, tr_filter_type_to_string(*filt_type), - items_to_json_array((void **)filt->lines, - (ITEM_ENCODER_FUNC *) tr_fline_to_json, - TR_MAX_FILTER_LINES)); + tr_flines_to_json_array(filt)); } } diff --git a/include/tr_filter.h b/include/tr_filter.h index ece3650..caabe5d 100644 --- a/include/tr_filter.h +++ b/include/tr_filter.h @@ -37,14 +37,13 @@ #include #include +#include #include #include #include #include -#define TR_MAX_FILTERS 5 -#define TR_MAX_FILTER_LINES 8 #define TR_MAX_FILTER_SPECS 8 #define TR_MAX_FILTER_SPEC_MATCHES 64 @@ -81,9 +80,13 @@ typedef struct tr_fline { typedef struct tr_filter { TR_FILTER_TYPE type; - TR_FLINE *lines[TR_MAX_FILTER_LINES]; + GPtrArray *lines; } TR_FILTER; +typedef struct tr_filter_iter { + TR_FILTER *filter; + guint ii; +} TR_FILTER_ITER; typedef struct tr_filter_set TR_FILTER_SET; struct tr_filter_set { @@ -109,12 +112,11 @@ int tr_filter_set_add(TR_FILTER_SET *set, TR_FILTER *new); TR_FILTER *tr_filter_set_get(TR_FILTER_SET *set, TR_FILTER_TYPE type); TR_FILTER *tr_filter_new(TALLOC_CTX *mem_ctx); - void tr_filter_free(TR_FILTER *filt); void tr_filter_set_type(TR_FILTER *filt, TR_FILTER_TYPE type); - TR_FILTER_TYPE tr_filter_get_type(TR_FILTER *filt); +TR_FLINE *tr_filter_add_line(TR_FILTER *filt, TR_FLINE *line); TR_FLINE *tr_fline_new(TALLOC_CTX *mem_ctx); @@ -128,6 +130,10 @@ void tr_fspec_add_match(TR_FSPEC *fspec, TR_NAME *match); int tr_fspec_matches(TR_FSPEC *fspec, TR_FILTER_TYPE ftype, TR_FILTER_TARGET *target); +TR_FILTER_ITER *tr_filter_iter_new(TALLOC_CTX *mem_ctx); +void tr_filter_iter_free(TR_FILTER_ITER *iter); +TR_FLINE *tr_filter_iter_first(TR_FILTER_ITER *iter, TR_FILTER *filter); +TR_FLINE *tr_filter_iter_next(TR_FILTER_ITER *iter); /*In tr_constraint.c and exported, but not really a public symbol; needed by tr_filter.c and by tr_constraint.c*/ int TR_EXPORT tr_prefix_wildcard_match(const char *str, const char *wc_str); -- 2.1.4