add tests for fr_pair_list_afrom_file()

and update the code to produce more descriptive error messages

diff --git a/src/bin/unit_test_attribute.c b/src/bin/unit_test_attribute.c
index 22ffc410da30c1f743a77acfa3fa82fbc81b9492..45f2ddb71f28e960296e624ab244cb5a600c90ff 100644 (file)
--- a/src/bin/unit_test_attribute.c
+++ b/src/bin/unit_test_attribute.c
@@ -2033,6 +2033,67 @@ static size_t command_encode_raw(command_result_t *result, command_file_ctx_t *c
        RETURN_OK(hex_print(data, COMMAND_OUTPUT_MAX, cc->buffer_start, len));
 }
 
+/** Parse a list of pairs
+ *
+ */
+static size_t command_read_file(command_result_t *result, command_file_ctx_t *cc,
+                           char *data, UNUSED size_t data_used, char *in, UNUSED size_t inlen)
+{
+       ssize_t slen;
+       fr_pair_list_t head;
+       fr_pair_t *vp;
+       bool done = false;
+       char *filename, *p, *end;
+       FILE *fp;
+
+       filename = talloc_asprintf(cc->tmp_ctx, "%s/%s", cc->path, in);
+
+       fp = fopen(filename, "r");
+       talloc_free(filename);
+
+       if (!fp) {
+               fr_strerror_printf("Failed opening %s - %s", in, fr_syserror(errno));
+               RETURN_OK_WITH_ERROR();
+       }
+
+       fr_pair_list_init(&head);
+       slen = fr_pair_list_afrom_file(cc->tmp_ctx, cc->tmpl_rules.attr.dict_def, &head, fp, &done);
+       fclose(fp);
+       if (slen < 0) {
+               RETURN_OK_WITH_ERROR();
+       }
+
+       /*
+        *      Print the pairs.
+        */
+       p = data;
+       end = data + COMMAND_OUTPUT_MAX;
+       for (vp = fr_pair_list_head(&head);
+            vp != NULL;
+            vp = fr_pair_list_next(&head, vp)) {
+               if ((slen = fr_pair_print(&FR_SBUFF_OUT(p, end), NULL, vp)) <= 0) RETURN_OK_WITH_ERROR();
+               p += (size_t)slen;
+
+               if (p >= end) break;
+
+               *(p++) = ',';
+               *(p++) = ' ';
+       }
+
+       /*
+        *      Delete the trailing ", ".
+        */
+       if (p > data) p -= 2;
+       *p = 0;
+
+       if (!done) strlcpy(p, "!DONE", (end - p));
+
+       fr_pair_list_free(&head);
+
+       RETURN_OK(p - data);
+}
+
+
 static size_t command_returned(command_result_t *result, command_file_ctx_t *cc,
                               char *data, UNUSED size_t data_used, UNUSED char *in, UNUSED size_t inlen)
 {
@@ -3134,6 +3195,11 @@ static fr_table_ptr_sorted_t     commands[] = {
                                        .usage = "raw <string>",
                                        .description = "Create nested attributes from OID strings and values"
                                }},
+       { L("read_file "),              &(command_entry_t){
+                                       .func = command_read_file,
+                                       .usage = "read_file <filename>",
+                                       .description = "Read a list of pairs from a file",
+                               }},
        { L("returned"),                &(command_entry_t){
                                        .func = command_returned,
                                        .usage = "returned",

diff --git a/src/lib/util/pair_legacy.c b/src/lib/util/pair_legacy.c
index 039bd757648c216d26acd4d41254fca00d54ae20..7385bc29a23a4630547a42135c6cda071cda4013 100644 (file)
--- a/src/lib/util/pair_legacy.c
+++ b/src/lib/util/pair_legacy.c
@@ -147,7 +147,7 @@ static ssize_t fr_pair_list_afrom_substr(TALLOC_CTX *ctx, fr_dict_attr_t const *
                        p++;
 
                        if (!*relative_vp) {
-                               fr_strerror_const("Relative attributes can only be used immediately after an attribute which has children");
+                               fr_strerror_const("The '.Attribute' syntax can only be used if the previous attribute is structural, and the line ends with ','");
                                goto error;
                        }
 
@@ -161,6 +161,14 @@ static ssize_t fr_pair_list_afrom_substr(TALLOC_CTX *ctx, fr_dict_attr_t const *
                } else {
                        fr_dict_attr_err_t err;
 
+                       /*
+                        *      We can find an attribute from the parent, but if the path is fully specified,
+                        *      then we reset any relative VP.  So that the _next_ line we parse cannot use
+                        *      ".foo = bar" to get a relative attribute which was used when parsing _this_
+                        *      line.
+                        */
+                       *relative_vp = NULL;
+
                        /*
                         *      Parse the name.
                         */
@@ -319,16 +327,6 @@ static ssize_t fr_pair_list_afrom_substr(TALLOC_CTX *ctx, fr_dict_attr_t const *
 
                fr_assert(vp != NULL);
 
-               /*
-                *      If there's a relative VP, and it's not the one
-                *      we just added above, and we're not adding this
-                *      VP to the relative one, then nuke the relative
-                *      VP.
-                */
-               if (*relative_vp && (vp != *relative_vp) && (my_ctx != *relative_vp)) {
-                       *relative_vp = NULL;
-               }
-
                PAIR_VERIFY(vp);
 
                /*
@@ -437,7 +435,7 @@ int fr_pair_list_afrom_file(TALLOC_CTX *ctx, fr_dict_t const *dict, fr_pair_list
                /*
                 *      Call our internal function, instead of the public wrapper.
                 */
-               if (fr_pair_list_afrom_substr(ctx, fr_dict_root(dict), NULL, buf, buf + sizeof(buf), &tmp_list, &last_token, 0, &relative_vp, false) < 0) {
+               if (fr_pair_list_afrom_substr(ctx, fr_dict_root(dict), NULL, buf, buf + strlen(buf), &tmp_list, &last_token, 0, &relative_vp, false) < 0) {
                        goto fail;
                }
 
@@ -449,16 +447,21 @@ int fr_pair_list_afrom_file(TALLOC_CTX *ctx, fr_dict_t const *dict, fr_pair_list
                 *      it's comments.
                 */
                if (!fr_pair_list_num_elements(&tmp_list)) {
-                       if (last_token == T_EOL) break;
-
                        /*
                         *      This is allowed for relative attributes.
                         */
-                       if (relative_vp && (last_token == T_COMMA)) {
-                               found = true;
+                       if (relative_vp) {
+                               if (last_token != T_COMMA) relative_vp = NULL;
                                continue;
                        }
 
+                       /*
+                        *      Blank line by itself, with no relative
+                        *      VP, and no output attributes means
+                        *      that we stop reading the file.
+                        */
+                       if (last_token == T_EOL) break;
+
                fail:
                        /*
                         *      Didn't read anything, but the previous

diff --git a/src/tests/unit/all.mk b/src/tests/unit/all.mk
index 482c20b0c5bdfc0752f4f76a51fb3e6cc33fb171..1f73022b25a14e48d74701bae20f5be4893f67c6 100644 (file)
--- a/src/tests/unit/all.mk
+++ b/src/tests/unit/all.mk
@@ -10,7 +10,7 @@ TEST := test.unit
 #
 #  Get all .txt files
 #
-FILES  := $(call FIND_FILES_SUFFIX,$(DIR),*.txt)
+FILES  := $(filter-out $(DIR)/files/%,$(call FIND_FILES_SUFFIX,$(DIR),*.txt))
 
 #
 #  If we don't have OpenSSL, filter out tests which need TLS.

diff --git a/src/tests/unit/file.txt b/src/tests/unit/file.txt
new file mode 100644 (file)
index 0000000..4031e76
--- /dev/null
+++ b/src/tests/unit/file.txt
@@ -0,0 +1,30 @@
+#
+#  Tests for parsing files ala radclient or "users"
+#
+#  $Id$
+#
+
+proto-dictionary radius
+
+#
+#  Fully specified paths.
+#
+#  @todo - we arguably want to force nesting on these attributes?  Or change the nesting when printed?
+#
+read_file files/cisco_avpair.txt
+match User-Name = "bob", User-Password = "hello", Vendor-Specific.Cisco.AVPair = "1", Vendor-Specific.Cisco.AVPair += "2", Vendor-Specific.Cisco.AVPair += "3", Vendor-Specific.Cisco.AVPair += "4"
+
+#
+#  Relative attributes, all on the same line.
+#
+read_file files/cisco_relative.txt
+match User-Name = "bob", User-Password = "hello", Vendor-Specific.Cisco = { AVPair = "1", AVPair += "2", AVPair += "3", AVPair += "4" }
+
+#
+#  Relative attributes, each on a different line
+#
+read_file files/cisco_multiline_relative.txt
+match User-Name = "bob", User-Password = "hello", Vendor-Specific.Cisco = { AVPair = "1", AVPair += "2", AVPair += "3", AVPair += "4" }
+
+count
+match 7

diff --git a/src/tests/unit/files/cisco_avpair.txt b/src/tests/unit/files/cisco_avpair.txt
new file mode 100644 (file)
index 0000000..cb13d31
--- /dev/null
+++ b/src/tests/unit/files/cisco_avpair.txt
@@ -0,0 +1,6 @@
+User-Name = "bob"
+User-Password = "hello"
+Vendor-Specific.Cisco.AVPair = "1"
+Vendor-Specific.Cisco.AVPair += "2"
+Vendor-Specific.Cisco.AVPair += "3"
+Vendor-Specific.Cisco.AVPair += "4"

diff --git a/src/tests/unit/files/cisco_multiline_relative.txt b/src/tests/unit/files/cisco_multiline_relative.txt
new file mode 100644 (file)
index 0000000..b81e7b4
--- /dev/null
+++ b/src/tests/unit/files/cisco_multiline_relative.txt
@@ -0,0 +1,7 @@
+User-Name = "bob"
+User-Password = "hello"
+Vendor-Specific.Cisco = { AVPair = "1" },
+.AVPair += "2",
+.AVPair += "3",
+.AVPair += "4"
+

diff --git a/src/tests/unit/files/cisco_relative.txt b/src/tests/unit/files/cisco_relative.txt
new file mode 100644 (file)
index 0000000..caaae22
--- /dev/null
+++ b/src/tests/unit/files/cisco_relative.txt
@@ -0,0 +1,3 @@
+User-Name = "bob"
+User-Password = "hello"
+Vendor-Specific.Cisco = { AVPair = "1" }, .AVPair += "2", .AVPair += "3", .AVPair += "4"