Use current argument parser for rest xlat, instead of rolling our own weird thing...

diff --git a/src/modules/rlm_rest/rlm_rest.c b/src/modules/rlm_rest/rlm_rest.c
index c2c34245671c474b8e0234e587e042e34f82c611..f7b8571ceaf42506a9b4670a2130ae736322988c 100644 (file)
--- a/src/modules/rlm_rest/rlm_rest.c
+++ b/src/modules/rlm_rest/rlm_rest.c
@@ -46,6 +46,7 @@ RCSID("$Id$")
 #include <freeradius-devel/unlang/xlat.h>
 
 #include <curl/curl.h>
+
 #include <talloc.h>
 
 #include "rest.h"
@@ -496,8 +497,9 @@ finish:
 }
 
 static xlat_arg_parser_t const rest_xlat_args[] = {
+       { .required = true, .single = true, .type = FR_TYPE_STRING },
        { .required = true, .safe_for = CURL_URI_SAFE_FOR, .type = FR_TYPE_STRING },
-       { .variadic = XLAT_ARG_VARIADIC_EMPTY_KEEP, .type = FR_TYPE_STRING },
+       { .concat = true, .type = FR_TYPE_STRING },
        XLAT_ARG_PARSER_TERMINATOR
 };
 
@@ -505,7 +507,7 @@ static xlat_arg_parser_t const rest_xlat_args[] = {
  *
  * Example:
 @verbatim
-%rest(http://example.com/)
+%rest(POST, http://example.com/, "{ \"key\": \"value\" }")
 @endverbatim
  *
  * @ingroup xlat_functions
@@ -520,12 +522,17 @@ static xlat_action_t rest_xlat(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out,
        fr_curl_io_request_t            *randle = NULL;
        int                             ret;
        http_method_t                   method;
-       fr_value_box_t                  *in_vb = fr_value_box_list_pop_head(in), *uri_vb = NULL;
+
+       fr_value_box_t                  *method_vb;
+       fr_value_box_t                  *uri_vb;
+       fr_value_box_t                  *data_vb;
 
        /* There are no configurable parameters other than the URI */
        rlm_rest_xlat_rctx_t            *rctx;
        rlm_rest_section_t              *section;
 
+       XLAT_ARGS(in, &method_vb, &uri_vb, &data_vb);
+
        MEM(rctx = talloc(request, rlm_rest_xlat_rctx_t));
        section = &rctx->section;
 
@@ -534,69 +541,35 @@ static xlat_action_t rest_xlat(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out,
         */
        memcpy(&rctx->section, &inst->xlat, sizeof(*section));
 
-       fr_assert(in_vb->type == FR_TYPE_GROUP);
-
        /*
-        *      If we have more than 1 argument, then the first is the method
+        *      Set the HTTP verb
         */
-       if  ((fr_value_box_list_head(in))) {
-               uri_vb = fr_value_box_list_head(&in_vb->vb_group);
-               if (fr_value_box_list_concat_in_place(uri_vb,
-                                                     uri_vb, &in_vb->vb_group, FR_TYPE_STRING,
-                                                     FR_VALUE_BOX_LIST_FREE, true,
-                                                     SIZE_MAX) < 0) {
-                       REDEBUG("Failed concatenating argument");
-                       return XLAT_ACTION_FAIL;
-               }
-               method = fr_table_value_by_substr(http_method_table, uri_vb->vb_strvalue, -1, REST_HTTP_METHOD_UNKNOWN);
-
-               if (method != REST_HTTP_METHOD_UNKNOWN) {
-                       section->request.method = method;
-               /*
-                *  If the method is unknown, it's a custom verb
-                */
-               } else {
-                       section->request.method = REST_HTTP_METHOD_CUSTOM;
-                       MEM(section->request.method_str = talloc_bstrndup(rctx, uri_vb->vb_strvalue, uri_vb->vb_length));
-               }
-               /*
-                *      Move to next argument
-                */
-               in_vb = fr_value_box_list_pop_head(in);
-       } else {
-               section->request.method = REST_HTTP_METHOD_GET;
-       }
-
+       method = fr_table_value_by_substr(http_method_table, method_vb->vb_strvalue, -1, REST_HTTP_METHOD_UNKNOWN);
+       if (method != REST_HTTP_METHOD_UNKNOWN) {
+               section->request.method = method;
        /*
-        *      We get a connection from the pool here as the CURL object
-        *      is needed to use curl_easy_escape() for escaping
+        *      If the method is unknown, it's a custom verb
         */
-       randle = rctx->handle = rest_slab_reserve(t->slab);
-       if (!randle) return XLAT_ACTION_FAIL;
+       } else {
+               section->request.method = REST_HTTP_METHOD_CUSTOM;
+               MEM(section->request.method_str = talloc_bstrndup(rctx, method_vb->vb_strvalue, method_vb->vb_length));
+       }
 
        /*
-        *      Walk the incoming boxes, assessing where each is in the URI,
-        *      escaping tainted ones where needed.  Following each space in the
-        *      input a new VB group is started.
+        *      Handle URI component escaping
         */
-
-       fr_assert(in_vb->type == FR_TYPE_GROUP);
-
-       randle->request = request;      /* Populate the request pointer for escape callbacks */
-
-       if (fr_uri_escape_list(&in_vb->vb_group, rest_uri_parts, NULL) < 0) {
+       if (fr_uri_escape_list(&uri_vb->vb_group, rest_uri_parts, NULL) < 0) {
                RPEDEBUG("Failed escaping URI");
-
        error:
-               rest_slab_release(randle);
                talloc_free(section);
-
                return XLAT_ACTION_FAIL;
        }
 
-       uri_vb = fr_value_box_list_head(&in_vb->vb_group);
+       /*
+        *      Smush all the URI components together
+        */
        if (fr_value_box_list_concat_in_place(uri_vb,
-                                             uri_vb, &in_vb->vb_group, FR_TYPE_STRING,
+                                             uri_vb, &uri_vb->vb_group, FR_TYPE_STRING,
                                              FR_VALUE_BOX_LIST_FREE, true,
                                              SIZE_MAX) < 0) {
                REDEBUG("Concatenating URI");
@@ -604,18 +577,14 @@ static xlat_action_t rest_xlat(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out,
        }
 
        /*
-        *      Any additional arguments are freeform data
+        *      We get a connection from the pool here as the CURL object
+        *      is needed to use curl_easy_escape() for escaping
         */
-       if ((in_vb = fr_value_box_list_head(in))) {
-               if (fr_value_box_list_concat_in_place(in_vb,
-                                                     in_vb, in, FR_TYPE_STRING,
-                                                     FR_VALUE_BOX_LIST_FREE, true,
-                                                     SIZE_MAX) < 0) {
-                       REDEBUG("Failed to concatenate freeform data");
-                       goto error;
-               }
-               section->request.body = REST_HTTP_BODY_CUSTOM;
-       }
+       randle = rctx->handle = rest_slab_reserve(t->slab);
+       if (!randle) return XLAT_ACTION_FAIL;
+
+       randle->request = request;      /* Populate the request pointer for escape callbacks */
+       if (data_vb) section->request.body = REST_HTTP_BODY_CUSTOM;
 
        RDEBUG2("Sending HTTP %s to \"%pV\"",
               (section->request.method == REST_HTTP_METHOD_CUSTOM) ?
@@ -631,8 +600,12 @@ static xlat_action_t rest_xlat(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out,
        ret = rest_request_config(MODULE_CTX(xctx->mctx->mi, t, xctx->env_data, NULL),
                                  section, request, randle, section->request.method,
                                  section->request.body,
-                                 uri_vb->vb_strvalue, in_vb ? in_vb->vb_strvalue : NULL);
-       if (ret < 0) goto error;
+                                 uri_vb->vb_strvalue, data_vb ? data_vb->vb_strvalue : NULL);
+       if (ret < 0) {
+       error_release:
+               rest_slab_release(randle);
+               goto error;
+       }
 
        /*
         *  Send the CURL request, pre-parse headers, aggregate incoming
@@ -641,7 +614,7 @@ static xlat_action_t rest_xlat(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out,
         * @fixme need to pass in thread to all xlat functions
         */
        ret = fr_curl_io_request_enqueue(t->mhandle, request, randle);
-       if (ret < 0) goto error;
+       if (ret < 0) goto error_release;
 
        return unlang_xlat_yield(request, rest_xlat_resume, rest_io_xlat_signal, ~FR_SIGNAL_CANCEL, rctx);
 }

diff --git a/src/tests/modules/rest/rest_xlat.unlang b/src/tests/modules/rest/rest_xlat.unlang
index 2b70e7295c6790bf89c7fec22d7556c67b2dccfd..339dcdec8d702c351e951fcb732b7e755e0a72a6 100644 (file)
--- a/src/tests/modules/rest/rest_xlat.unlang
+++ b/src/tests/modules/rest/rest_xlat.unlang
@@ -24,14 +24,14 @@ if (!(&result_string == "Sample text response\n")) {
 }
 
 # Take host from incoming packet
-&result_string := %rest("http://%{Login-IP-Host}:%uri.safe(%{server_port})/test.txt")
+&result_string := %rest('GET', "http://%{Login-IP-Host}:%uri.safe(%{server_port})/test.txt")
 
 if (!(&REST-HTTP-Status-Code == 200) || !(&result_string == "Sample text response\n")) {
        test_fail
 }
 
 # Port is not allowed from incoming packets
-&result_string := %rest("http://%{server_host}:%{NAS-Port}/test.txt")
+&result_string := %rest('GET', "http://%{server_host}:%{NAS-Port}/test.txt")
 
 if (!(&Module-Failure-Message == "Failed escaping URI: Unsafe input \"8080\" not allowed in URI part port") || &result_string) {
        test_fail
@@ -160,7 +160,7 @@ group {
 
        &test_string := ""
 
-       map json %rest(http://%{server_host}:%uri.safe(%{server_port})/user/%{User-Name}/reflect/%{test_string}?station=%{User-Name}) {
+       map json %rest('GET', "http://%{server_host}:%uri.safe(%{server_port})/user/%{User-Name}/reflect/%{test_string}?station=%{User-Name}") {
                &headers := '$.reply\.Reply-Message.value[0]'
                &arguments := '$.reply\.Reply-Message.value[1]'
        }
@@ -190,7 +190,7 @@ group {
 group {
        string arguments
 
-       map json %rest("http://%{server_host}:%uri.safe(%{server_port})/user/%{User-Name}/reflect/%{NAS-Identifier}?station=%{Called-Station-Id}") {
+       map json %rest('GET', "http://%{server_host}:%uri.safe(%{server_port})/user/%{User-Name}/reflect/%{NAS-Identifier}?station=%{Called-Station-Id}") {
                &arguments := '$.reply\.Reply-Message.value[1]'
        }
 
@@ -202,7 +202,7 @@ group {
 }
 
 # Test against endpoint which will time out
-&result_string := %restshorttimeout("http://%{server_host}:%uri.safe(%{server_port})/delay")
+&result_string := %restshorttimeout('GET', "http://%{server_host}:%uri.safe(%{server_port})/delay")
 
 if (&REST-HTTP-Status-Code) {
        test_fail