move to "edit" flag for foreach / edits

which does more protection than using immutable, and catches more
cases.

diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index 4f8614eb6685ee073cc26d4c02170699dbb60afe..7d0ea224b8be8de83024a836563cc08f904c278b 100644 (file)
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -389,6 +389,11 @@ static int apply_edits_to_list(request_t *request, unlang_frame_state_edit_t *st
                                continue;
                        }
 
+                       if (vp->vp_edit) {
+                               RWDEBUG("Attribute cannot be removed, as it is being used in a 'foreach' loop - %pP", vp);
+                               continue;
+                       }
+
                        if (fr_edit_list_pair_delete(current->el, list, vp) < 0) {
                                tmpl_dcursor_clear(&cc);
                                return -1;
@@ -509,6 +514,22 @@ done:
        return rcode;
 }
 
+static bool pair_is_editable(request_t *request, fr_pair_t *vp)
+{
+       if (vp->vp_edit) {
+               RWDEBUG("Attribute cannot be removed, as it is being used in a 'foreach' loop - %s", vp->da->name);
+               return false;
+       }
+
+       if (!fr_type_is_structural(vp->vp_type)) return true;
+
+       fr_pair_list_foreach(&vp->vp_group, child) {
+               if (!pair_is_editable(request, child)) return false;
+       }
+
+       return true;
+}
+
 static int edit_delete_lhs(request_t *request, edit_map_t *current, bool delete)
 {
        tmpl_dcursor_ctx_t cc;
@@ -549,7 +570,10 @@ static int edit_delete_lhs(request_t *request, edit_map_t *current, bool delete)
                fr_assert(parent != NULL);
 
                if (!delete) {
+                       if (!pair_is_editable(request, vp)) return -1;
+
                        if (fr_type_is_structural(vp->vp_type)) {
+
                                if (fr_edit_list_free_pair_children(current->el, vp) < 0) return -1;
                        } else {
                                /*
@@ -958,7 +982,7 @@ static int check_rhs(request_t *request, unlang_frame_state_edit_t *state, edit_
            ((tmpl_attr_tail_num(current->lhs.vpt) == NUM_UNSPEC) || (tmpl_attr_tail_num(current->lhs.vpt) > 0) ||
             !current->map->rhs)) {
                if (edit_delete_lhs(request, current,
-                               (tmpl_attr_tail_num(current->lhs.vpt) == NUM_UNSPEC) || !current->map->rhs) < 0) return -1;
+                                   (tmpl_attr_tail_num(current->lhs.vpt) == NUM_UNSPEC) || !current->map->rhs) < 0) return -1;
        }
 
        /*

diff --git a/src/lib/unlang/foreach.c b/src/lib/unlang/foreach.c
index ad79510a4757e3b3adc59ea32ee22970449154ac..ae1abd172f1c138f3a209e97cb6fb630feeca3db 100644 (file)
--- a/src/lib/unlang/foreach.c
+++ b/src/lib/unlang/foreach.c
@@ -166,8 +166,7 @@ static int _free_unlang_frame_state_foreach(unlang_frame_state_foreach_t *state)
                fr_assert(vp != NULL);
 
                do {
-                       if (fr_type_is_leaf(vp->vp_type)) fr_pair_clear_immutable(vp);
-               
+                       vp->vp_edit = false;
                } while ((vp = fr_dcursor_next(&state->cursor)) != NULL);
                tmpl_dcursor_clear(&state->cc);
 
@@ -450,8 +449,13 @@ static unlang_action_t unlang_foreach_attr_init(rlm_rcode_t *p_result, request_t
         *      under us.
         */
        do {
-               if (fr_type_is_leaf(vp->vp_type)) fr_pair_set_immutable(vp);
-               
+               if (vp->vp_edit) {
+                       REDEBUG("Cannot do nested 'foreach' loops over the same attribute %pP", vp);
+                       *p_result = RLM_MODULE_FAIL;
+                       return UNLANG_ACTION_CALCULATE_RESULT;
+               }
+
+               vp->vp_edit = true;
        } while ((vp = fr_dcursor_next(&state->cursor)) != NULL);
        tmpl_dcursor_clear(&state->cc);
 

diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
index 721395c196ed4d2d338759c6270043585e97fcc4..63431d52efb23caa95828c634bbd699140cb2ed1 100644 (file)
--- a/src/lib/util/pair.c
+++ b/src/lib/util/pair.c
@@ -206,9 +206,14 @@ static inline CC_HINT(always_inline) void pair_init_from_da(fr_pair_t *vp, fr_di
                memset(&vp->data, 0xff, sizeof(vp->data));
 #endif
 
+               /*
+                *      Make sure that the pad field is initialized.
+                */
+               if (sizeof(vp->pad)) memset(vp->pad, 0, sizeof(vp->pad));
+
                /*
                 *      Hack around const issues...
-                *      Here again, the orkaround suffices for the compiler but
+                *      Here again, the workaround suffices for the compiler but
                 *      not for Coverity, so again we annotate.
                 */
                /* coverity[store_writes_const_field] */

diff --git a/src/lib/util/pair.h b/src/lib/util/pair.h
index d8ef88666cfa55be3e0f0153d9aa5e20c994de4a..a1aac7ea57dfd2f707225714d8fb3b407bd51d24 100644 (file)
--- a/src/lib/util/pair.h
+++ b/src/lib/util/pair.h
@@ -142,6 +142,7 @@ struct value_pair_s {
 #define vp_type                        data.type
 #define vp_tainted             data.tainted
 #define vp_immutable           data.immutable
+#define vp_edit                        data.edit
 #define vp_raw                 da->flags.is_raw
 
 #define ATTRIBUTE_EQ(_x, _y) ((_x && _y) && (_x->da == _y->da))

diff --git a/src/lib/util/value.h b/src/lib/util/value.h
index 91e3647f41a75a2776738d2a88d4b2c66cb90847..208c0e2541f047fcec3e1cc8f97d683e64f32629 100644 (file)
--- a/src/lib/util/value.h
+++ b/src/lib/util/value.h
@@ -176,6 +176,9 @@ struct value_box_s {
        unsigned int                            secret : 1;             //!< Same as #fr_dict_attr_flags_t secret
        unsigned int                            immutable : 1;          //!< once set, the value cannot be changed
        unsigned int                            talloced : 1;           //!< Talloced, not stack or text allocated.
+
+       unsigned int                            edit : 1;               //!< to control foreach / edits
+
        fr_value_box_safe_for_t _CONST          safe_for;               //!< A unique value to indicate if that value box is safe
                                                                        ///< for consumption by a particular module for a particular
                                                                        ///< purpose.  e.g. LDAP, SQL, etc.