From: Alan T. DeKok Date: Sun, 21 Aug 2022 14:23:51 +0000 (-0400) Subject: if we set use_new_conditions, don't even parse the old ones X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c33c4740627bead0db3aa032653ca64bc2892d4a;p=thirdparty%2Ffreeradius-server.git if we set use_new_conditions, don't even parse the old ones and remove the one test which no longer fails at load time. The old code would check if the condition could at all be done, such as assigning a value which is too large. The old code would see that one side had a fixed data type, and try to cast the other side to that data type. The new code does not (as yet) do this --- diff --git a/src/lib/server/cf_file.c b/src/lib/server/cf_file.c index 64c6ef1d448..3ab73fcd327 100644 --- a/src/lib/server/cf_file.c +++ b/src/lib/server/cf_file.c @@ -1469,24 +1469,26 @@ static CONF_ITEM *process_if(cf_stack_t *stack) return NULL; } - my_slen = fr_cond_tokenize(cs, &cond, &t_rules, &FR_SBUFF_IN(buff[3], strlen(buff[3])), false); - if (my_slen <= 0) { - char *spaces, *text; + if (!main_config->use_new_conditions) { + my_slen = fr_cond_tokenize(cs, &cond, &t_rules, &FR_SBUFF_IN(buff[3], strlen(buff[3])), false); + if (my_slen <= 0) { + char *spaces, *text; - ptr = buff[3]; - slen = my_slen; + ptr = buff[3]; + slen = my_slen; - parse_error: - fr_canonicalize_error(cs, &spaces, &text, slen, ptr); + parse_error: + fr_canonicalize_error(cs, &spaces, &text, slen, ptr); - cf_log_err(cs, "Parse error in condition"); - cf_log_err(cs, "%s", text); - cf_log_err(cs, "%s^ %s", spaces, fr_strerror()); + cf_log_err(cs, "Parse error in condition"); + cf_log_err(cs, "%s", text); + cf_log_err(cs, "%s^ %s", spaces, fr_strerror()); - talloc_free(spaces); - talloc_free(text); - talloc_free(cs); - return NULL; + talloc_free(spaces); + talloc_free(text); + talloc_free(cs); + return NULL; + } } if (main_config->parse_new_conditions) { @@ -1516,8 +1518,13 @@ static CONF_ITEM *process_if(cf_stack_t *stack) * Now that the CONF_SECTION and condition are OK, add * the condition to the CONF_SECTION. */ - cf_data_add(cs, cond, NULL, true); - cf_data_add(cs, head, NULL, true); + if (!main_config->use_new_conditions) { + cf_data_add(cs, cond, NULL, true); + } + + if (main_config->parse_new_conditions) { + cf_data_add(cs, head, NULL, true); + } stack->ptr = ptr; cs->allow_unlang = true; diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index 4799580f6d9..83d6d57fd52 100644 --- a/src/lib/unlang/compile.c +++ b/src/lib/unlang/compile.c @@ -2326,7 +2326,8 @@ static unlang_t *compile_children(unlang_group_t *g, unlang_compile_t *unlang_ct case UNLANG_TYPE_ELSIF: case UNLANG_TYPE_IF: was_if = true; - { + + if (!main_config->use_new_conditions) { unlang_group_t *f; unlang_cond_t *gext; @@ -2351,6 +2352,30 @@ static unlang_t *compile_children(unlang_group_t *g, unlang_compile_t *unlang_ct default: break; } + } else { + unlang_group_t *f; + unlang_cond_t *gext; + + /* + * Skip else, and/or omit things which will never be run. + */ + f = unlang_generic_to_group(single); + gext = unlang_group_to_cond(f); + + if (gext->is_truthy) { + if (gext->value) { + skip_else = single->debug_name; + } else { + /* + * The condition never + * matches, so we can + * avoid putting it into + * the unlang tree. + */ + talloc_free(single); + continue; + } + } } break; @@ -3105,9 +3130,9 @@ 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; - xlat_exp_head_t *head; - bool is_truthy, value; + fr_cond_t *cond = NULL; + xlat_exp_head_t *head = NULL; + bool is_truthy = false, value = false; xlat_res_rules_t xr_rules = { .tr_rules = &(tmpl_res_rules_t) { .dict_def = unlang_ctx->rules->attr.dict_def, @@ -3119,9 +3144,6 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan return NULL; } - cond = cf_data_value(cf_data_find(cs, fr_cond_t, NULL)); - fr_assert(cond != NULL); - /* * Migration support. */ @@ -3138,13 +3160,27 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan } is_truthy = xlat_is_truthy(head, &value); + + /* + * If the condition is always false, we don't compile the + * children. + */ + if (main_config->use_new_conditions) { + 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); @@ -3190,6 +3226,7 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan fr_cond_async_update(cond); + do_compile: c = compile_section(parent, unlang_ctx, cs, ext); } if (!c) return NULL; diff --git a/src/lib/unlang/condition.c b/src/lib/unlang/condition.c index e29cc577d0b..537dcab1e5b 100644 --- a/src/lib/unlang/condition.c +++ b/src/lib/unlang/condition.c @@ -77,12 +77,12 @@ 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); - fr_assert(gext->cond != NULL); - /* * 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; @@ -104,6 +104,8 @@ static unlang_action_t unlang_if(rlm_rcode_t *p_result, request_t *request, unla return unlang_group(p_result, request, frame); } + fr_assert(gext->head != NULL); + /* * If we always run this condition, then don't bother pushing anything onto the stack. * diff --git a/src/tests/keywords/integer-overflow-error b/src/tests/keywords/integer-overflow-error.ignore similarity index 100% rename from src/tests/keywords/integer-overflow-error rename to src/tests/keywords/integer-overflow-error.ignore