From: Alan T. DeKok Date: Sun, 27 Aug 2023 21:46:51 +0000 (-0400) Subject: remove use_new_conditions flag, and start hard-coding it X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=77b95a3f229355c722348fdbd9e623c51a24c65d;p=thirdparty%2Ffreeradius-server.git remove use_new_conditions flag, and start hard-coding it the command-line parameter is still accepted for compatibility, but it is ignored. --- diff --git a/src/lib/server/cf_file.c b/src/lib/server/cf_file.c index 222dd6e12b8..b41056dd815 100644 --- a/src/lib/server/cf_file.c +++ b/src/lib/server/cf_file.c @@ -1516,7 +1516,7 @@ static CONF_ITEM *process_if(cf_stack_t *stack) CONF_SECTION *parent = frame->current; char *buff[4]; tmpl_rules_t t_rules; - bool use_new_conditions = main_config_migrate_option_get("use_new_conditions"); + bool use_new_conditions = true; /* * Short names are nicer. diff --git a/src/lib/server/main_config.c b/src/lib/server/main_config.c index 6ba9d06941c..77bc604301a 100644 --- a/src/lib/server/main_config.c +++ b/src/lib/server/main_config.c @@ -194,7 +194,6 @@ static const CONF_PARSER migrate_config[] = { { FR_CONF_OFFSET("flatten_after_decode", FR_TYPE_BOOL | FR_TYPE_HIDDEN, main_config_t, flatten_after_decode), .dflt = "yes" }, { FR_CONF_OFFSET("flatten_before_encode", FR_TYPE_BOOL | FR_TYPE_HIDDEN, main_config_t, flatten_before_encode), .dflt = "yes" }, { FR_CONF_OFFSET("tmpl_tokenize_all_nested", FR_TYPE_BOOL | FR_TYPE_HIDDEN, main_config_t, tmpl_tokenize_all_nested) }, - { FR_CONF_OFFSET("use_new_conditions", FR_TYPE_BOOL | FR_TYPE_HIDDEN, main_config_t, use_new_conditions) }, { FR_CONF_OFFSET("rewrite_update", FR_TYPE_BOOL | FR_TYPE_HIDDEN, main_config_t, rewrite_update) }, { FR_CONF_OFFSET("forbid_update", FR_TYPE_BOOL | FR_TYPE_HIDDEN, main_config_t, forbid_update) }, { FR_CONF_POINTER("pair_legacy_nested", FR_TYPE_BOOL | FR_TYPE_HIDDEN, &fr_pair_legacy_nested), }, @@ -1009,11 +1008,6 @@ main_config_t *main_config_alloc(TALLOC_CTX *ctx) main_config_raddb_dir_set(config, RADDBDIR); main_config_dict_dir_set(config, DICTDIR); - /* - * Force this to be true. - */ - config->use_new_conditions = true; - main_config = config; return config; @@ -1279,7 +1273,6 @@ do {\ * decoding, or before encoding. The code should handle everything correctly. */ if (config->tmpl_tokenize_all_nested) { - config->use_new_conditions = true; config->flatten_after_decode = false; config->flatten_before_encode = false; @@ -1505,7 +1498,6 @@ void main_config_hup(main_config_t *config) } static fr_table_num_ordered_t config_arg_table[] = { - { L("use_new_conditions"), offsetof(main_config_t, use_new_conditions) }, { L("tmpl_tokenize_all_nested"), offsetof(main_config_t, tmpl_tokenize_all_nested) }, { L("rewrite_update"), offsetof(main_config_t, rewrite_update) }, { L("forbid_update"), offsetof(main_config_t, forbid_update) }, @@ -1563,6 +1555,8 @@ bool main_config_migrate_option_get(char const *name) if (!main_config) return false; + if (strcmp(name, "use_new_conditions") == 0) return true; /* ignore this for migration */ + if (strcmp(name, "pair_legacy_nested") == 0) return fr_pair_legacy_nested; offset = fr_table_value_by_substr(config_arg_table, name, strlen(name), 0); diff --git a/src/lib/server/main_config.h b/src/lib/server/main_config.h index 9129b4fd37f..b579936c9a8 100644 --- a/src/lib/server/main_config.h +++ b/src/lib/server/main_config.h @@ -164,7 +164,6 @@ struct main_config_s { bool flatten_after_decode; //!< the worker will call "flatten" after protocol decoding bool flatten_before_encode; //!< the worker will call "flatten" before all encoding bool tmpl_tokenize_all_nested; //!< tmpl_tokenize will create nested tmpls instead of flat ones - bool use_new_conditions; //!< the new xlat expressions will be used for conditions, instead of the old code bool rewrite_update; //!< rewrite "update" to be new edit sections bool forbid_update; //!< forbid "update" sections }; diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index 4ca2ce67813..b81de04a5ae 100644 --- a/src/lib/unlang/compile.c +++ b/src/lib/unlang/compile.c @@ -267,276 +267,6 @@ static bool pass2_fixup_tmpl(TALLOC_CTX *ctx, tmpl_t **vpt_p, CONF_ITEM const *c return true; } -static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci, fr_dict_t const *dict) -{ - tmpl_t *vpt; - map_t *map; - - map = c->data.map; /* shorter */ - - /* - * Auth-Type := foo - * - * Where "foo" is dynamically defined. - */ - if (c->pass2_fixup == PASS2_FIXUP_TYPE) { - if (!fr_dict_enum_by_name(tmpl_attr_tail_da(map->lhs), map->rhs->name, -1)) { - cf_log_err(map->ci, "Invalid reference to non-existent %s %s { ... }", - tmpl_attr_tail_da(map->lhs)->name, - map->rhs->name); - return false; - } - - /* - * These guys can't have a paircmp fixup applied. - */ - c->pass2_fixup = PASS2_FIXUP_NONE; - return true; - } - - if (c->pass2_fixup == PASS2_FIXUP_ATTR) { - /* - * Resolve the attribute references first - */ - if (tmpl_is_attr_unresolved(map->lhs)) { - if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) return false; - } - - if (tmpl_is_attr_unresolved(map->rhs)) { - if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) return false; - } - - c->pass2_fixup = PASS2_FIXUP_NONE; - - /* - * Now that we have known data types for the LHS - * / RHS attribute(s), go check them. - */ - if (fr_cond_promote_types(c, NULL, NULL, NULL, false) < 0) { - cf_log_perr(ci, "Failed parsing condition after dynamic attributes were defined"); - return false; - } - } - - /* - * Just in case someone adds a new fixup later. - */ - fr_assert((c->pass2_fixup == PASS2_FIXUP_NONE) || - (c->pass2_fixup == PASS2_PAIRCOMPARE)); - - /* - * Precompile xlat's - */ - if (tmpl_is_xlat_unresolved(map->lhs)) { - /* - * Compile the LHS to an attribute reference only - * if the RHS is a literal. - * - * @todo - allow anything anywhere. - */ - if (!tmpl_is_unresolved(map->rhs)) { - if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) { - return false; - } - } else { - if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) { - return false; - } - - /* - * Attribute compared to a literal gets - * the literal cast to the data type of - * the attribute. - * - * The code in parser.c did this for - * - * &Attr == data - * - * But now we've just converted "%{Attr}" - * to &Attr, so we've got to do it again. - */ - if (tmpl_is_attr(map->lhs)) { - if ((map->rhs->len > 0) || - (map->op != T_OP_CMP_EQ) || - (tmpl_attr_tail_da(map->lhs)->type == FR_TYPE_STRING) || - (tmpl_attr_tail_da(map->lhs)->type == FR_TYPE_OCTETS)) { - - if (tmpl_cast_in_place(map->rhs, tmpl_attr_tail_da(map->lhs)->type, tmpl_attr_tail_da(map->lhs)) < 0) { - cf_log_err(map->ci, "Failed to parse data type %s from string: %pV", - fr_type_to_str(tmpl_attr_tail_da(map->lhs)->type), - fr_box_strvalue_len(map->rhs->name, map->rhs->len)); - return false; - } /* else the cast was successful */ - - } else { /* RHS is empty, it's just a check for empty / non-empty string */ - vpt = talloc_steal(c, map->lhs); - map->lhs = NULL; - talloc_free(c->data.map); - - /* - * "%{Foo}" == '' ---> !Foo - * "%{Foo}" != '' ---> Foo - */ - c->type = COND_TYPE_TMPL; - c->data.vpt = vpt; - c->negate = !c->negate; - - WARN("%s[%d]: Please change (\"%%{%s}\" %s '') to %c&%s", - cf_filename(cf_item_to_section(ci)), - cf_lineno(cf_item_to_section(ci)), - vpt->name, c->negate ? "==" : "!=", - c->negate ? '!' : ' ', vpt->name); - - /* - * No more RHS, so we can't do more optimizations - */ - return true; - } - } - } - } - - if (tmpl_is_xlat_unresolved(map->rhs)) { - /* - * Convert the RHS to an attribute reference only - * if the LHS is an attribute reference, AND is - * of the same type as the RHS. - * - * We can fix this when the code in evaluate.c - * can handle strings on the LHS, and attributes - * on the RHS. For now, the code in parser.c - * forbids this. - */ - if (tmpl_is_attr(map->lhs)) { - if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) return false; - } else { - if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) return false; - } - } - - if (tmpl_is_exec_unresolved(map->lhs)) { - if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) { - return false; - } - } - - if (tmpl_is_exec_unresolved(map->rhs)) { - if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) { - return false; - } - } - - /* - * Convert bare refs to %{Foreach-Variable-N} - */ - if (tmpl_is_unresolved(map->lhs) && - (strncmp(map->lhs->name, "Foreach-Variable-", 17) == 0)) { - char *fmt; - ssize_t slen; - - fmt = talloc_typed_asprintf(map->lhs, "%%{%s}", map->lhs->name); - slen = tmpl_afrom_substr(map, &vpt, &FR_SBUFF_IN(fmt, talloc_array_length(fmt) - 1), - T_DOUBLE_QUOTED_STRING, - NULL, - &(tmpl_rules_t){ - .attr = { - .list_def = request_attr_request, - .allow_unknown = true - } - }); - if (!vpt) { - char *spaces, *text; - - fr_canonicalize_error(map->ci, &spaces, &text, slen, fr_strerror()); - - cf_log_err(map->ci, "Failed converting %s to xlat", map->lhs->name); - cf_log_err(map->ci, "%s", fmt); - cf_log_err(map->ci, "%s^ %s", spaces, text); - - talloc_free(spaces); - talloc_free(text); - talloc_free(fmt); - - return false; - } - talloc_free(map->lhs); - map->lhs = vpt; - } - -#ifdef HAVE_REGEX - if (tmpl_is_regex_xlat_unresolved(map->rhs)) { - if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) { - return false; - } - } - fr_assert(!tmpl_is_regex_xlat_unresolved(map->lhs)); -#endif - - /* - * Convert &Packet-Type to "%{Packet-Type}", because - * these attributes don't really exist. The code to - * find an attribute reference doesn't work, but the - * xlat code does. - */ - vpt = c->data.map->lhs; - if (tmpl_is_attr(vpt) && tmpl_attr_tail_da(vpt)->flags.virtual) { - if (tmpl_attr_to_xlat(c, &vpt) < 0) return false; - - fr_assert(!tmpl_is_xlat_unresolved(map->lhs)); - } - - /* - * @todo - do the same thing for the RHS... - */ - - /* - * Only attributes can have a paircmp registered, and - * they can only be with the current request_t, and only - * with the request pairs. - */ - if (!tmpl_is_attr(map->lhs) || - !tmpl_request_ref_is_current(tmpl_request(map->lhs)) || - (tmpl_list(map->lhs) != request_attr_request)) { - return true; - } - - if (!paircmp_find(tmpl_attr_tail_da(map->lhs))) return true; - - /* - * It's a pair comparison. Do additional checks. - */ - if (tmpl_contains_regex(map->rhs)) { - cf_log_err(map->ci, "Cannot compare virtual attribute %s via a regex", map->lhs->name); - return false; - } - - if (tmpl_rules_cast(c->data.map->lhs) != FR_TYPE_NULL) { - cf_log_err(map->ci, "Cannot cast virtual attribute %s to %s", map->lhs->name, - tmpl_type_to_str(c->data.map->lhs->type)); - return false; - } - - /* - * Force the RHS to be cast to whatever the LHS da is. - */ - if (tmpl_cast_set(map->rhs, tmpl_attr_tail_da(map->lhs)->type) < 0) { - cf_log_perr(map->ci, "Failed setting rhs type"); - } - - if (map->op != T_OP_CMP_EQ) { - cf_log_err(map->ci, "Must use '==' for comparisons with virtual attribute %s", map->lhs->name); - return false; - } - - /* - * Mark it as requiring a paircmp() call, instead of - * fr_pair_cmp(). - */ - c->pass2_fixup = PASS2_PAIRCOMPARE; - - return true; -} - /** Fixup ONE map (recursively) * * This function resolves most things. Most notable it CAN leave the @@ -2608,32 +2338,7 @@ static unlang_t *compile_children(unlang_group_t *g, unlang_compile_t *unlang_ct case UNLANG_TYPE_IF: was_if = true; - if (!main_config->use_new_conditions) { - unlang_group_t *f; - unlang_cond_t *gext; - - f = unlang_generic_to_group(single); - gext = unlang_group_to_cond(f); - - switch (gext->cond->type) { - case COND_TYPE_TRUE: - skip_else = single->debug_name; - break; - - case COND_TYPE_FALSE: - /* - * The condition never - * matches, so we can - * avoid putting it into - * the unlang tree. - */ - talloc_free(single); - continue; - - default: - break; - } - } else { + { unlang_group_t *f; unlang_cond_t *gext; @@ -3649,7 +3354,6 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan unlang_group_t *g; unlang_cond_t *gext; - fr_cond_t *cond = NULL; xlat_exp_head_t *head = NULL; bool is_truthy = false, value = false; xlat_res_rules_t xr_rules = { @@ -3666,7 +3370,7 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan /* * Migration support. */ - if (main_config->use_new_conditions) { + { char const *name2 = cf_section_name2(cs); ssize_t slen; @@ -3712,74 +3416,29 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan * If the condition is always false, we don't compile the * children. */ - if (is_truthy && !value) goto skip; - - goto do_compile; - } - - cond = cf_data_value(cf_data_find(cs, fr_cond_t, NULL)); - fr_assert(cond != NULL); - - /* - * We still do some resolving of old-style conditions, - * and skipping of sections. - */ - if (cond->type == COND_TYPE_FALSE) { - skip: - cf_log_debug_prefix(cs, "Skipping contents of '%s' as it is always 'false'", - unlang_ops[ext->type].name); - - /* - * Free the children, which frees any xlats, - * conditions, etc. which were defined, but are - * now entirely unused. - * - * However, we still need to cache the conditions, as they will be accessed at run-time. - */ - c = compile_empty(parent, unlang_ctx, cs, ext); - cf_section_free_children(cs); + if (is_truthy && !value) { + cf_log_debug_prefix(cs, "Skipping contents of '%s' as it is always 'false'", + unlang_ops[ext->type].name); - } else { - fr_cond_iter_t iter; - fr_cond_t *leaf; - - for (leaf = fr_cond_iter_init(&iter, cond); - leaf; - leaf = fr_cond_iter_next(&iter)) { - switch (leaf->type) { /* - * Fix up the template. - */ - case COND_TYPE_TMPL: - fr_assert(!tmpl_is_regex_xlat_unresolved(leaf->data.vpt)); - if (!pass2_fixup_tmpl(leaf, &leaf->data.vpt, cf_section_to_item(cs), - unlang_ctx->rules->attr.dict_def)) return false; - break; - - /* - * Fixup the map + * Free the children, which frees any xlats, + * conditions, etc. which were defined, but are + * now entirely unused. + * + * However, we still need to cache the conditions, as they will be accessed at run-time. */ - case COND_TYPE_MAP: - if (!pass2_fixup_cond_map(leaf, cf_section_to_item(cs), - unlang_ctx->rules->attr.dict_def)) return false; - break; - - default: - continue; - } + c = compile_empty(parent, unlang_ctx, cs, ext); + cf_section_free_children(cs); + } else { + c = compile_section(parent, unlang_ctx, cs, ext); } - - fr_cond_async_update(cond); - - do_compile: - c = compile_section(parent, unlang_ctx, cs, ext); } + if (!c) return NULL; fr_assert(c != UNLANG_IGNORE); g = unlang_generic_to_group(c); gext = unlang_group_to_cond(g); - gext->cond = cond; gext->head = head; gext->is_truthy = is_truthy; diff --git a/src/lib/unlang/condition.c b/src/lib/unlang/condition.c index 90bb27d82d3..5b0f6440a07 100644 --- a/src/lib/unlang/condition.c +++ b/src/lib/unlang/condition.c @@ -79,33 +79,6 @@ static unlang_action_t unlang_if(rlm_rcode_t *p_result, request_t *request, unla unlang_cond_t *gext = unlang_group_to_cond(g); unlang_frame_state_cond_t *state = talloc_get_type_abort(frame->state, unlang_frame_state_cond_t); - /* - * Migration support. - */ - if (!main_config->use_new_conditions) { - fr_assert(gext->cond != NULL); - - if (!cond_eval(request, *p_result, gext->cond)) { - RDEBUG2("..."); - return UNLANG_ACTION_EXECUTE_NEXT; - } - - /* - * Tell the main interpreter to skip over the else / - * elsif blocks, as this "if" condition was taken. - */ - while (frame->next && - ((frame->next->type == UNLANG_TYPE_ELSE) || - (frame->next->type == UNLANG_TYPE_ELSIF))) { - frame->next = frame->next->next; - } - - /* - * We took the "if". Go recurse into its' children. - */ - return unlang_group(p_result, request, frame); - } - fr_assert(gext->head != NULL); /* diff --git a/src/lib/unlang/condition_priv.h b/src/lib/unlang/condition_priv.h index 9bd0cf36cd7..a8e8dd23d2c 100644 --- a/src/lib/unlang/condition_priv.h +++ b/src/lib/unlang/condition_priv.h @@ -27,11 +27,9 @@ extern "C" { #endif #include "unlang_priv.h" -#include typedef struct { unlang_group_t group; - fr_cond_t *cond; xlat_exp_head_t *head; bool is_truthy; bool value;