diff --git a/src/input/Manager.cc b/src/input/Manager.cc index ac79257ad9..0de6efd782 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -971,7 +971,7 @@ Val* Manager::RecordValToIndexVal(RecordVal *r) } -Val* Manager::ValueToIndexVal(const Stream* i, int num_fields, const RecordType *type, const Value* const *vals) +Val* Manager::ValueToIndexVal(const Stream* i, int num_fields, const RecordType *type, const Value* const *vals, bool& have_error) { Val* idxval; int position = 0; @@ -979,7 +979,7 @@ Val* Manager::ValueToIndexVal(const Stream* i, int num_fields, const RecordType if ( num_fields == 1 && type->FieldType(0)->Tag() != TYPE_RECORD ) { - idxval = ValueToVal(i, vals[0], type->FieldType(0)); + idxval = ValueToVal(i, vals[0], type->FieldType(0), have_error); position = 1; } else @@ -989,10 +989,10 @@ Val* Manager::ValueToIndexVal(const Stream* i, int num_fields, const RecordType { if ( type->FieldType(j)->Tag() == TYPE_RECORD ) l->Append(ValueToRecordVal(i, vals, - type->FieldType(j)->AsRecordType(), &position)); + type->FieldType(j)->AsRecordType(), &position, have_error)); else { - l->Append(ValueToVal(i, vals[position], type->FieldType(j))); + l->Append(ValueToVal(i, vals[position], type->FieldType(j), have_error)); position++; } } @@ -1079,7 +1079,7 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals) { // seen before if ( stream->num_val_fields == 0 || h->valhash == valhash ) - { + { // ok, exact duplicate, move entry to new dicrionary and do nothing else. stream->lastDict->Remove(idxhash); stream->currDict->Insert(idxhash, h); @@ -1094,8 +1094,8 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals) stream->lastDict->Remove(idxhash); // keep h for predicates updated = true; - } + } Val* valval; @@ -1103,63 +1103,68 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals) int position = stream->num_idx_fields; + bool convert_error = false; // this will be set to true by ValueTo* on Error + if ( stream->num_val_fields == 0 ) valval = 0; else if ( stream->num_val_fields == 1 && !stream->want_record ) - valval = ValueToVal(i, vals[position], stream->rtype->FieldType(0)); + valval = ValueToVal(i, vals[position], stream->rtype->FieldType(0), convert_error); else - valval = ValueToRecordVal(i, vals, stream->rtype, &position); - + valval = ValueToRecordVal(i, vals, stream->rtype, &position, convert_error); // call stream first to determine if we really add / change the entry - if ( stream->pred ) + if ( stream->pred && !convert_error ) { EnumVal* ev; int startpos = 0; - predidx = ValueToRecordVal(i, vals, stream->itype, &startpos); + bool pred_convert_error = false; + predidx = ValueToRecordVal(i, vals, stream->itype, &startpos, pred_convert_error); - if ( updated ) - ev = new EnumVal(BifEnum::Input::EVENT_CHANGED, BifType::Enum::Input::Event); - else - ev = new EnumVal(BifEnum::Input::EVENT_NEW, BifType::Enum::Input::Event); - - bool result; - if ( stream->num_val_fields > 0 ) // we have values - result = CallPred(stream->pred, 3, ev, predidx->Ref(), valval->Ref()); - else // no values - result = CallPred(stream->pred, 2, ev, predidx->Ref()); - - if ( result == false ) + // if we encountered a convert error here - just continue as we would have without + // emitting the event. I do not really think that that can happen just here and not + // at the top-level. But - this is safe. + if ( !pred_convert_error ) { - Unref(predidx); - Unref(valval); - - if ( ! updated ) - { - // just quit and delete everything we created. - delete idxhash; - return stream->num_val_fields + stream->num_idx_fields; - } - + if ( updated ) + ev = new EnumVal(BifEnum::Input::EVENT_CHANGED, BifType::Enum::Input::Event); else + ev = new EnumVal(BifEnum::Input::EVENT_NEW, BifType::Enum::Input::Event); + + bool result; + if ( stream->num_val_fields > 0 ) // we have values + result = CallPred(stream->pred, 3, ev, predidx->Ref(), valval->Ref()); + else // no values + result = CallPred(stream->pred, 2, ev, predidx->Ref()); + + if ( result == false ) { - // keep old one - stream->currDict->Insert(idxhash, h); - delete idxhash; - return stream->num_val_fields + stream->num_idx_fields; + Unref(predidx); + Unref(valval); + + if ( ! updated ) + { + // just quit and delete everything we created. + delete idxhash; + return stream->num_val_fields + stream->num_idx_fields; + } + + else + { + // keep old one + stream->currDict->Insert(idxhash, h); + delete idxhash; + return stream->num_val_fields + stream->num_idx_fields; + } } } + } // now we don't need h anymore - if we are here, the entry is updated and a new h is created. - if ( h ) - { - delete h; - h = 0; - } - + delete h; + h = 0; Val* idxval; if ( predidx != 0 ) @@ -1168,7 +1173,20 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals) // I think there is an unref missing here. But if I insert is, it crashes :) } else - idxval = ValueToIndexVal(i, stream->num_idx_fields, stream->itype, vals); + idxval = ValueToIndexVal(i, stream->num_idx_fields, stream->itype, vals, convert_error); + + if ( convert_error ) + { + // abort here and free everything that was allocated so far. + Unref(predidx); + Unref(valval); + Unref(idxval); + + delete idxhash; + return stream->num_val_fields + stream->num_idx_fields; + } + + assert(idxval); Val* oldval = 0; if ( updated == true ) @@ -1178,7 +1196,7 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals) oldval = stream->tab->Lookup(idxval, false); } - assert(idxval); + HashKey* k = stream->tab->ComputeHash(idxval); if ( ! k ) reporter->InternalError("could not hash"); @@ -1203,16 +1221,21 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals) { EnumVal* ev; int startpos = 0; - Val* predidx = ValueToRecordVal(i, vals, stream->itype, &startpos); + Val* predidx = ValueToRecordVal(i, vals, stream->itype, &startpos, convert_error); - if ( updated ) + if ( convert_error ) + { + // the only thing to clean up here is predidx. Everything else should + // already be ok again + Unref(predidx); + } + else if ( updated ) { // in case of update send back the old value. assert ( stream->num_val_fields > 0 ); ev = new EnumVal(BifEnum::Input::EVENT_CHANGED, BifType::Enum::Input::Event); assert ( oldval != 0 ); SendEvent(stream->event, 4, stream->description->Ref(), ev, predidx, oldval); } - else { ev = new EnumVal(BifEnum::Input::EVENT_NEW, BifType::Enum::Input::Event); @@ -1223,7 +1246,6 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals) } else SendEvent(stream->event, 4, stream->description->Ref(), ev, predidx, valval->Ref()); - } } @@ -1416,7 +1438,6 @@ int Manager::SendEventStreamEvent(Stream* i, EnumVal* type, const Value* const * assert(i->stream_type == EVENT_STREAM); EventStream* stream = (EventStream*) i; - Val *val; list out_vals; Ref(stream->description); out_vals.push_back(stream->description); @@ -1425,9 +1446,11 @@ int Manager::SendEventStreamEvent(Stream* i, EnumVal* type, const Value* const * int position = 0; + bool convert_error = false; + if ( stream->want_record ) { - RecordVal * r = ValueToRecordVal(i, vals, stream->fields, &position); + RecordVal * r = ValueToRecordVal(i, vals, stream->fields, &position, convert_error); out_vals.push_back(r); } @@ -1440,11 +1463,11 @@ int Manager::SendEventStreamEvent(Stream* i, EnumVal* type, const Value* const * if ( stream->fields->FieldType(j)->Tag() == TYPE_RECORD ) val = ValueToRecordVal(i, vals, stream->fields->FieldType(j)->AsRecordType(), - &position); + &position, convert_error); else { - val = ValueToVal(i, vals[position], stream->fields->FieldType(j)); + val = ValueToVal(i, vals[position], stream->fields->FieldType(j), convert_error); position++; } @@ -1452,7 +1475,14 @@ int Manager::SendEventStreamEvent(Stream* i, EnumVal* type, const Value* const * } } - SendEvent(stream->event, out_vals); + if ( convert_error ) + { + // we have an error somewhere in our out_vals. Just delete all of them. + for ( list::const_iterator it = out_vals.begin(), end = out_vals.end(); it != end; ++it ) + Unref(*it); + } + else + SendEvent(stream->event, out_vals); return stream->num_fields; } @@ -1464,7 +1494,9 @@ int Manager::PutTable(Stream* i, const Value* const *vals) assert(i->stream_type == TABLE_STREAM); TableStream* stream = (TableStream*) i; - Val* idxval = ValueToIndexVal(i, stream->num_idx_fields, stream->itype, vals); + bool convert_error = 0; + + Val* idxval = ValueToIndexVal(i, stream->num_idx_fields, stream->itype, vals, convert_error); Val* valval; int position = stream->num_idx_fields; @@ -1473,9 +1505,16 @@ int Manager::PutTable(Stream* i, const Value* const *vals) valval = 0; else if ( stream->num_val_fields == 1 && stream->want_record == 0 ) - valval = ValueToVal(i, vals[position], stream->rtype->FieldType(0)); + valval = ValueToVal(i, vals[position], stream->rtype->FieldType(0), convert_error); else - valval = ValueToRecordVal(i, vals, stream->rtype, &position); + valval = ValueToRecordVal(i, vals, stream->rtype, &position, convert_error); + + if ( convert_error ) + { + Unref(valval); + Unref(idxval); + return stream->num_idx_fields + stream->num_val_fields; + } // if we have a subscribed event, we need to figure out, if this is an update or not // same for predicates @@ -1503,31 +1542,37 @@ int Manager::PutTable(Stream* i, const Value* const *vals) { EnumVal* ev; int startpos = 0; - Val* predidx = ValueToRecordVal(i, vals, stream->itype, &startpos); + bool pred_convert_error = false; + Val* predidx = ValueToRecordVal(i, vals, stream->itype, &startpos, pred_convert_error); - if ( updated ) - ev = new EnumVal(BifEnum::Input::EVENT_CHANGED, - BifType::Enum::Input::Event); + if ( pred_convert_error ) + Unref(predidx); else - ev = new EnumVal(BifEnum::Input::EVENT_NEW, - BifType::Enum::Input::Event); - - bool result; - if ( stream->num_val_fields > 0 ) // we have values { - Ref(valval); - result = CallPred(stream->pred, 3, ev, predidx, valval); - } - else // no values - result = CallPred(stream->pred, 2, ev, predidx); + if ( updated ) + ev = new EnumVal(BifEnum::Input::EVENT_CHANGED, + BifType::Enum::Input::Event); + else + ev = new EnumVal(BifEnum::Input::EVENT_NEW, + BifType::Enum::Input::Event); - if ( result == false ) - { - // do nothing - Unref(idxval); - Unref(valval); - Unref(oldval); - return stream->num_val_fields + stream->num_idx_fields; + bool result; + if ( stream->num_val_fields > 0 ) // we have values + { + Ref(valval); + result = CallPred(stream->pred, 3, ev, predidx, valval); + } + else // no values + result = CallPred(stream->pred, 2, ev, predidx); + + if ( result == false ) + { + // do nothing + Unref(idxval); + Unref(valval); + Unref(oldval); + return stream->num_val_fields + stream->num_idx_fields; + } } } @@ -1538,28 +1583,34 @@ int Manager::PutTable(Stream* i, const Value* const *vals) { EnumVal* ev; int startpos = 0; - Val* predidx = ValueToRecordVal(i, vals, stream->itype, &startpos); + bool event_convert_error = false; + Val* predidx = ValueToRecordVal(i, vals, stream->itype, &startpos, event_convert_error); - if ( updated ) - { - // in case of update send back the old value. - assert ( stream->num_val_fields > 0 ); - ev = new EnumVal(BifEnum::Input::EVENT_CHANGED, - BifType::Enum::Input::Event); - assert ( oldval != 0 ); - SendEvent(stream->event, 4, stream->description->Ref(), - ev, predidx, oldval); - } + if ( event_convert_error ) + Unref(predidx); else { - ev = new EnumVal(BifEnum::Input::EVENT_NEW, - BifType::Enum::Input::Event); - if ( stream->num_val_fields == 0 ) + if ( updated ) + { + // in case of update send back the old value. + assert ( stream->num_val_fields > 0 ); + ev = new EnumVal(BifEnum::Input::EVENT_CHANGED, + BifType::Enum::Input::Event); + assert ( oldval != 0 ); SendEvent(stream->event, 4, stream->description->Ref(), - ev, predidx); + ev, predidx, oldval); + } else - SendEvent(stream->event, 4, stream->description->Ref(), - ev, predidx, valval->Ref()); + { + ev = new EnumVal(BifEnum::Input::EVENT_NEW, + BifType::Enum::Input::Event); + if ( stream->num_val_fields == 0 ) + SendEvent(stream->event, 4, stream->description->Ref(), + ev, predidx); + else + SendEvent(stream->event, 4, stream->description->Ref(), + ev, predidx, valval->Ref()); + } } } @@ -1612,29 +1663,42 @@ bool Manager::Delete(ReaderFrontend* reader, Value* *vals) if ( i->stream_type == TABLE_STREAM ) { TableStream* stream = (TableStream*) i; - Val* idxval = ValueToIndexVal(i, stream->num_idx_fields, stream->itype, vals); + bool convert_error = false; + Val* idxval = ValueToIndexVal(i, stream->num_idx_fields, stream->itype, vals, convert_error); assert(idxval != 0); readVals = stream->num_idx_fields + stream->num_val_fields; bool streamresult = true; + if ( convert_error ) + { + Unref(idxval); + return false; + } + if ( stream->pred || stream->event ) { Val *val = stream->tab->Lookup(idxval); if ( stream->pred ) { - Ref(val); - EnumVal *ev = new EnumVal(BifEnum::Input::EVENT_REMOVED, BifType::Enum::Input::Event); int startpos = 0; - Val* predidx = ValueToRecordVal(i, vals, stream->itype, &startpos); + Val* predidx = ValueToRecordVal(i, vals, stream->itype, &startpos, convert_error); - streamresult = CallPred(stream->pred, 3, ev, predidx, val); - - if ( streamresult == false ) + if ( convert_error ) + Unref(predidx); + else { - // keep it. - Unref(idxval); - success = true; + Ref(val); + EnumVal *ev = new EnumVal(BifEnum::Input::EVENT_REMOVED, BifType::Enum::Input::Event); + + streamresult = CallPred(stream->pred, 3, ev, predidx, val); + + if ( streamresult == false ) + { + // keep it. + Unref(idxval); + success = true; + } } } @@ -1739,13 +1803,22 @@ bool Manager::SendEvent(ReaderFrontend* reader, const string& name, const int nu return false; } + bool convert_error = false; + val_list* vl = new val_list; for ( int j = 0; j < num_vals; j++) - vl->append(ValueToVal(i, vals[j], type->FieldType(j))); - - mgr.QueueEvent(handler, vl, SOURCE_LOCAL); + vl->append(ValueToVal(i, vals[j], type->FieldType(j), convert_error)); delete_value_ptr_array(vals, num_vals); + + if ( convert_error ) + { + delete_vals(vl); + return false; + } + else + mgr.QueueEvent(handler, vl, SOURCE_LOCAL); + return true; } @@ -1817,7 +1890,7 @@ RecordVal* Manager::ListValToRecordVal(ListVal* list, RecordType *request_type, // Convert a threading value to a record value RecordVal* Manager::ValueToRecordVal(const Stream* stream, const Value* const *vals, - RecordType *request_type, int* position) + RecordType *request_type, int* position, bool& have_error) { assert(position != 0); // we need the pointer to point to data. @@ -1826,7 +1899,7 @@ RecordVal* Manager::ValueToRecordVal(const Stream* stream, const Value* const *v { Val* fieldVal = 0; if ( request_type->FieldType(i)->Tag() == TYPE_RECORD ) - fieldVal = ValueToRecordVal(stream, vals, request_type->FieldType(i)->AsRecordType(), position); + fieldVal = ValueToRecordVal(stream, vals, request_type->FieldType(i)->AsRecordType(), position, have_error); else if ( request_type->FieldType(i)->Tag() == TYPE_FILE || request_type->FieldType(i)->Tag() == TYPE_FUNC ) { @@ -1841,7 +1914,7 @@ RecordVal* Manager::ValueToRecordVal(const Stream* stream, const Value* const *v } else { - fieldVal = ValueToVal(stream, vals[*position], request_type->FieldType(i)); + fieldVal = ValueToVal(stream, vals[*position], request_type->FieldType(i), have_error); (*position)++; } @@ -2110,8 +2183,13 @@ HashKey* Manager::HashValues(const int num_elements, const Value* const *vals) } // convert threading value to Bro value -Val* Manager::ValueToVal(const Stream* i, const Value* val, BroType* request_type) +// have_error is a reference to a boolean which is set to true as soon as an error occured. +// When have_error is set to true at the beginning of the function, it is assumed that +// an error already occured in the past and processing is aborted. +Val* Manager::ValueToVal(const Stream* i, const Value* val, BroType* request_type, bool& have_error) { + if ( have_error ) + return 0; if ( request_type->Tag() != TYPE_ANY && request_type->Tag() != val->type ) { @@ -2198,9 +2276,10 @@ Val* Manager::ValueToVal(const Stream* i, const Value* val, BroType* request_typ TableVal* t = new TableVal(s); for ( int j = 0; j < val->val.set_val.size; j++ ) { - Val* assignval = ValueToVal(i, val->val.set_val.vals[j], type); + Val* assignval = ValueToVal(i, val->val.set_val.vals[j], type, have_error); + t->Assign(assignval, 0); - Unref(assignval); // idex is not consumed by assign. + Unref(assignval); // index is not consumed by assign. } Unref(s); @@ -2214,7 +2293,9 @@ Val* Manager::ValueToVal(const Stream* i, const Value* val, BroType* request_typ VectorType* vt = new VectorType(type->Ref()); VectorVal* v = new VectorVal(vt); for ( int j = 0; j < val->val.vector_val.size; j++ ) - v->Assign(j, ValueToVal(i, val->val.set_val.vals[j], type)); + { + v->Assign(j, ValueToVal(i, val->val.set_val.vals[j], type, have_error)); + } Unref(vt); return v; @@ -2233,9 +2314,14 @@ Val* Manager::ValueToVal(const Stream* i, const Value* val, BroType* request_typ // but well. bro_int_t index = request_type->AsEnumType()->Lookup(module, var.c_str()); if ( index == -1 ) - reporter->InternalError("Value not '%s' for stream '%s' is not a valid enum.", + { + reporter->Error("Value not '%s' for stream '%s' is not a valid enum.", enum_string.c_str(), i->name.c_str()); + have_error = true; + return 0; + } + return new EnumVal(index, request_type->Ref()->AsEnumType()); } diff --git a/src/input/Manager.h b/src/input/Manager.h index 551eeaecf3..d61ed3a485 100644 --- a/src/input/Manager.h +++ b/src/input/Manager.h @@ -205,14 +205,14 @@ private: // Convert Threading::Value to an internal Bro Type (works also with // Records). - Val* ValueToVal(const Stream* i, const threading::Value* val, BroType* request_type); + Val* ValueToVal(const Stream* i, const threading::Value* val, BroType* request_type, bool& have_error); // Convert Threading::Value to an internal Bro List type. - Val* ValueToIndexVal(const Stream* i, int num_fields, const RecordType* type, const threading::Value* const *vals); + Val* ValueToIndexVal(const Stream* i, int num_fields, const RecordType* type, const threading::Value* const *vals, bool& have_error); // Converts a threading::value to a record type. Mostly used by // ValueToVal. - RecordVal* ValueToRecordVal(const Stream* i, const threading::Value* const *vals, RecordType *request_type, int* position); + RecordVal* ValueToRecordVal(const Stream* i, const threading::Value* const *vals, RecordType *request_type, int* position, bool& have_error); Val* RecordValToIndexVal(RecordVal *r); diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stderr b/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stderr new file mode 100644 index 0000000000..c8e56d4c21 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stderr @@ -0,0 +1,2 @@ +error: Value not 'IdoNot::Exist' for stream 'enum' is not a valid enum. +received termination signal diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stdout b/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stdout new file mode 100644 index 0000000000..e760668629 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stdout @@ -0,0 +1,4 @@ +Table: +{ + +} diff --git a/testing/btest/scripts/base/frameworks/input/missing-enum.bro b/testing/btest/scripts/base/frameworks/input/missing-enum.bro new file mode 100644 index 0000000000..0d37aae453 --- /dev/null +++ b/testing/btest/scripts/base/frameworks/input/missing-enum.bro @@ -0,0 +1,37 @@ +# @TEST-EXEC: btest-bg-run bro bro -b %INPUT +# @TEST-EXEC: btest-bg-wait 10 +# @TEST-EXEC: btest-diff bro/.stderr +# @TEST-EXEC: btest-diff bro/.stdout + +@TEST-START-FILE input.log +#fields e i +IdoNot::Exist 1 +@TEST-END-FILE + +redef exit_only_after_terminate = T; + +module A; + +type Idx: record { + i: int; +}; + +type Val: record { + e: Log::ID; +}; + +global etable: table[int] of Log::ID = table(); + +event bro_init() + { + # first read in the old stuff into the table... + Input::add_table([$source="../input.log", $name="enum", $idx=Idx, $val=Val, $destination=etable, $want_record=F]); + } + +event Input::end_of_data(name: string, source:string) + { + print "Table:"; + print etable; + Input::remove("enum"); + terminate(); + }