it's OK for the RHS of an expansion to *not* be an attr ref

We should really do

	&foo := &%{...}

for "RHS is an attr ref".

A reasonable compromise is instead

	&foo := %{...}

because that's a "bare word".  If the user wants a string on the
RHS, he can do

	&foo := "%{...}"

and we won't try to parse the RHS as an attribute

diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index a5c687e2ca25d83451900e6afafba9eeed05c9c8..f2012c598119d49d8df974a294159883c25971d9 100644 (file)
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -87,7 +87,7 @@ static fr_pair_t *edit_list_pair_build(fr_pair_t *parent, fr_dcursor_t *cursor,
 /*
  *  Convert a value-box list to a LHS attribute #tmpl_t
  */
-static int tmpl_attr_from_result(TALLOC_CTX *ctx, edit_result_t *out, request_t *request)
+static int tmpl_attr_from_result(TALLOC_CTX *ctx, map_t const *map, edit_result_t *out, request_t *request)
 {
        ssize_t slen;
        fr_value_box_t *box = fr_value_box_list_head(&out->result);
@@ -97,6 +97,36 @@ static int tmpl_attr_from_result(TALLOC_CTX *ctx, edit_result_t *out, request_t
                return -1;
        }
 
+       /*
+        *      String outputs can be interpreted as attribute references.
+        *
+        *      LHS of an assignment MUST be an attribute.  The RHS
+        *      MAY be an attribute, if the stars align.
+        */
+       if (map && map->rhs) {
+               /*
+                *      &foo := "bar"
+                *
+                *      RHS can't be an attribute.
+                */
+               if (map->rhs->quote != T_BARE_WORD) return 0;
+
+               /*
+                *      &foo := %{bin:0102030005060708}
+                *
+                *      RHS is octets (or other non-string), and can't be an attribute references.
+                */
+               if (box->type != FR_TYPE_STRING) return 0;
+
+               /*
+                *      @todo - arguably the assignment should be something like:
+                *
+                *      &foo := &%{expansion}
+                *
+                *      To say "yes, we really want the RHS to be an attribute reference".
+                */
+       }
+
        /*
         *      Mash all of the results together.
         */
@@ -897,7 +927,7 @@ static int expanded_rhs(request_t *request, unlang_frame_state_edit_t *state, ed
                return check_rhs(request, state, current);
        }
 
-       if (tmpl_attr_from_result(state, &current->rhs, request) < 0) return -1;
+       if (tmpl_attr_from_result(state, map, &current->rhs, request) < 0) return -1;
 
        return check_rhs(request, state, current);
 }
@@ -1248,7 +1278,7 @@ static int expanded_lhs_attribute(request_t *request, unlang_frame_state_edit_t
 {
        REXDENT();
 
-       if (tmpl_attr_from_result(state, &current->lhs, request) < 0) return -1;
+       if (tmpl_attr_from_result(state, NULL, &current->lhs, request) < 0) return -1;
 
        return current->check_lhs(request, state, current);
 }

diff --git a/src/tests/keywords/xlat-dhcpv4 b/src/tests/keywords/xlat-dhcpv4
index e4c218cbeb3d5e54ff20ed95607a595df319cace..e6efb2cdd9a1fa56e30d2911b172515ca1d4628e 100644 (file)
--- a/src/tests/keywords/xlat-dhcpv4
+++ b/src/tests/keywords/xlat-dhcpv4
@@ -7,20 +7,18 @@ if ("%(decode.dhcpv4:%{Tmp-Octets-0})" != 2) {
        test_fail
 }
 
-debug_request
-
-if ( &Relay-Agent-Information.Circuit-Id != 0xabcdef ) {
+if (&Relay-Agent-Information.Circuit-Id != 0xabcdef) {
        test_fail
 }
 
-if ( &Relay-Agent-Information.Remote-Id != 0x010203040506 ) {
+if (&Relay-Agent-Information.Remote-Id != 0x010203040506) {
        test_fail
 }
 
-&Tmp-Octets-1 := "%(encode.dhcpv4:&request[*])"
-if ( &Tmp-Octets-1 != 0x520d0103abcdef0206010203040506 ) {
+&Tmp-Octets-1 := %(encode.dhcpv4:&request)
+
+if !(&Tmp-Octets-1 == 0x520d0103abcdef0206010203040506) {
        test_fail
 }
 
-debug_control
 success