From 33d6e1a011d97c08bb9b45849ef480b2d2311966 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 21 Jul 2023 15:11:31 +0100 Subject: [PATCH] Better input framework error messages for unset non-optionals The input framework currently gives a rather opaque error message when encountering a line in which a required value is not provided. This change updates this behavior; the error message now provides the record element (or the name or the index element) which was not set in the input data, even though it is required to be set by the underlying Zeek type. --- src/input/Manager.cc | 10 +-- .../.stderrwithoutfirstline | 4 +- .../out | 4 +- .../.stderrwithoutfirstline | 4 ++ .../out | 14 ++++ .../base/frameworks/input/invalidtable.zeek | 72 +++++++++++++++++++ 6 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.frameworks.input.invalidtable/.stderrwithoutfirstline create mode 100644 testing/btest/Baseline/scripts.base.frameworks.input.invalidtable/out create mode 100644 testing/btest/scripts/base/frameworks/input/invalidtable.zeek diff --git a/src/input/Manager.cc b/src/input/Manager.cc index dbe8454b23..943b140aaa 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -1104,9 +1104,10 @@ Val* Manager::ValueToIndexVal(const Stream* i, int num_fields, const RecordType* const char* warning = "Skipping input with missing non-optional value"; if ( source && file_pos != -1 ) - Warning(i, "%s:%d: %s", source, file_pos, warning); + Warning(i, "%s:%d: %s (index field %s)", source, file_pos, warning, + type->FieldName(j)); else - Warning(i, "%s", warning); + Warning(i, "%s (index field %s)", warning, type->FieldName(j)); have_error = true; } @@ -1961,9 +1962,10 @@ RecordVal* Manager::ValueToRecordVal(const Stream* stream, const Value* const* v const char* warning = "Skipping input with missing non-optional value"; if ( source && file_pos != -1 ) - Warning(stream, "%s:%d: %s", source, file_pos, warning); + Warning(stream, "%s:%d: %s (record field %s)", source, file_pos, warning, + request_type->FieldName(i)); else - Warning(stream, "%s", warning); + Warning(stream, "%s (record field %s)", warning, request_type->FieldName(i)); have_error = true; } diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/.stderrwithoutfirstline b/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/.stderrwithoutfirstline index c61258df12..16d2cd924a 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/.stderrwithoutfirstline +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/.stderrwithoutfirstline @@ -1,9 +1,9 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -warning: ../input.log:3: Skipping input with missing non-optional value +warning: ../input.log:3: Skipping input with missing non-optional value (record field s) warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Invalid value for subnet: 127.0.0.1 warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Error while reading set or vector warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Could not convert line 'name 127.0.0.1' of ../input.log to Val. Ignoring line. -warning: ../input.log:3: Skipping input with missing non-optional value +warning: ../input.log:3: Skipping input with missing non-optional value (record field s) warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Invalid value for subnet: 127.0.0.1 warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Error while reading set or vector warning: ..<...>/Input::READER_ASCII: ../input.log, line 4: Could not convert line 'name 127.0.0.1' of ../input.log to Val. Ignoring line. diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/out b/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/out index c4077d75dc..fbbf9f0904 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/out +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/out @@ -1,11 +1,11 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. ### NOTE: This file has been sorted with diff-sort. -EventErrorEvent, ../input.log:3: Skipping input with missing non-optional value, Reporter::WARNING +EventErrorEvent, ../input.log:3: Skipping input with missing non-optional value (record field s), Reporter::WARNING EventErrorEvent, Could not convert line 'name\x09127.0.0.1' of ../input.log to Val. Ignoring line., Reporter::WARNING EventErrorEvent, Error while reading set or vector, Reporter::WARNING EventErrorEvent, Invalid value for subnet: 127.0.0.1, Reporter::WARNING -TableErrorEvent, ../input.log:3: Skipping input with missing non-optional value, Reporter::WARNING +TableErrorEvent, ../input.log:3: Skipping input with missing non-optional value (record field s), Reporter::WARNING TableErrorEvent, Could not convert line 'name\x09127.0.0.1' of ../input.log to Val. Ignoring line., Reporter::WARNING TableErrorEvent, Error while reading set or vector, Reporter::WARNING TableErrorEvent, Invalid value for subnet: 127.0.0.1, Reporter::WARNING diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalidtable/.stderrwithoutfirstline b/testing/btest/Baseline/scripts.base.frameworks.input.invalidtable/.stderrwithoutfirstline new file mode 100644 index 0000000000..86d628c373 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalidtable/.stderrwithoutfirstline @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +warning: ../input.log:3: Skipping input with missing non-optional value (index field y) +received termination signal +>>> diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalidtable/out b/testing/btest/Baseline/scripts.base.frameworks.input.invalidtable/out new file mode 100644 index 0000000000..31fff5fd7a --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalidtable/out @@ -0,0 +1,14 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +### NOTE: This file has been sorted with diff-sort. +127.0.0.1 +127.0.0.1 +127.0.0.1 +Event, [s={ +Event, [s={ +TableErrorEvent, ../input.log:3: Skipping input with missing non-optional value (index field y), Reporter::WARNING +[name, name2] = [s={ +{ +} +}] +}] +}] diff --git a/testing/btest/scripts/base/frameworks/input/invalidtable.zeek b/testing/btest/scripts/base/frameworks/input/invalidtable.zeek new file mode 100644 index 0000000000..8a3f90d3b2 --- /dev/null +++ b/testing/btest/scripts/base/frameworks/input/invalidtable.zeek @@ -0,0 +1,72 @@ +# @TEST-EXEC: btest-bg-run zeek zeek -b %INPUT +# @TEST-EXEC: btest-bg-wait 10 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-sort btest-diff out +# @TEST-EXEC: sed 1d .stderr > .stderrwithoutfirstline +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderrwithoutfirstline + +@TEST-START-FILE input.log +#separator \x09 +#fields i y s +name - 127.0.0.1 +name name2 127.0.0.1 +@TEST-END-FILE + +redef exit_only_after_terminate = T; +redef InputAscii::fail_on_invalid_lines = T; + +global outfile: file; + +module A; + +type Idx: record { + i: string; + y: string; +}; + +type Val: record { + s: set[addr]; +}; + +global endcount: count = 0; + +global servers: table[string, string] of Val = table(); + +event handle_our_errors(desc: Input::TableDescription, msg: string, level: Reporter::Level) + { + print outfile, "TableErrorEvent", msg, level; + } + +event handle_our_errors_event(desc: Input::EventDescription, msg: string, level: Reporter::Level) + { + print outfile, "EventErrorEvent", msg, level; + } + +event line(description: Input::EventDescription, tpe: Input::Event, v: Val) + { + print outfile, "Event", v; + } + +event zeek_init() + { + outfile = open("../out"); + # first read in the old stuff into the table... + Input::add_table([$source="../input.log", $name="ssh", $error_ev=handle_our_errors, $idx=Idx, $val=Val, $destination=servers]); + } + +event Input::end_of_data(name: string, source:string) + { + ++endcount; + + # ... and when we're done, move to reading via events. + # This makes the reads sequential, avoiding races in the output. + if ( endcount == 1 ) + { + Input::add_event([$source="../input.log", $name="sshevent", $error_ev=handle_our_errors_event, $fields=Val, $want_record=T, $ev=line]); + } + + if ( endcount == 2 ) + { + print outfile, servers; + terminate(); + } + }