Change the way the input framework deals with values it cannot convert

into BroVals (especially enums)

Not we do not force an internal error anymore. Instead, we raise an
normal error and set an error flag that signals to the top-level
functions that the value could not be converted and should not be
propagated to the Bro core. This sadly makes the already messy code even
more messy - but since errors can happen in deeply nested data
structures, the alternative (catching the error at every possible
location and then trying to clean up there instead of recursively
deleting the data that cannot be used later) is much worse.

Addresses BIT-1199
This commit is contained in:
Johanna Amann 2015-03-19 14:58:38 -07:00
parent d236643894
commit c27848fc32
5 changed files with 248 additions and 119 deletions

View file

@ -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<Val*> 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<Val*>::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());
}

View file

@ -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);

View file

@ -0,0 +1,2 @@
error: Value not 'IdoNot::Exist' for stream 'enum' is not a valid enum.
received termination signal

View file

@ -0,0 +1,4 @@
Table:
{
}

View file

@ -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();
}