Deduplicate attribute name resolution code between valuepair.c and evaluate.c

diff --git a/src/include/radiusd.h b/src/include/radiusd.h
index c7911c4b991c95e1b595f04f903ed03b4ed4e831..50992a9f0ccb8537989808f495a65774726b007e 100644 (file)
--- a/src/include/radiusd.h
+++ b/src/include/radiusd.h
@@ -765,11 +765,11 @@ int radius_evaluate_condition(REQUEST *request, int modreturn, int depth,
 int radius_update_attrlist(REQUEST *request, CONF_SECTION *cs,
                           VALUE_PAIR *input_vps, const char *name);
 void radius_pairmove(REQUEST *request, VALUE_PAIR **to, VALUE_PAIR *from);
-int radius_get_vp(REQUEST *request, const char *name, VALUE_PAIR **vp_p);
-int radius_get_vps(REQUEST *request, const char *name,
-       VALUE_PAIR **vps, const char **attr);
-
 
+VALUE_PAIR **radius_list(REQUEST *request, pair_lists_t list);
+pair_lists_t radius_list_name(const char **name, pair_lists_t unknown);
+int radius_ref_request(const char **name, REQUEST **request);
+int radius_get_vp(REQUEST *request, const char *name, VALUE_PAIR **vp_p);
 
 #ifdef WITH_TLS
 /*

diff --git a/src/main/evaluate.c b/src/main/evaluate.c
index 92dd0fad4f0dddb36ba9f65126891c1549259082..cf4a209f8f197094a38902e6f577db0d993560ad 100644 (file)
--- a/src/main/evaluate.c
+++ b/src/main/evaluate.c
@@ -1124,85 +1124,34 @@ int radius_update_attrlist(REQUEST *request, CONF_SECTION *cs,
 {
        CONF_ITEM *ci;
        VALUE_PAIR *newlist, *vp;
-       VALUE_PAIR **output_vps = NULL;
-       REQUEST *myrequest = request;
+       VALUE_PAIR **output_vps;
+       
+       int list;
 
        if (!request || !cs) return RLM_MODULE_INVALID;
 
-       /*
-        *      If we are an inner tunnel session, permit the
-        *      policy to update the outer lists directly.
+       /* 
+        *      Qualifiers not valid for this request
         */
-       if (strncmp(name, "outer.", 6) == 0) {
-               if (!request->parent) return RLM_MODULE_NOOP;
-
-               myrequest = request->parent;
-               name += 6;
+       if(!radius_ref_request(&name, &request)){
+               RDEBUG("WARNING: List name refers to outer request"
+                      " but not in a tunnel.");
+               return RLM_MODULE_NOOP; 
        }
-
-       if (strcmp(name, "request") == 0) {
-               output_vps = &myrequest->packet->vps;
-
-       } else if (strcmp(name, "reply") == 0) {
-               output_vps = &myrequest->reply->vps;
-
-#ifdef WITH_PROXY
-       } else if (strcmp(name, "proxy-request") == 0) {
-               if (request->proxy) output_vps = &myrequest->proxy->vps;
-
-       } else if (strcmp(name, "proxy-reply") == 0) {
-               if (request->proxy_reply) output_vps = &request->proxy_reply->vps;
-#endif
-
-       } else if (strcmp(name, "config") == 0) {
-               output_vps = &myrequest->config_items;
-
-       } else if (strcmp(name, "control") == 0) {
-               output_vps = &myrequest->config_items;
-
-#ifdef WITH_COA
-       } else if (strcmp(name, "coa") == 0) {
-               if (!myrequest->coa) {
-                       request_alloc_coa(myrequest);
-                       myrequest->coa->proxy->code = PW_COA_REQUEST;
-               }
-                 
-               if (myrequest->coa &&
-                   (myrequest->coa->proxy->code == PW_COA_REQUEST)) {
-                       output_vps = &myrequest->coa->proxy->vps;
-               }
-
-       } else if (strcmp(name, "coa-reply") == 0) {
-               if (myrequest->coa && /* match reply with request */
-                   (myrequest->coa->proxy->code == PW_COA_REQUEST) &&
-                    (myrequest->coa->proxy_reply)) {
-                     output_vps = &myrequest->coa->proxy_reply->vps;
-               }
-
-       } else if (strcmp(name, "disconnect") == 0) {
-               if (!myrequest->coa) {
-                       request_alloc_coa(myrequest);
-                       if (myrequest->coa) myrequest->coa->proxy->code = PW_DISCONNECT_REQUEST;
-               }
-
-               if (myrequest->coa &&
-                   (myrequest->coa->proxy->code == PW_DISCONNECT_REQUEST)) {
-                       output_vps = &myrequest->coa->proxy->vps;
-               }
-
-       } else if (strcmp(name, "disconnect-reply") == 0) {
-               if (myrequest->coa && /* match reply with request */
-                   (myrequest->coa->proxy->code == PW_DISCONNECT_REQUEST) &&
-                   (myrequest->coa->proxy_reply)) {
-                       output_vps = &myrequest->coa->proxy_reply->vps;
-               }
-#endif
-
-       } else {
+       
+       list = fr_str2int(pair_lists, name, PAIR_LIST_UNKNOWN);
+ 
+       /* 
+        *      Bad list name name
+        */
+       if (list == PAIR_LIST_UNKNOWN) {
+               RDEBUG("ERROR: Invalid list name");
                return RLM_MODULE_INVALID;
        }
-
-       if (!output_vps) return RLM_MODULE_NOOP; /* didn't update the list */
+       
+       output_vps = radius_list(request, list);
+       
+       rad_assert(output_vps);
 
        newlist = paircopy(input_vps);
        if (!newlist) {

diff --git a/src/main/valuepair.c b/src/main/valuepair.c
index 534d82efa685383f73f4b8addbae88a6846e0cf7..1787162311d8092b3152d47ed37d21beae06aabd 100644 (file)
--- a/src/main/valuepair.c
+++ b/src/main/valuepair.c
@@ -854,105 +854,145 @@ void debug_pair_list(VALUE_PAIR *vp)
        fflush(fr_log_fp);
 }
 
-/** @brief Resolve attribute qualifiers to an attribute list
+/** Resolve attribute pair_lists_t value to an attribute list.
+ * 
+ * The value returned is a pointer to the pointer of the HEAD of the list
+ * in the REQUEST. If the head of the list changes, the pointer will still
+ * be valid.
  *
- * @param request The current request
- * @param name Attribute name including qualifiers
- * @param vps Where to write the pointer to resolved list. Will be NULL if list name
- * couldn't be resolved, or is invalid in the current context
- * @param attribute Where to write pointer into name (name minus qualifiers)
- * @return False if the list qualifiers were invalid, else true
+ * @param[in] request containing the target lists.
+ * @param[in] list pair_list_t value to resolve to VALUE_PAIR list.
+ *     Will be NULL if list name couldn't be resolved.
  */
-int radius_get_vps(REQUEST *request, const char *name,
-       VALUE_PAIR **vps, const char **attribute){
-       REQUEST *my_request = request;
-       
-       const char *p = name;
-       const char *q;
-       
-       *vps = NULL;
-       *attribute = p;
-       
-       pair_lists_t list;
-
-       /*
-        *      Allow for tunneled sessions.
-        */
-       if (strncmp(p, "outer.", 6) == 0) {
-               if (!request->parent) return TRUE;
-               p += 6;
-               my_request = request->parent;
-       }
-
-       q = strchr(p, ':');
-       if (q == NULL) {
-               *vps = my_request->packet->vps;
-               *attribute = p;
-               return TRUE;
-       }
-       *attribute = q + 1;
-
-       list = fr_substr2int(pair_lists, p, PAIR_LIST_UNKNOWN, q - p);
+VALUE_PAIR **radius_list(REQUEST *request, pair_lists_t list)
+{      
        switch (list) {
-               case PAIR_LIST_REQUEST:
-                       *vps = my_request->packet->vps;
+               case PAIR_LIST_UNKNOWN:
+               default:
                        break;
 
+               case PAIR_LIST_REQUEST:
+                       return &request->packet->vps;
+
                case PAIR_LIST_REPLY:
-                       *vps = my_request->reply->vps;
-                       break;
+                       return &request->reply->vps;
 
                case PAIR_LIST_CONTROL:
-                       *vps = my_request->config_items;
-                       break;
+                       return &request->config_items;
 
 #ifdef WITH_PROXY
                case PAIR_LIST_PROXY_REQUEST:
-                       *vps = my_request->proxy->vps;
-                       break;
+                       return &request->proxy->vps;
 
                case PAIR_LIST_PROXY_REPLY:
-                       *vps = my_request->proxy_reply->vps;
-                       break;
+                       return &request->proxy_reply->vps;
 #endif
 #ifdef WITH_COA
                case PAIR_LIST_COA:
-                       if (my_request->coa &&
-                          (my_request->coa->proxy->code == PW_COA_REQUEST))
-                               *vps = my_request->coa->proxy->vps;
-                               
+                       if (request->coa &&
+                           (request->coa->proxy->code == PW_COA_REQUEST)) {
+                               return &request->coa->proxy->vps;
+                       }
                        break;
 
                case PAIR_LIST_COA_REPLY:
-                       if (my_request->coa && /* match reply with request */
-                          (my_request->coa->proxy->code == PW_COA_REQUEST) &&
-                           my_request->coa->proxy_reply)
-                               *vps = my_request->coa->proxy_reply->vps;
-                       
+                       if (request->coa && /* match reply with request */
+                           (request->coa->proxy->code == PW_COA_REQUEST) &&
+                           request->coa->proxy_reply) {
+                               return &request->coa->proxy_reply->vps;
+                       }
                        break;
 
                case PAIR_LIST_DM:
-                       if (my_request->coa &&
-                          (my_request->coa->proxy->code == PW_DISCONNECT_REQUEST))
-                               *vps = my_request->coa->proxy->vps;
-                       
+                       if (request->coa &&
+                           (request->coa->proxy->code == PW_DISCONNECT_REQUEST)) {
+                               return &request->coa->proxy->vps;
+                       }
                        break;
 
                case PAIR_LIST_DM_REPLY:
-                       if (my_request->coa && /* match reply with request */
-                          (my_request->coa->proxy->code == PW_DISCONNECT_REQUEST) &&
-                           my_request->coa->proxy_reply)
-                               *vps = my_request->coa->proxy->vps;
+                       if (request->coa && /* match reply with request */
+                           (request->coa->proxy->code == PW_DISCONNECT_REQUEST) &&
+                           request->coa->proxy_reply) {
+                               return &request->coa->proxy->vps;
+                       }
                        break;
 #endif
-               default:
-                       return FALSE;
+       }
+       
+       return NULL;
+}
+
+/** Resolve attribute name to a list.
+ *
+ * Check the name string for qualifiers that specify a list and return
+ * an pair_lists_t value for that list. This value may be passed to
+ * radius_list, along with the current request, to get a pointer to the
+ * actual list in the request.
+ * 
+ * If qualifiers were consumed, write a new pointer into name to the
+ * char after the last qualifier to be consumed.
+ *
+ * radius_list_name should be called before passing a name string that
+ * may contain qualifiers to dict_attrbyname.
+ *
+ * @see dict_attrbyname
+ *
+ * @param[in,out] name of attribute.
+ * @param[in] unknown the list to return if no qualifiers were found.
+ * @return PAIR_LIST_UNKOWN if qualifiers couldn't be resolved to a list.
+ */
+pair_lists_t radius_list_name(const char **name, pair_lists_t unknown)
+{
+       const char *p = *name;
+       char *q;
+       
+       /* This should never be a NULL pointer or zero length string */
+       rad_assert(name && *name);
 
+       q = strrchr(p, ':');
+       if (!q) {
+               return unknown;
        }
-       return TRUE;
+       
+       *name = (q + 1);
+       
+       return fr_substr2int(pair_lists, p, PAIR_LIST_UNKNOWN, (q - p));
+}
+
+/** Resolve attribute name to a request.
+ * 
+ * Check the name string for qualifiers that reference a parent request and
+ * write the pointer to this request to 'request'.
+ *
+ * If qualifiers were consumed, write a new pointer into name to the
+ * char after the last qualifier to be consumed.
+ *
+ * radius_ref_request should be called before radius_list_name.
+ *
+ * @see radius_list_name
+ *
+ * @param[in,out] current request.
+ * @param[in,out] name of attribute.
+ * @return FALSE if qualifiers found but not in a tunnel, else TRUE.
+ */
+int radius_ref_request(const char **name, REQUEST **request)
+{
+       rad_assert(name && *name);
+       rad_assert(request && *request);
+       
+       if (strncmp(*name, "outer.", 6) == 0) {
+               if (!(*request)->parent) {
+                       return FALSE;
+               }
+               *request = (*request)->parent;
+               *name += 6;
        }
+       
+       return TRUE;
+}
 
-/** @brief Return a VP from the specified request
+/** Return a VP from the specified request.
  *
  * @param request The current request
  * @param name Attribute name including qualifiers
@@ -962,25 +1002,38 @@ int radius_get_vps(REQUEST *request, const char *name,
  */
 int radius_get_vp(REQUEST *request, const char *name, VALUE_PAIR **vp_p)
 {
-       const char *attribute;
+       VALUE_PAIR **vps;
+       pair_lists_t list;
        
-       VALUE_PAIR *vps = NULL;
-       DICT_ATTR *da;
+       const DICT_ATTR *da;
        
-       int ret;
-
        *vp_p = NULL;
+       
+       if (!radius_ref_request(&name, &request)) {
+               RDEBUG("WARNING: Attribute name refers to outer request"
+                      " but not in a tunnel.");
+               return TRUE;    /* Discuss, we don't actually know if
+                                  the attrname was valid... */
+       }
+       
+       list = radius_list_name(&name, PAIR_LIST_REQUEST);
+       if (list == PAIR_LIST_UNKNOWN) {
+               RDEBUG("ERROR: Invalid list qualifier");
+               return FALSE;
+       }
+       
+       da = dict_attrbyname(name);
+       if (!da) {
+               RDEBUG("ERROR: Attribute \"%s\" unknown", name);
+               return FALSE;
+       }
 
-       ret = radius_get_vps(request, name, &vps, &attribute);
-       if (!ret) return FALSE; /* not a valid attribute list */
-       if (!vps) return TRUE;  /* list is valid but not in current context */
-
-       da = dict_attrbyname(attribute);
-       if (!da) return FALSE;  /* not a dictionary name */
-
+       vps = radius_list(request, list);
+       rad_assert(vps);
+       
        /*
         *      May not may not be found, but it *is* a known name.
         */
-       *vp_p = pairfind(vps, da->attr, da->vendor);
+       *vp_p = pairfind(*vps, da->attr, da->vendor);
        return TRUE;
 }