From: Arran Cudbard-Bell Date: Tue, 24 Jan 2017 14:04:17 +0000 (+0000) Subject: Remove stack based variants tmpl_attr functions X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=174d89df5f13b304d45bd8281758f72cd94c1949;p=thirdparty%2Ffreeradius-server.git Remove stack based variants tmpl_attr functions A major limitation of using stack based allocations was that nested templates couldn't be used. This meant syntax like Tmp-Attr-1[&Tmp-Integer-0] wasn't possible, as there was no way to include the structure required for the index reference without allocating it. --- diff --git a/src/include/tmpl.h b/src/include/tmpl.h index 51659763778..2bcb1a9aa5f 100644 --- a/src/include/tmpl.h +++ b/src/include/tmpl.h @@ -40,8 +40,6 @@ * * @see tmpl_afrom_str * @see tmpl_afrom_attr_str - * @see tmpl_from_attr_str - * @see tmpl_from_attr_substr * * In the case of #TMPL_TYPE_ATTR and #TMPL_TYPE_LIST, there are special cursor overlay * functions which can be used to iterate over only the #VALUE_PAIR that match a @@ -144,22 +142,6 @@ typedef enum tmpl_type { extern const FR_NAME_NUMBER tmpl_names[]; -/** Describes a #TMPL_TYPE_ATTR, #TMPL_TYPE_ATTR_UNDEFINED or #TMPL_TYPE_LIST - */ -typedef struct { - request_refs_t request; //!< Request to search or insert in. - pair_lists_t list; //!< List to search or insert in. - - fr_dict_attr_t const *da; //!< Resolved dictionary attribute. - union { - uint8_t da[FR_DICT_ATTR_SIZE]; //!< Unknown dictionary attribute buffer. - uint8_t vendor[FR_DICT_ATTR_SIZE]; //!< Unknown dictionary attribute buffer. - char name[FR_DICT_ATTR_SIZE]; //!< Raw unknown dictionary name. - } unknown; - int num; //!< For array references. - int8_t tag; //!< For tag references. -} value_pair_tmpl_attr_t; - /** A source or sink of value data. * * Is used as both the RHS and LHS of a map (both update, and conditional types) @@ -200,12 +182,20 @@ typedef struct vp_tmpl_t { #endif union { - /* - * Attribute reference. Either an attribute currently in the request - * or an attribute to create. - */ - value_pair_tmpl_attr_t attribute; - + struct { + request_refs_t request; //!< Request to search or insert in. + pair_lists_t list; //!< List to search or insert in. + + fr_dict_attr_t const *da; //!< Resolved dictionary attribute. + union { + uint8_t da[FR_DICT_ATTR_SIZE]; //!< Unknown dictionary attribute buffer. + uint8_t vendor[FR_DICT_ATTR_SIZE]; //!< Unknown dictionary attribute buffer. + char name[FR_DICT_ATTR_SIZE]; //!< Raw unknown dictionary name. + } unknown; + int num; //!< For array references. + int8_t tag; //!< For tag references. + } attribute; + /* * Attribute value. Typically used as the RHS of an update map. */ @@ -227,6 +217,7 @@ typedef struct vp_tmpl_t { #define tmpl_list data.attribute.list #define tmpl_da data.attribute.da #define tmpl_unknown data.attribute.unknown.da +#define tmpl_unknown_vendor data.attribute.unknown.vendor #define tmpl_unknown_name data.attribute.unknown.name #define tmpl_num data.attribute.num #define tmpl_tag data.attribute.tag @@ -398,11 +389,6 @@ void tmpl_from_da(vp_tmpl_t *vpt, fr_dict_attr_t const *da, int8_t tag, int nu int tmpl_afrom_value_box(TALLOC_CTX *ctx, vp_tmpl_t **out, value_box_t *data, bool steal); -ssize_t tmpl_from_attr_str(vp_tmpl_t *vpt, char const *name, - request_refs_t request_def, - pair_lists_t list_def, - bool allow_unknown, bool allow_undefined); - ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, vp_tmpl_t **out, char const *name, request_refs_t request_def, pair_lists_t list_def, bool allow_unknown, bool allow_undefined); diff --git a/src/main/map.c b/src/main/map.c index 944c8c3d6bb..6645fe06df3 100644 --- a/src/main/map.c +++ b/src/main/map.c @@ -1028,19 +1028,21 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t int rcode = 0; int num; VALUE_PAIR **list, *vp, *dst, *head = NULL; - REQUEST *context; + REQUEST *context, *tmp_ctx = NULL; TALLOC_CTX *parent; vp_cursor_t dst_list, src_list; bool found = false; vp_map_t exp_map; - vp_tmpl_t exp_lhs; + vp_tmpl_t *exp_lhs; VERIFY_MAP(map); rad_assert(map->lhs != NULL); rad_assert(map->rhs != NULL); + tmp_ctx = talloc_new(request); + /* * Preprocessing of the LHS of the map. */ @@ -1053,34 +1055,39 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t break; /* - * Everything else gets expanded, then re-parsed as an - * attribute reference. + * Everything else gets expanded, then re-parsed as an attribute reference. + * + * This allows the syntax like: + * - "Attr-%{number}" := "value" */ case TMPL_TYPE_XLAT: case TMPL_TYPE_XLAT_STRUCT: case TMPL_TYPE_EXEC: { - char *attr; + char *attr_str; ssize_t slen; - slen = tmpl_aexpand(request, &attr, request, map->lhs, NULL, NULL); + slen = tmpl_aexpand(request, &attr_str, request, map->lhs, NULL, NULL); if (slen <= 0) { REDEBUG("Left side \"%.*s\" of map failed expansion", (int)map->lhs->len, map->lhs->name); - rad_assert(!attr); - return -1; + rad_assert(!attr_str); + rcode = -1; + goto finish; } - slen = tmpl_from_attr_str(&exp_lhs, attr, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) ; + slen = tmpl_afrom_attr_str(tmp_ctx, &exp_lhs, attr_str, + REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false); if (slen <= 0) { REDEBUG("Left side \"%.*s\" expansion to \"%s\" not an attribute reference: %s", - (int)map->lhs->len, map->lhs->name, attr, fr_strerror()); - talloc_free(attr); - return -1; + (int)map->lhs->len, map->lhs->name, attr_str, fr_strerror()); + talloc_free(attr_str); + rcode = -1; + goto finish; } - rad_assert((exp_lhs.type == TMPL_TYPE_ATTR) || (exp_lhs.type == TMPL_TYPE_LIST)); + rad_assert((exp_lhs->type == TMPL_TYPE_ATTR) || (exp_lhs->type == TMPL_TYPE_LIST)); memcpy(&exp_map, map, sizeof(exp_map)); - exp_map.lhs = &exp_lhs; + exp_map.lhs = exp_lhs; map = &exp_map; } break; @@ -1100,14 +1107,16 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t REDEBUG("Left side \"%.*s\" of map should be an attr or list but is an %s", (int)map->lhs->len, map->lhs->name, fr_int2str(tmpl_names, map->lhs->type, "")); - return -2; + rcode = -2; + goto finish; } context = request; if (radius_request(&context, map->lhs->tmpl_request) < 0) { REDEBUG("Mapping \"%.*s\" -> \"%.*s\" invalid in this context", (int)map->rhs->len, map->rhs->name, (int)map->lhs->len, map->lhs->name); - return -2; + rcode = -2; + goto finish; } /* @@ -1118,7 +1127,8 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t (map->lhs->tmpl_list == PAIR_LIST_DM)) && !request->coa) { if (!request_alloc_coa(context)) { REDEBUG("Failed to create a CoA/Disconnect Request message"); - return -2; + rcode = -2; + goto finish; } context->coa->proxy->packet->code = (map->lhs->tmpl_list == PAIR_LIST_COA) ? PW_CODE_COA_REQUEST : @@ -1129,8 +1139,8 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t if (!list) { REDEBUG("Mapping \"%.*s\" -> \"%.*s\" invalid in this context", (int)map->rhs->len, map->rhs->name, (int)map->lhs->len, map->lhs->name); - - return -2; + rcode = -2; + goto finish; } parent = radius_list_ctx(context, map->lhs->tmpl_list); @@ -1146,11 +1156,11 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t rcode = func(parent, &head, request, map, ctx); if (rcode < 0) { rad_assert(!head); - return rcode; + goto finish; } if (!head) { RDEBUG2("%.*s skipped: No values available", (int)map->lhs->len, map->lhs->name); - return rcode; + goto finish; } } else { if (rad_debug_lvl) map_debug_log(request, map, NULL); @@ -1183,7 +1193,7 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t context->username = NULL; context->password = NULL; } - return 0; + goto finish; case T_OP_SET: if (map->rhs->type == TMPL_TYPE_LIST) { @@ -1197,11 +1207,12 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t fr_pair_list_move(parent, list, &head); fr_pair_list_free(&head); } - goto finish; + goto update; default: fr_pair_list_free(&head); - return -1; + rcode = -1; + goto finish; } } @@ -1234,7 +1245,7 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t case T_OP_CMP_FALSE: /* We don't need the src VPs (should just be 'ANY') */ rad_assert(!head); - if (!dst) return 0; + if (!dst) goto finish; /* * Wildcard: delete all of the matching ones, based on tag. @@ -1255,7 +1266,7 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t * Check that the User-Name and User-Password * caches point to the correct attribute. */ - goto finish; + goto update; /* * -= - Delete attributes in the dst list which match any of the @@ -1271,7 +1282,7 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t /* We didn't find any attributes earlier */ if (!dst) { fr_pair_list_free(&head); - return 0; + goto finish; } /* @@ -1289,9 +1300,10 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t found = true; } } + rcode = 0; fr_pair_list_free(&head); - if (!found) return 0; - goto finish; + if (!found) goto finish; + goto update; } /* @@ -1312,9 +1324,10 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t } } } + rcode = 0; fr_pair_list_free(&head); - if (!found) return 0; - goto finish; + if (!found) goto finish; + goto update; } /* @@ -1336,7 +1349,7 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t if (dst) { RDEBUG3("Refusing to overwrite (use :=)"); fr_pair_list_free(&head); - return 0; + goto finish; } /* Insert first instance (if multiple) */ @@ -1415,10 +1428,11 @@ int map_to_request(REQUEST *request, vp_map_t const *map, radius_map_getvalue_t default: rad_assert(0); /* Should have been caught be the caller */ - return -1; + rcode = -1; + goto finish; } -finish: +update: rad_assert(!head); /* @@ -1458,7 +1472,10 @@ finish: } } } - return 0; + +finish: + talloc_free(tmp_ctx); + return rcode; } /** Check whether the destination of a map is currently valid diff --git a/src/main/pair.c b/src/main/pair.c index 9fde7c00762..9b3e08bf69b 100644 --- a/src/main/pair.c +++ b/src/main/pair.c @@ -821,15 +821,15 @@ void rdebug_proto_pair_list(log_lvl_t level, REQUEST *request, VALUE_PAIR *vp, c int radius_get_vp(VALUE_PAIR **out, REQUEST *request, char const *name) { int rcode; - vp_tmpl_t vpt; + vp_tmpl_t *vpt; *out = NULL; - if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) { - return -4; - } + if (tmpl_afrom_attr_str(request, &vpt, name, + REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) return -4; - rcode = tmpl_find_vp(out, request, &vpt); + rcode = tmpl_find_vp(out, request, vpt); + talloc_free(vpt); return rcode; } @@ -848,15 +848,15 @@ int radius_get_vp(VALUE_PAIR **out, REQUEST *request, char const *name) int radius_copy_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, char const *name) { int rcode; - vp_tmpl_t vpt; + vp_tmpl_t *vpt; *out = NULL; - if (tmpl_from_attr_str(&vpt, name, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) { - return -4; - } + if (tmpl_afrom_attr_str(request, &vpt, name, + REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) return -4; - rcode = tmpl_copy_vps(ctx, out, request, &vpt); + rcode = tmpl_copy_vps(ctx, out, request, vpt); + talloc_free(vpt); return rcode; } diff --git a/src/main/tmpl.c b/src/main/tmpl.c index 552c45a9651..3ef7354d8ea 100644 --- a/src/main/tmpl.c +++ b/src/main/tmpl.c @@ -632,13 +632,10 @@ int tmpl_afrom_value_box(TALLOC_CTX *ctx, vp_tmpl_t **out, value_box_t *data, bo /** Parse a string into a TMPL_TYPE_ATTR_* or #TMPL_TYPE_LIST type #vp_tmpl_t * - * @note The name field is just a copy of the input pointer, if you know that string might be - * freed before you're done with the #vp_tmpl_t use #tmpl_afrom_attr_str - * instead. - * - * @param[out] vpt to modify. + * @param[in,out] ctx to allocate #vp_tmpl_t in. + * @param[out] out Where to write pointer to new #vp_tmpl_t. * @param[in] name of attribute including #request_refs and #pair_lists qualifiers. - * If only #request_refs and #pair_lists qualifiers are found, a #TMPL_TYPE_LIST + * If only #request_refs #pair_lists qualifiers are found, a #TMPL_TYPE_LIST * #vp_tmpl_t will be produced. * @param[in] request_def The default #REQUEST to set if no #request_refs qualifiers are * found in name. @@ -657,51 +654,50 @@ int tmpl_afrom_value_box(TALLOC_CTX *ctx, vp_tmpl_t **out, value_box_t *data, bo * cannot be used to search for a #VALUE_PAIR in a #REQUEST. * @param[in] allow_undefined If true, we don't generate a parse error on unknown attributes. * If an unknown attribute is found a #TMPL_TYPE_ATTR_UNDEFINED #vp_tmpl_t - * will be produced. A #vp_tmpl_t of this type can be passed to - * #tmpl_define_undefined_attr which will add the attribute to the global dictionary, - * and fixup the #vp_tmpl_t, changing it to a #TMPL_TYPE_ATTR with a pointer to the - * new #fr_dict_attr_t. - * @return - * - <= 0 on error (parse failure offset as negative integer). - * - > 0 on success (number of bytes parsed). + * will be produced. + * @return <= 0 on error (offset as negative integer), > 0 on success + * (number of bytes parsed). * * @see REMARKER to produce pretty error markers from the return value. */ -static ssize_t tmpl_from_attr_substr(vp_tmpl_t *vpt, char const *name, - request_refs_t request_def, pair_lists_t list_def, - bool allow_unknown, bool allow_undefined) +ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, vp_tmpl_t **out, char const *name, + request_refs_t request_def, pair_lists_t list_def, + bool allow_unknown, bool allow_undefined) { - char const *p; - long num; - char *q; - tmpl_type_t type = TMPL_TYPE_ATTR; - - value_pair_tmpl_attr_t attr; /* So we don't fill the tmpl with junk and then error out */ + char const *p; + long num; + char *q; + tmpl_type_t type = TMPL_TYPE_ATTR; + ssize_t slen; + vp_tmpl_t *vpt; - memset(vpt, 0, sizeof(*vpt)); - memset(&attr, 0, sizeof(attr)); + MEM(vpt = talloc_zero(ctx, vp_tmpl_t)); /* tmpl_from_attr_substr zeros it */ p = name; if (*p == '&') p++; - p += radius_request_name(&attr.request, p, request_def); - if (attr.request == REQUEST_UNKNOWN) { + p += radius_request_name(&vpt->tmpl_request, p, request_def); + if (vpt->tmpl_request == REQUEST_UNKNOWN) { fr_strerror_printf("Invalid request qualifier"); - return -(p - name); + slen = -(p - name); + error: + talloc_free(vpt); + return slen; } /* * Finding a list qualifier is optional */ - p += radius_list_name(&attr.list, p, list_def); - if (attr.list == PAIR_LIST_UNKNOWN) { + p += radius_list_name(&vpt->tmpl_list, p, list_def); + if (vpt->tmpl_list == PAIR_LIST_UNKNOWN) { fr_strerror_printf("Invalid list qualifier"); - return -(p - name); + slen = -(p - name); + goto error; } - attr.tag = TAG_ANY; - attr.num = NUM_ANY; + vpt->tmpl_tag = TAG_ANY; + vpt->tmpl_num = NUM_ANY; /* * This may be just a bare list, but it can still @@ -720,8 +716,8 @@ static ssize_t tmpl_from_attr_substr(vp_tmpl_t *vpt, char const *name, break; } - attr.da = fr_dict_attr_by_name_substr(NULL, &p); - if (!attr.da) { + vpt->tmpl_da = fr_dict_attr_by_name_substr(NULL, &p); + if (!vpt->tmpl_da) { char const *a; /* @@ -734,8 +730,8 @@ static ssize_t tmpl_from_attr_substr(vp_tmpl_t *vpt, char const *name, /* * Attr-1.2.3.4 is OK. */ - if (fr_dict_unknown_from_suboid(NULL, (fr_dict_attr_t *)&attr.unknown.vendor, - (fr_dict_attr_t *)&attr.unknown.da, fr_dict_root(fr_dict_internal), + if (fr_dict_unknown_from_suboid(NULL, (fr_dict_attr_t *)&vpt->tmpl_unknown_vendor, + (fr_dict_attr_t *)&vpt->tmpl_unknown, fr_dict_root(fr_dict_internal), &p) == 0) { /* * Check what we just parsed really hasn't been defined @@ -744,23 +740,24 @@ static ssize_t tmpl_from_attr_substr(vp_tmpl_t *vpt, char const *name, * If it has, parsing is the same as if the attribute * name had been used instead of its OID. */ - attr.da = fr_dict_attr_by_name(NULL, a); - if (attr.da) { + vpt->tmpl_da = fr_dict_attr_by_name(NULL, a); + if (vpt->tmpl_da) { vpt->auto_converted = true; goto do_num; } if (!allow_unknown) { fr_strerror_printf("Unknown attribute"); - return -(a - name); + slen = -(a - name); + goto error; } /* * Unknown attributes can't be encoded, as we don't * know how to encode them! */ - ((fr_dict_attr_t *)attr.unknown.da)->flags.internal = 1; - attr.da = (fr_dict_attr_t *)&attr.unknown.da; + ((fr_dict_attr_t *)vpt->tmpl_unknown)->flags.internal = 1; + vpt->tmpl_da = (fr_dict_attr_t *)&vpt->tmpl_unknown; goto do_num; /* unknown attributes can't have tags */ } @@ -772,16 +769,21 @@ static ssize_t tmpl_from_attr_substr(vp_tmpl_t *vpt, char const *name, * Don't alter the fr_strerror buffer, should contain the parse * error from fr_dict_unknown_from_suboid. */ - if (!allow_undefined) return -(a - name); + if (!allow_undefined) { + fr_strerror_printf("Undefined attributes not allowed here"); + slen = -(a - name); + goto error; + } /* * Copy the name to a field for later resolution */ type = TMPL_TYPE_ATTR_UNDEFINED; - for (q = attr.unknown.name; fr_dict_attr_allowed_chars[(int) *p]; *q++ = *p++) { - if (q >= (attr.unknown.name + sizeof(attr.unknown.name) - 1)) { + for (q = vpt->tmpl_unknown_name; fr_dict_attr_allowed_chars[(int) *p]; *q++ = *p++) { + if (q >= (vpt->tmpl_unknown_name + sizeof(vpt->tmpl_unknown_name) - 1)) { fr_strerror_printf("Attribute name is too long"); - return -(p - name); + slen = -(p - name); + goto error; } } *q = '\0'; @@ -793,18 +795,20 @@ static ssize_t tmpl_from_attr_substr(vp_tmpl_t *vpt, char const *name, * The string MIGHT have a tag. */ if (*p == ':') { - if (attr.da && !attr.da->flags.has_tag) { /* Lists don't have a da */ - fr_strerror_printf("Attribute '%s' cannot have a tag", attr.da->name); - return -(p - name); + if (vpt->tmpl_da && !vpt->tmpl_da->flags.has_tag) { /* Lists don't have a da */ + fr_strerror_printf("Attribute '%s' cannot have a tag", vpt->tmpl_da->name); + slen = -(p - name); + goto error; } num = strtol(p + 1, &q, 10); if ((num > 0x1f) || (num < 0)) { fr_strerror_printf("Invalid tag value '%li' (should be between 0-31)", num); - return -((p + 1)- name); + slen = -((p + 1)- name); + goto error; } - attr.tag = num; + vpt->tmpl_tag = num; p = q; } @@ -816,17 +820,17 @@ do_num: switch (*p) { case '#': - attr.num = NUM_COUNT; + vpt->tmpl_num = NUM_COUNT; p++; break; case '*': - attr.num = NUM_ALL; + vpt->tmpl_num = NUM_ALL; p++; break; case 'n': - attr.num = NUM_LAST; + vpt->tmpl_num = NUM_LAST; p++; break; @@ -834,28 +838,31 @@ do_num: num = strtol(p, &q, 10); if (p == q) { fr_strerror_printf("Array index is not an integer"); - return -(p - name); + slen = -(p - name); + goto error; } if ((num > 1000) || (num < 0)) { fr_strerror_printf("Invalid array reference '%li' (should be between 0-1000)", num); - return -(p - name); + slen = -(p - name); + goto error; } - attr.num = num; + vpt->tmpl_num = num; p = q; break; } if (*p != ']') { fr_strerror_printf("No closing ']' for array index"); - return -(p - name); + slen = -(p - name); + goto error; } p++; } finish: vpt->type = type; - vpt->name = name; + vpt->name = talloc_strndup(vpt, name, p - name); vpt->len = p - name; vpt->quote = T_BARE_WORD; @@ -863,93 +870,15 @@ finish: * Copy over the attribute definition, now we're * sure what we were passed is valid. */ - memcpy(&vpt->data.attribute, &attr, sizeof(vpt->data.attribute)); - if ((vpt->type == TMPL_TYPE_ATTR) && attr.da->flags.is_unknown) { - vpt->tmpl_da = (fr_dict_attr_t *)&vpt->data.attribute.unknown.da; + if ((vpt->type == TMPL_TYPE_ATTR) && vpt->tmpl_da->flags.is_unknown) { + vpt->tmpl_da = (fr_dict_attr_t *)&vpt->tmpl_unknown; } VERIFY_TMPL(vpt); /* Because we want to ensure we produced something sane */ - return vpt->len; -} - -/** Parse a string into a TMPL_TYPE_ATTR_* or #TMPL_TYPE_LIST type #vp_tmpl_t - * - * @note Unlike #tmpl_from_attr_substr this function will error out if the entire - * name string isn't parsed. - * - * @copydetails tmpl_from_attr_substr - */ -ssize_t tmpl_from_attr_str(vp_tmpl_t *vpt, char const *name, - request_refs_t request_def, pair_lists_t list_def, - bool allow_unknown, bool allow_undefined) -{ - ssize_t slen; - - slen = tmpl_from_attr_substr(vpt, name, request_def, list_def, allow_unknown, allow_undefined); - if (slen <= 0) return slen; - if (name[slen] != '\0') { - /* This looks wrong, but it produces meaningful errors for unknown attrs with tags */ - fr_strerror_printf("Unexpected text after %s", fr_int2str(tmpl_names, vpt->type, "")); - return -slen; - } - - VERIFY_TMPL(vpt); - - return slen; -} - -/** Parse a string into a TMPL_TYPE_ATTR_* or #TMPL_TYPE_LIST type #vp_tmpl_t - * - * @param[in,out] ctx to allocate #vp_tmpl_t in. - * @param[out] out Where to write pointer to new #vp_tmpl_t. - * @param[in] name of attribute including #request_refs and #pair_lists qualifiers. - * If only #request_refs #pair_lists qualifiers are found, a #TMPL_TYPE_LIST - * #vp_tmpl_t will be produced. - * @param[in] request_def The default #REQUEST to set if no #request_refs qualifiers are - * found in name. - * @param[in] list_def The default list to set if no #pair_lists qualifiers are found in - * name. - * @param[in] allow_unknown If true attributes in the format accepted by - * #fr_dict_unknown_from_suboid will be allowed, even if they're not in the main - * dictionaries. - * If an unknown attribute is found a #TMPL_TYPE_ATTR #vp_tmpl_t will be - * produced with the unknown #fr_dict_attr_t stored in the ``unknown.da`` buffer. - * This #fr_dict_attr_t will have its ``flags.is_unknown`` field set to true. - * If #tmpl_from_attr_substr is being called on startup, the #vp_tmpl_t may be - * passed to #tmpl_define_unknown_attr to add the unknown attribute to the main - * dictionary. - * If the unknown attribute is not added to the main dictionary the #vp_tmpl_t - * cannot be used to search for a #VALUE_PAIR in a #REQUEST. - * @param[in] allow_undefined If true, we don't generate a parse error on unknown attributes. - * If an unknown attribute is found a #TMPL_TYPE_ATTR_UNDEFINED #vp_tmpl_t - * will be produced. - * @return <= 0 on error (offset as negative integer), > 0 on success - * (number of bytes parsed). - * - * @see REMARKER to produce pretty error markers from the return value. - */ -ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, vp_tmpl_t **out, char const *name, - request_refs_t request_def, pair_lists_t list_def, - bool allow_unknown, bool allow_undefined) -{ - ssize_t slen; - vp_tmpl_t *vpt; - - MEM(vpt = talloc(ctx, vp_tmpl_t)); /* tmpl_from_attr_substr zeros it */ - - slen = tmpl_from_attr_substr(vpt, name, request_def, list_def, allow_unknown, allow_undefined); - if (slen <= 0) { - TALLOC_FREE(vpt); - return slen; - } - vpt->name = talloc_strndup(vpt, vpt->name, slen); - - VERIFY_TMPL(vpt); - *out = vpt; - return slen; + return vpt->len; } /** Parse a string into a TMPL_TYPE_ATTR_* or #TMPL_TYPE_LIST type #vp_tmpl_t @@ -964,26 +893,17 @@ ssize_t tmpl_afrom_attr_str(TALLOC_CTX *ctx, vp_tmpl_t **out, char const *name, bool allow_unknown, bool allow_undefined) { ssize_t slen; - vp_tmpl_t *vpt; - MEM(vpt = talloc(ctx, vp_tmpl_t)); /* tmpl_from_attr_substr zeros it */ + slen = tmpl_afrom_attr_substr(ctx, out, name, request_def, list_def, allow_unknown, allow_undefined); + if (slen <= 0) return slen; - slen = tmpl_from_attr_substr(vpt, name, request_def, list_def, allow_unknown, allow_undefined); - if (slen <= 0) { - TALLOC_FREE(vpt); - return slen; - } if (name[slen] != '\0') { /* This looks wrong, but it produces meaningful errors for unknown attrs with tags */ - fr_strerror_printf("Unexpected text after %s", fr_int2str(tmpl_names, vpt->type, "")); - TALLOC_FREE(vpt); + fr_strerror_printf("Unexpected text after %s", fr_int2str(tmpl_names, (*out)->type, "")); return -slen; } - vpt->name = talloc_strndup(vpt, vpt->name, vpt->len); - VERIFY_TMPL(vpt); - - *out = vpt; + VERIFY_TMPL(*out); return slen; } @@ -2639,7 +2559,8 @@ void tmpl_verify(char const *file, int line, vp_tmpl_t const *vpt) } if (vpt->tmpl_da != NULL) { - FR_FAULT_LOG("CONSISTENCY CHECK FAILED %s[%u]: TMPL_TYPE_LIST da pointer was NULL", file, line); + FR_FAULT_LOG("CONSISTENCY CHECK FAILED %s[%u]: TMPL_TYPE_LIST da pointer was not NULL. " + "da name was %s", file, line, vpt->tmpl_da->name); if (!fr_cond_assert(0)) fr_exit_now(1); } break; diff --git a/src/main/xlat_func.c b/src/main/xlat_func.c index 63ffcd63358..6a36b1c9ce7 100644 --- a/src/main/xlat_func.c +++ b/src/main/xlat_func.c @@ -326,13 +326,13 @@ static ssize_t xlat_debug_attr(UNUSED TALLOC_CTX *ctx, UNUSED char **out, UNUSED VALUE_PAIR *vp; vp_cursor_t cursor; - vp_tmpl_t vpt; + vp_tmpl_t *vpt; if (!RDEBUG_ENABLED2) return -1; while (isspace((int) *fmt)) fmt++; - if (tmpl_from_attr_str(&vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) { + if (tmpl_afrom_attr_str(request, &vpt, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) <= 0) { RDEBUG("%s", fr_strerror()); return -1; } @@ -340,23 +340,23 @@ static ssize_t xlat_debug_attr(UNUSED TALLOC_CTX *ctx, UNUSED char **out, UNUSED RIDEBUG("Attributes matching \"%s\"", fmt); RINDENT(); - for (vp = tmpl_cursor_init(NULL, &cursor, request, &vpt); + for (vp = tmpl_cursor_init(NULL, &cursor, request, vpt); vp; - vp = tmpl_cursor_next(&cursor, &vpt)) { + vp = tmpl_cursor_next(&cursor, vpt)) { FR_NAME_NUMBER const *type; char *value; value = fr_pair_value_asprint(vp, vp, '\''); if (vp->da->flags.has_tag) { RIDEBUG2("&%s:%s:%i %s %s", - fr_int2str(pair_lists, vpt.tmpl_list, ""), + fr_int2str(pair_lists, vpt->tmpl_list, ""), vp->da->name, vp->tag, fr_int2str(fr_tokens_table, vp->op, ""), value); } else { RIDEBUG2("&%s:%s %s %s", - fr_int2str(pair_lists, vpt.tmpl_list, ""), + fr_int2str(pair_lists, vpt->tmpl_list, ""), vp->da->name, fr_int2str(fr_tokens_table, vp->op, ""), value); @@ -419,6 +419,8 @@ static ssize_t xlat_debug_attr(UNUSED TALLOC_CTX *ctx, UNUSED char **out, UNUSED type++; } } + talloc_free(vpt); + return 0; } diff --git a/src/modules/proto_dhcp/rlm_dhcp.c b/src/modules/proto_dhcp/rlm_dhcp.c index c5a34b1801d..72be01343ae 100644 --- a/src/modules/proto_dhcp/rlm_dhcp.c +++ b/src/modules/proto_dhcp/rlm_dhcp.c @@ -53,36 +53,37 @@ static ssize_t dhcp_options_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outl REQUEST *request, char const *fmt) { vp_cursor_t cursor, src_cursor; - vp_tmpl_t src; + vp_tmpl_t *src; VALUE_PAIR *vp, *head = NULL; int decoded = 0; ssize_t slen; while (isspace((int) *fmt)) fmt++; - slen = tmpl_from_attr_str(&src, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false); + slen = tmpl_afrom_attr_str(request, &src, fmt, REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false); if (slen <= 0) { REMARKER(fmt, slen, fr_strerror()); error: + talloc_free(src); return -1; } - if (src.type != TMPL_TYPE_ATTR) { - REDEBUG("dhcp_options cannot operate on a %s", fr_int2str(tmpl_names, src.type, "")); + if (src->type != TMPL_TYPE_ATTR) { + REDEBUG("dhcp_options cannot operate on a %s", fr_int2str(tmpl_names, src->type, "")); goto error; } - if (src.tmpl_da->type != PW_TYPE_OCTETS) { + if (src->tmpl_da->type != PW_TYPE_OCTETS) { REDEBUG("dhcp_options got a %s attribute needed octets", - fr_int2str(dict_attr_types, src.tmpl_da->type, "")); + fr_int2str(dict_attr_types, src->tmpl_da->type, "")); goto error; } fr_pair_cursor_init(&cursor, &head); - for (vp = tmpl_cursor_init(NULL, &src_cursor, request, &src); + for (vp = tmpl_cursor_init(NULL, &src_cursor, request, src); vp; - vp = tmpl_cursor_next(&src_cursor, &src)) { + vp = tmpl_cursor_next(&src_cursor, src)) { uint8_t const *p = vp->vp_octets, *end = p + vp->vp_length; ssize_t len; VALUE_PAIR *vps = NULL; @@ -119,6 +120,8 @@ static ssize_t dhcp_options_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outl snprintf(*out, outlen, "%i", decoded); + talloc_free(src); + return strlen(*out); } diff --git a/src/modules/rlm_cache/drivers/rlm_cache_redis/rlm_cache_redis.c b/src/modules/rlm_cache/drivers/rlm_cache_redis/rlm_cache_redis.c index 027ba425297..e1840eb2826 100644 --- a/src/modules/rlm_cache/drivers/rlm_cache_redis/rlm_cache_redis.c +++ b/src/modules/rlm_cache/drivers/rlm_cache_redis/rlm_cache_redis.c @@ -39,8 +39,8 @@ typedef struct rlm_cache_redis { fr_redis_conf_t conf; //!< Connection parameters for the Redis server. //!< Must be first field in this struct. - vp_tmpl_t created_attr; //!< LHS of the Cache-Created map. - vp_tmpl_t expires_attr; //!< LHS of the Cache-Expires map. + vp_tmpl_t *created_attr; //!< LHS of the Cache-Created map. + vp_tmpl_t *expires_attr; //!< LHS of the Cache-Expires map. fr_redis_cluster_t *cluster; } rlm_cache_redis_t; @@ -70,14 +70,14 @@ static int mod_instantiate(rlm_cache_config_t const *config, void *instance, CON /* * These never change, so do it once on instantiation */ - if (tmpl_from_attr_str(&driver->created_attr, "&Cache-Created", - REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) < 0) { + if (tmpl_afrom_attr_str(driver, &driver->created_attr, "&Cache-Created", + REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) < 0) { ERROR("Cache-Created attribute not defined"); return -1; } - if (tmpl_from_attr_str(&driver->expires_attr, "&Cache-Expires", - REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) < 0) { + if (tmpl_afrom_attr_str(driver, &driver->expires_attr, "&Cache-Expires", + REQUEST_CURRENT, PAIR_LIST_REQUEST, false, false) < 0) { ERROR("Cache-Expires attribute not defined"); return -1; } @@ -272,14 +272,14 @@ static cache_status_t cache_entry_insert(UNUSED rlm_cache_config_t const *config vp_tmpl_t expires_value; vp_map_t expires = { .op = T_OP_SET, - .lhs = &driver->expires_attr, + .lhs = driver->expires_attr, .rhs = &expires_value, }; vp_tmpl_t created_value; vp_map_t created = { .op = T_OP_SET, - .lhs = &driver->created_attr, + .lhs = driver->created_attr, .rhs = &created_value, .next = &expires }; diff --git a/src/modules/rlm_python/rlm_python.c b/src/modules/rlm_python/rlm_python.c index 350fa6d95ea..9b2f587d616 100644 --- a/src/modules/rlm_python/rlm_python.c +++ b/src/modules/rlm_python/rlm_python.c @@ -225,14 +225,12 @@ failed: static void mod_vptuple(TALLOC_CTX *ctx, REQUEST *request, VALUE_PAIR **vps, PyObject *pValue, char const *funcname, char const *list_name) { - int i; - int tuplesize; - vp_tmpl_t dst; + int i; + int tuplesize; + vp_tmpl_t *dst; VALUE_PAIR *vp; REQUEST *current = request; - memset(&dst, 0, sizeof(dst)); - /* * If the Python function gave us None for the tuple, * then just return. @@ -300,22 +298,26 @@ static void mod_vptuple(TALLOC_CTX *ctx, REQUEST *request, VALUE_PAIR **vps, PyO } } - if (tmpl_from_attr_str(&dst, s1, REQUEST_CURRENT, PAIR_LIST_REPLY, false, false) <= 0) { + if (tmpl_afrom_attr_str(ctx, &dst, s1, REQUEST_CURRENT, PAIR_LIST_REPLY, false, false) <= 0) { ERROR("%s - Failed to find attribute %s:%s", funcname, list_name, s1); continue; } - if (radius_request(¤t, dst.tmpl_request) < 0) { + if (radius_request(¤t, dst->tmpl_request) < 0) { ERROR("%s - Attribute name %s:%s refers to outer request but not in a tunnel, skipping...", funcname, list_name, s1); + talloc_free(dst); continue; } - if (!(vp = fr_pair_afrom_da(ctx, dst.tmpl_da))) { + vp = fr_pair_afrom_da(ctx, dst->tmpl_da); + talloc_free(dst); + if (!vp) { ERROR("%s - Failed to create attribute %s:%s", funcname, list_name, s1); continue; } + vp->op = op; if (fr_pair_value_from_str(vp, s2, -1) < 0) { DEBUG("%s - Failed: '%s:%s' %s '%s'", funcname, list_name, s1, diff --git a/src/modules/rlm_rest/rest.c b/src/modules/rlm_rest/rest.c index b0decedf711..f8fee740103 100644 --- a/src/modules/rlm_rest/rest.c +++ b/src/modules/rlm_rest/rest.c @@ -1030,7 +1030,8 @@ static int json_pair_make(rlm_rest_t const *instance, rlm_rest_section_t const * REQUEST *request, json_object *object, UNUSED int level, int max) { int max_attrs = max; - + vp_tmpl_t *dst = NULL; + if (!fr_json_object_is_type(object, json_type_object)) { #ifdef HAVE_JSON_TYPE_TO_NAME REDEBUG("Can't process VP container, expected JSON object" @@ -1057,33 +1058,32 @@ static int json_pair_make(rlm_rest_t const *instance, rlm_rest_section_t const * .is_json = 0 }; - vp_tmpl_t dst; REQUEST *current = request; VALUE_PAIR **vps, *vp = NULL; - - memset(&dst, 0, sizeof(dst)); + + TALLOC_FREE(dst); /* * Resolve attribute name to a dictionary entry and pairlist. */ RDEBUG2("Parsing attribute \"%s\"", name); - if (tmpl_from_attr_str(&dst, name, REQUEST_CURRENT, PAIR_LIST_REPLY, false, false) <= 0) { + if (tmpl_afrom_attr_str(request, &dst, name, REQUEST_CURRENT, PAIR_LIST_REPLY, false, false) <= 0) { RWDEBUG("Failed parsing attribute: %s, skipping...", fr_strerror()); continue; } - if (radius_request(¤t, dst.tmpl_request) < 0) { + if (radius_request(¤t, dst->tmpl_request) < 0) { RWDEBUG("Attribute name refers to outer request but not in a tunnel, skipping..."); continue; } - vps = radius_list(current, dst.tmpl_list); + vps = radius_list(current, dst->tmpl_list); if (!vps) { RWDEBUG("List not valid in this context, skipping..."); continue; } - ctx = radius_list_ctx(current, dst.tmpl_list); + ctx = radius_list_ctx(current, dst->tmpl_list); /* * Alternative JSON structure which allows operator, @@ -1181,7 +1181,7 @@ static int json_pair_make(rlm_rest_t const *instance, rlm_rest_section_t const * level + 1, max_attrs);*/ } else { vp = json_pair_make_leaf(instance, section, ctx, request, - dst.tmpl_da, &flags, element); + dst->tmpl_da, &flags, element); if (!vp) continue; } rdebug_pair(2, request, vp, NULL); @@ -1192,6 +1192,8 @@ static int json_pair_make(rlm_rest_t const *instance, rlm_rest_section_t const * */ } while ((++i < elements) && (element = json_object_array_get_idx(value, i))); } + + talloc_free(dst); return max - max_attrs; }