From: Alan T. DeKok Date: Wed, 12 Jul 2023 02:59:43 +0000 (-0400) Subject: add tests for fr_pair_list_afrom_file() X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d5ac986f1a644a29c328b86ec3facc283dc03d0f;p=thirdparty%2Ffreeradius-server.git 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 22ffc410da3..45f2ddb71f2 100644 --- 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 ", .description = "Create nested attributes from OID strings and values" }}, + { L("read_file "), &(command_entry_t){ + .func = command_read_file, + .usage = "read_file ", + .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 039bd757648..7385bc29a23 100644 --- 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 482c20b0c5b..1f73022b25a 100644 --- 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 index 00000000000..4031e76e510 --- /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 index 00000000000..cb13d31087a --- /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 index 00000000000..b81e7b40242 --- /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 index 00000000000..caaae22277a --- /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"