From 937bdccab58864c3d21837304892d91e495e3f6f Mon Sep 17 00:00:00 2001 From: Christian Kreibich Date: Wed, 16 Jun 2021 16:55:28 -0700 Subject: [PATCH] Skip input framework entries with missing but non-optional fields The framework so far populated data structures with missing fields even when those fields are defined without the &optional attribute. When using the attribute, such entries continue to get populated. Update tests to reflect focus on unset fields. --- src/input/Manager.cc | 6 ++ .../events.out | 56 ++++++++++-------- .../preds.out | 26 +++++---- .../servers.out | 8 +-- .../out | 5 +- .../.stderrwithoutfirstline | 2 + .../out | 7 +-- .../input/empty-values-hashing.zeek | 13 +++-- .../base/frameworks/input/emptyvals.zeek | 58 ++++++++++++++++--- 9 files changed, 122 insertions(+), 59 deletions(-) diff --git a/src/input/Manager.cc b/src/input/Manager.cc index e224740eba..8c17933e79 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -1935,6 +1935,12 @@ RecordVal* Manager::ValueToRecordVal(const Stream* stream, const Value* const *v // Better check that it really is optional. Uou never know. assert(request_type->FieldDecl(i)->GetAttr(zeek::detail::ATTR_OPTIONAL)); } + else if ( ! vals[*position]->present && + ! request_type->FieldDecl(i)->GetAttr(zeek::detail::ATTR_OPTIONAL) ) + { + Warning(stream, "Skipping input with missing non-optional value"); + have_error = true; + } else { fieldVal = ValueToVal(stream, vals[*position], request_type->GetFieldType(i).get(), have_error); diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/events.out b/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/events.out index bdd698f119..e76ea0abb8 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/events.out +++ b/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/events.out @@ -5,49 +5,55 @@ Description reader, Input::READER_ASCII mode, Input::REREAD name, ssh - destination[left = 1], [s=, ss=TEST] + destination[left = 3], [s=TEST, ss=] + idx, A::Idx + val, A::Val + want_record, T +Type, Input::EVENT_NEW +Left, [i=3] +Right, [s=TEST, ss=] +============EVENT============ +Description + source, ../input.log + reader, Input::READER_ASCII + mode, Input::REREAD + name, ssh + destination[left = 4], [s=TEST, ss=TEST] + idx, A::Idx + val, A::Val + want_record, T +Type, Input::EVENT_NEW +Left, [i=4] +Right, [s=TEST, ss=TEST] +============EVENT============ +Description + source, ../input.log + reader, Input::READER_ASCII + mode, Input::REREAD + name, ssh + destination[left = 1], [s=TEST2, ss=] idx, A::Idx val, A::Val want_record, T Type, Input::EVENT_NEW Left, [i=1] -Right, [s=, ss=TEST] +Right, [s=TEST2, ss=] ============EVENT============ Description source, ../input.log reader, Input::READER_ASCII mode, Input::REREAD name, ssh - destination[left = 2], [s=, ss=] - idx, A::Idx - val, A::Val - want_record, T -Type, Input::EVENT_NEW -Left, [i=2] -Right, [s=, ss=] -============EVENT============ -Description - source, ../input.log - reader, Input::READER_ASCII - mode, Input::REREAD - name, ssh - destination[left = 1], [s=TEST, ss=] + destination[left = 4], [s=TEST2, ss=TEST2] idx, A::Idx val, A::Val want_record, T Type, Input::EVENT_CHANGED -Left, [i=1] -Right, [s=, ss=TEST] +Left, [i=4] +Right, [s=TEST, ss=TEST] ============EVENT============ Description source, ../input.log reader, Input::READER_ASCII mode, Input::REREAD name, ssh - destination[left = 2], [s=TEST, ss=TEST] - idx, A::Idx - val, A::Val - want_record, T -Type, Input::EVENT_CHANGED -Left, [i=2] -Right, [s=, ss=] diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/preds.out b/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/preds.out index 537ed439f2..69a8217f8f 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/preds.out +++ b/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/preds.out @@ -1,17 +1,21 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. ============PREDICATE============ Input::EVENT_NEW -[i=1] -[s=, ss=TEST] -============PREDICATE============ -Input::EVENT_NEW -[i=2] -[s=, ss=] -============PREDICATE============ -Input::EVENT_CHANGED -[i=1] +[i=3] [s=TEST, ss=] ============PREDICATE============ -Input::EVENT_CHANGED -[i=2] +Input::EVENT_NEW +[i=4] [s=TEST, ss=TEST] +============PREDICATE============ +Input::EVENT_NEW +[i=1] +[s=TEST2, ss=] +============PREDICATE============ +Input::EVENT_CHANGED +[i=4] +[s=TEST2, ss=TEST2] +============PREDICATE============ +Input::EVENT_REMOVED +[i=3] +[s=TEST, ss=] diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/servers.out b/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/servers.out index f18be12673..8f4b116230 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/servers.out +++ b/testing/btest/Baseline/scripts.base.frameworks.input.empty-values-hashing/servers.out @@ -1,12 +1,12 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. ==========SERVERS============ { -[2] = [s=, ss=], -[1] = [s=, ss=TEST] +[4] = [s=TEST, ss=TEST], +[3] = [s=TEST, ss=] } ==========SERVERS============ { -[2] = [s=TEST, ss=TEST], -[1] = [s=TEST, ss=] +[4] = [s=TEST2, ss=TEST2], +[1] = [s=TEST2, ss=] } done diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.emptyvals/out b/testing/btest/Baseline/scripts.base.frameworks.input.emptyvals/out index b780948271..24731a5bf2 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.emptyvals/out +++ b/testing/btest/Baseline/scripts.base.frameworks.input.emptyvals/out @@ -1,5 +1,8 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. { -[2] = [b=], [1] = [b=T] } +{ +[1, 1] = [b=T], +[2, 2] = [b=] +} diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/.stderrwithoutfirstline b/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/.stderrwithoutfirstline index 5bdaaf3d9a..72c19e7b16 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/.stderrwithoutfirstline +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/.stderrwithoutfirstline @@ -1,7 +1,9 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +warning: Skipping input with missing non-optional value warning: ..<...>/Input::READER_ASCII: Invalid value for subnet: 127.0.0.1 warning: ..<...>/Input::READER_ASCII: Error while reading set or vector warning: ..<...>/Input::READER_ASCII: Could not convert line 'name 127.0.0.1' of ../input.log to Val. Ignoring line. +warning: Skipping input with missing non-optional value warning: ..<...>/Input::READER_ASCII: Invalid value for subnet: 127.0.0.1 warning: ..<...>/Input::READER_ASCII: Error while reading set or vector warning: ..<...>/Input::READER_ASCII: 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 6dec2c576e..199db80c7d 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/out +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalidset/out @@ -1,16 +1,13 @@ ### 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. - -Event, [s={ 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 +EventErrorEvent, Skipping input with missing non-optional value, 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 -[name] = [s={ +TableErrorEvent, Skipping input with missing non-optional value, Reporter::WARNING { } -}] -}] diff --git a/testing/btest/scripts/base/frameworks/input/empty-values-hashing.zeek b/testing/btest/scripts/base/frameworks/input/empty-values-hashing.zeek index c6e945dbc3..5064aaf63d 100644 --- a/testing/btest/scripts/base/frameworks/input/empty-values-hashing.zeek +++ b/testing/btest/scripts/base/frameworks/input/empty-values-hashing.zeek @@ -11,15 +11,18 @@ #separator \x09 #fields i s ss #types int sting string -1 - TEST -2 - - +1 - - +2 - TEST +3 TEST - +4 TEST TEST @TEST-END-FILE @TEST-START-FILE input2.log #separator \x09 #fields i s ss #types int sting string -1 TEST - -2 TEST TEST +1 TEST2 - +4 TEST2 TEST2 +5 - TEST2 @TEST-END-FILE redef exit_only_after_terminate = T; @@ -32,7 +35,7 @@ type Idx: record { type Val: record { s: string; - ss: string; + ss: string &optional; }; type servers_type: table[int] of Val; diff --git a/testing/btest/scripts/base/frameworks/input/emptyvals.zeek b/testing/btest/scripts/base/frameworks/input/emptyvals.zeek index b495832d6d..fcdae61aa9 100644 --- a/testing/btest/scripts/base/frameworks/input/emptyvals.zeek +++ b/testing/btest/scripts/base/frameworks/input/emptyvals.zeek @@ -1,14 +1,31 @@ +# This test verifies the handling of unset fields in input files. +# For table indexes, columns wwith undefined fields cannot work +# and are skipped. For values, unset fields are safe for the user +# only when those fields are defined &optional, otherwise they +# too are skipped. + # @TEST-EXEC: btest-bg-run zeek zeek -b %INPUT # @TEST-EXEC: btest-bg-wait 10 # @TEST-EXEC: btest-diff out -@TEST-START-FILE input.log +@TEST-START-FILE input1.log #separator \x09 #path ssh #fields b i ##types bool int T 1 - 2 +F - +@TEST-END-FILE + +@TEST-START-FILE input2.log +#separator \x09 +#path ssh +#fields b i j +##types bool int int +T 1 1 +- 2 2 +F - 3 @TEST-END-FILE redef exit_only_after_terminate = T; @@ -19,27 +36,52 @@ redef InputAscii::empty_field = "EMPTY"; module A; -type Idx: record { +# We use two different index records just because the internal code +# paths differ slightly for these. And one used to crash. :) +type Idx1: record { i: int; }; -type Val: record { +type Idx2: record { + i: int; + j: int; +}; + +type ValReq: record { b: bool; }; -global servers: table[int] of Val = table(); +type ValOpt: record { + b: bool &optional; +}; + +global servers1: table[int] of ValReq = table(); +global servers2: table[int, int] of ValOpt = table(); + +# Counter to track when we're ready to report both table's contents in +# pre-defined order. +global reads_done = 0; event zeek_init() { - outfile = open("../out"); + outfile = open("../out"); # first read in the old stuff into the table... - Input::add_table([$source="../input.log", $name="ssh", $idx=Idx, $val=Val, $destination=servers]); + Input::add_table([$source="../input1.log", $name="ssh1", $idx=Idx1, $val=ValReq, $destination=servers1]); + Input::add_table([$source="../input2.log", $name="ssh2", $idx=Idx2, $val=ValOpt, $destination=servers2]); } event Input::end_of_data(name: string, source:string) { - print outfile, servers; - Input::remove("ssh"); + reads_done += 1; + if ( reads_done < 2 ) + return; + + print outfile, servers1; + print outfile, servers2; + + Input::remove("ssh1"); + Input::remove("ssh2"); + close(outfile); terminate(); }