set tag for :V properly.

Hoist setting of new->tag to before we call fr_pair_value_from_str()

when converting LITERAL to DATA in modcall_fixup_update(), check
for TAG_VALUE, and then parse the tag from the value.  Once that's
done, parse the value with from the remaining part of the string.

Add tests

diff --git a/src/main/map.c b/src/main/map.c
index 09ec8b3a09407307a29137cfcd04e73481028e08..c6eeed410b4688338d01e143a01949e039cce357 100644 (file)
--- a/src/main/map.c
+++ b/src/main/map.c
@@ -302,12 +302,25 @@ int map_afrom_cp(TALLOC_CTX *ctx, vp_map_t **out, CONF_PAIR *cp,
                goto error;
        }
 
-       if (map->rhs->type == TMPL_TYPE_ATTR) {
-               if (map->rhs->tmpl_tag == TAG_VALUE) {
+       /*
+        *      For :V, enforce / verify limits on the RHS.
+        */
+       if ((map->lhs->type == TMPL_TYPE_ATTR) && (map->lhs->tmpl_tag == TAG_VALUE)) {
+               fr_assert(map->lhs->tmpl_da->flags.has_tag);
+
+               switch (map->rhs->type) {
+               case TMPL_TYPE_XLAT_STRUCT:
+               case TMPL_TYPE_XLAT:
+               case TMPL_TYPE_LITERAL:
+                       break;
+
+               default:
                        cf_log_err_cp(cp, "Cannot use ':V' for tags here.");
                        goto error;
                }
+       }
 
+       if (map->rhs->type == TMPL_TYPE_ATTR) {
                /*
                 *      We cannot assign a count to an attribute.  That must
                 *      be done in an xlat.
@@ -845,14 +858,15 @@ int map_to_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, vp_map_t cons
                RDEBUG2("EXPAND %s", map->rhs->name);
                RDEBUG2("   --> %s", str);
 
+               new->op = map->op;
+               new->tag = map->lhs->tmpl_tag;
+
                rcode = fr_pair_value_from_str(new, str, -1);
                talloc_free(str);
                if (rcode < 0) {
                        fr_pair_list_free(&new);
                        goto error;
                }
-               new->op = map->op;
-               new->tag = map->lhs->tmpl_tag;
                *out = new;
                break;
 
@@ -870,14 +884,15 @@ int map_to_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, vp_map_t cons
                        goto error;
                }
 
+               new->op = map->op;
+               new->tag = map->lhs->tmpl_tag;
+
                rcode = fr_pair_value_from_str(new, str, -1);
                talloc_free(str);
                if (rcode < 0) {
                        fr_pair_list_free(&new);
                        goto error;
                }
-               new->op = map->op;
-               new->tag = map->lhs->tmpl_tag;
                *out = new;
                break;
 
@@ -888,12 +903,13 @@ int map_to_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, vp_map_t cons
                new = fr_pair_afrom_da(ctx, map->lhs->tmpl_da);
                if (!new) return -1;
 
+               new->op = map->op;
+               new->tag = map->lhs->tmpl_tag;
+
                if (fr_pair_value_from_str(new, map->rhs->name, -1) < 0) {
                        rcode = 0;
                        goto error;
                }
-               new->op = map->op;
-               new->tag = map->lhs->tmpl_tag;
                *out = new;
                break;
 
@@ -924,6 +940,10 @@ int map_to_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, vp_map_t cons
                                new = fr_pair_afrom_da(ctx, map->lhs->tmpl_da);
                                if (!new) return -1;
 
+                               new->op = map->op;
+                               new->tag = map->lhs->tmpl_tag;
+                               fr_assert(new->tag != TAG_VALUE);
+
                                len = value_data_cast(new, &new->data, new->da->type, new->da,
                                                      vp->da->type, vp->da, &vp->data, vp->vp_length);
                                if (len < 0) {
@@ -941,8 +961,6 @@ int map_to_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, vp_map_t cons
                                        rad_assert(new->vp_strvalue != NULL);
                                }
 
-                               new->op = map->op;
-                               new->tag = map->lhs->tmpl_tag;
                                fr_cursor_insert(&to, new);
                        }
                        return 0;
@@ -952,6 +970,7 @@ int map_to_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, vp_map_t cons
                 *   Otherwise we just need to fixup the attribute types
                 *   and operators
                 */
+               fr_assert(map->lhs->tmpl_tag != TAG_VALUE);
                for (; vp; vp = fr_cursor_next(&from)) {
                        vp->da = map->lhs->tmpl_da;
                        vp->op = map->op;
@@ -969,15 +988,16 @@ int map_to_vp(TALLOC_CTX *ctx, VALUE_PAIR **out, REQUEST *request, vp_map_t cons
                new = fr_pair_afrom_da(ctx, map->lhs->tmpl_da);
                if (!new) return -1;
 
+               new->op = map->op;
+               new->tag = map->lhs->tmpl_tag;
+               fr_assert(new->tag != TAG_VALUE);
+
                len = value_data_copy(new, &new->data, new->da->type, &map->rhs->tmpl_data_value,
                                      map->rhs->tmpl_data_length);
                if (len < 0) goto error;
 
                new->vp_length = len;
-               new->op = map->op;
-               new->tag = map->lhs->tmpl_tag;
                *out = new;
-
                VERIFY_MAP(map);
                break;
 

diff --git a/src/main/modcall.c b/src/main/modcall.c
index 29fc2be1b9912c51affbe05ab1300630008c6dbd..2af860c4819ef36638e6b6f86a66efb281d07511 100644 (file)
--- a/src/main/modcall.c
+++ b/src/main/modcall.c
@@ -1688,9 +1688,54 @@ int modcall_fixup_update(vp_map_t *map, UNUSED void *ctx)
                                }
                                talloc_free(vpt);
 
-                       } else if (tmpl_cast_in_place(map->rhs, map->lhs->tmpl_da->type, map->lhs->tmpl_da) < 0) {
-                               cf_log_err(map->ci, "%s", fr_strerror());
-                               return -1;
+                       } else if (map->lhs->tmpl_tag != TAG_VALUE) {
+                       do_cast:
+                               if (tmpl_cast_in_place(map->rhs, map->lhs->tmpl_da->type, map->lhs->tmpl_da) < 0) {
+                                       cf_log_err(map->ci, "%s", fr_strerror());
+                                       return -1;
+                               }
+
+                       } else if (map->rhs->name[0] != ':') {
+                               map->lhs->tmpl_tag = TAG_NONE; /* no tag, just do the cast */
+                               goto do_cast;
+
+                       } else {
+                               ssize_t ret;
+                               long num;
+                               char const *p = map->rhs->name;
+                               char *end;
+                               vp_tmpl_t *vpt;
+
+                               num = strtol(p + 1, &end, 10);
+                               if ((num > 0x1f) || (num < 0)) {
+                                       cf_log_err(map->ci, "Invalid tag value '%li' (should be between 0-31)", num);
+                                       return -1;
+                               }
+
+                               if (*end && (*end != ':')) {
+                                       cf_log_err(map->ci, "Invalid tag is missing trailing ':'");
+                                       return -1;
+                               }
+
+                               map->lhs->tmpl_tag = num;
+                               vpt = map->rhs;
+                               vpt->tmpl_data_type = map->lhs->tmpl_da->type;
+
+                               /*
+                                *      Just to tmpl_cast_in_place()
+                                *      ourselves, rather than mucking
+                                *      with strings.
+                                */
+                               ret = value_data_from_str(vpt, &vpt->tmpl_data_value,
+                                                         &vpt->tmpl_data_type, map->lhs->tmpl_da,
+                                                         end + 1, strlen(end + 1), '\0');
+                               if (ret < 0) {
+                                       cf_log_err(map->ci, "%s", fr_strerror());
+                                       return -1;
+                               }
+
+                               vpt->type = TMPL_TYPE_DATA;
+                               vpt->tmpl_data_length = (size_t) ret;
                        }
 
                        /*

diff --git a/src/tests/keywords/tag b/src/tests/keywords/tag
new file mode 100644 (file)
index 0000000..5996419
--- /dev/null
+++ b/src/tests/keywords/tag
@@ -0,0 +1,8 @@
+update control {
+       Cleartext-Password := 'hello'
+}
+
+update reply {
+       Tunnel-Assignment-Id:1 = "foo"
+       Tunnel-Assignment-Id:V = ":2:bar"
+}

diff --git a/src/tests/keywords/tag.attrs b/src/tests/keywords/tag.attrs
new file mode 100644 (file)
index 0000000..a7cc86a
--- /dev/null
+++ b/src/tests/keywords/tag.attrs
@@ -0,0 +1,13 @@
+#
+#  Input packet
+#
+User-Name = "bob"
+User-Password = "hello"
+
+#
+#  Expected answer
+#
+Response-Packet-Type == Access-Accept
+Tunnel-Assignment-Id:1 == "foo"
+Tunnel-Assignment-Id:2 == "bar"
+