Improve Broker store API's handling of invalid arguments

* Some methods mistakenly returned a bool instead of QueryResult
  when passed an invalid `opaque of Broker::Store` handle.

* Now generates a runtime exception for store_name() and is_closed()
  calls that pass an invalid `opaque of Broker::Store` handle as any
  returned value can't be reasonably used in any subsequent logic.

* Descriptions of any invalid arguments are now given in the error
  message.
This commit is contained in:
Jon Siwek 2020-06-24 22:56:14 -07:00
parent 2b698c4e1b
commit a9f853efcd
3 changed files with 66 additions and 59 deletions

View file

@ -93,10 +93,7 @@ function Broker::__is_closed%(h: opaque of Broker::Store%): bool
auto handle = to_store_handle(h); auto handle = to_store_handle(h);
if ( ! handle ) if ( ! handle )
{ builtin_exception("invalid Broker store handle", h);
builtin_error("invalid Broker store handle");
return val_mgr->False();
}
return val_mgr->Bool(broker_mgr->LookupStore(handle->store.name())); return val_mgr->Bool(broker_mgr->LookupStore(handle->store.name()));
%} %}
@ -108,7 +105,7 @@ function Broker::__close%(h: opaque of Broker::Store%): bool
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -120,10 +117,7 @@ function Broker::__store_name%(h: opaque of Broker::Store%): string
auto handle = to_store_handle(h); auto handle = to_store_handle(h);
if ( ! handle ) if ( ! handle )
{ builtin_exception("invalid Broker store handle", h);
builtin_error("invalid Broker store handle");
return val_mgr->EmptyString();
}
return make_intrusive<StringVal>(handle->store.name()); return make_intrusive<StringVal>(handle->store.name());
%} %}
@ -135,15 +129,15 @@ function Broker::__exists%(h: opaque of Broker::Store,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return bro_broker::query_result();
} }
auto key = bro_broker::val_to_data(k); auto key = bro_broker::val_to_data(k);
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return bro_broker::query_result(); return bro_broker::query_result();
} }
@ -181,15 +175,15 @@ function Broker::__get%(h: opaque of Broker::Store,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return bro_broker::query_result();
} }
auto key = bro_broker::val_to_data(k); auto key = bro_broker::val_to_data(k);
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return bro_broker::query_result(); return bro_broker::query_result();
} }
@ -227,8 +221,8 @@ function Broker::__put_unique%(h: opaque of Broker::Store,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return bro_broker::query_result();
} }
auto key = bro_broker::val_to_data(k); auto key = bro_broker::val_to_data(k);
@ -236,13 +230,13 @@ function Broker::__put_unique%(h: opaque of Broker::Store,
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return bro_broker::query_result(); return bro_broker::query_result();
} }
if ( ! val ) if ( ! val )
{ {
builtin_error("invalid Broker data conversion for value argument"); builtin_error("invalid Broker data conversion for value argument", v);
return bro_broker::query_result(); return bro_broker::query_result();
} }
@ -282,15 +276,15 @@ function Broker::__get_index_from_value%(h: opaque of Broker::Store,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return bro_broker::query_result();
} }
auto key = bro_broker::val_to_data(k); auto key = bro_broker::val_to_data(k);
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return bro_broker::query_result(); return bro_broker::query_result();
} }
@ -298,10 +292,10 @@ function Broker::__get_index_from_value%(h: opaque of Broker::Store,
if ( ! index ) if ( ! index )
{ {
builtin_error("invalid Broker data conversion for index argument"); builtin_error("invalid Broker data conversion for index argument", i);
return bro_broker::query_result(); return bro_broker::query_result();
} }
auto trigger = frame->GetTrigger(); auto trigger = frame->GetTrigger();
if ( ! trigger ) if ( ! trigger )
@ -336,8 +330,8 @@ function Broker::__keys%(h: opaque of Broker::Store%): Broker::QueryResult
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return bro_broker::query_result();
} }
auto trigger = frame->GetTrigger(); auto trigger = frame->GetTrigger();
@ -374,7 +368,7 @@ function Broker::__put%(h: opaque of Broker::Store,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -383,13 +377,13 @@ function Broker::__put%(h: opaque of Broker::Store,
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
if ( ! val ) if ( ! val )
{ {
builtin_error("invalid Broker data conversion for value argument"); builtin_error("invalid Broker data conversion for value argument", v);
return val_mgr->False(); return val_mgr->False();
} }
@ -403,7 +397,7 @@ function Broker::__erase%(h: opaque of Broker::Store, k: any%): bool
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -411,7 +405,7 @@ function Broker::__erase%(h: opaque of Broker::Store, k: any%): bool
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
@ -426,7 +420,7 @@ function Broker::__increment%(h: opaque of Broker::Store, k: any, a: any,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -435,13 +429,13 @@ function Broker::__increment%(h: opaque of Broker::Store, k: any, a: any,
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
if ( ! amount ) if ( ! amount )
{ {
builtin_error("invalid Broker data conversion for amount argument"); builtin_error("invalid Broker data conversion for amount argument", a);
return val_mgr->False(); return val_mgr->False();
} }
@ -457,7 +451,7 @@ function Broker::__decrement%(h: opaque of Broker::Store, k: any, a: any,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -466,13 +460,13 @@ function Broker::__decrement%(h: opaque of Broker::Store, k: any, a: any,
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
if ( ! amount ) if ( ! amount )
{ {
builtin_error("invalid Broker data conversion for amount argument"); builtin_error("invalid Broker data conversion for amount argument", a);
return val_mgr->False(); return val_mgr->False();
} }
@ -487,7 +481,7 @@ function Broker::__append%(h: opaque of Broker::Store, k: any, s: any,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -496,13 +490,13 @@ function Broker::__append%(h: opaque of Broker::Store, k: any, s: any,
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
if ( ! str ) if ( ! str )
{ {
builtin_error("invalid Broker data conversion for str argument"); builtin_error("invalid Broker data conversion for str argument", s);
return val_mgr->False(); return val_mgr->False();
} }
@ -517,7 +511,7 @@ function Broker::__insert_into_set%(h: opaque of Broker::Store, k: any, i: any,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -526,13 +520,13 @@ function Broker::__insert_into_set%(h: opaque of Broker::Store, k: any, i: any,
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
if ( ! idx ) if ( ! idx )
{ {
builtin_error("invalid Broker data conversion for index argument"); builtin_error("invalid Broker data conversion for index argument", i);
return val_mgr->False(); return val_mgr->False();
} }
@ -548,7 +542,7 @@ function Broker::__insert_into_table%(h: opaque of Broker::Store, k: any,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -558,19 +552,19 @@ function Broker::__insert_into_table%(h: opaque of Broker::Store, k: any,
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
if ( ! idx ) if ( ! idx )
{ {
builtin_error("invalid Broker data conversion for index argument"); builtin_error("invalid Broker data conversion for index argument", i);
return val_mgr->False(); return val_mgr->False();
} }
if ( ! val ) if ( ! val )
{ {
builtin_error("invalid Broker data conversion for value argument"); builtin_error("invalid Broker data conversion for value argument", v);
return val_mgr->False(); return val_mgr->False();
} }
@ -586,7 +580,7 @@ function Broker::__remove_from%(h: opaque of Broker::Store, k: any, i: any,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -595,13 +589,13 @@ function Broker::__remove_from%(h: opaque of Broker::Store, k: any, i: any,
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
if ( ! idx ) if ( ! idx )
{ {
builtin_error("invalid Broker data conversion for index argument"); builtin_error("invalid Broker data conversion for index argument", i);
return val_mgr->False(); return val_mgr->False();
} }
@ -617,7 +611,7 @@ function Broker::__push%(h: opaque of Broker::Store, k: any, v: any,
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -626,13 +620,13 @@ function Broker::__push%(h: opaque of Broker::Store, k: any, v: any,
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
if ( ! val ) if ( ! val )
{ {
builtin_error("invalid Broker data conversion for value argument"); builtin_error("invalid Broker data conversion for value argument", v);
return val_mgr->False(); return val_mgr->False();
} }
@ -646,7 +640,7 @@ function Broker::__pop%(h: opaque of Broker::Store, k: any, e: interval%): bool
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
@ -654,7 +648,7 @@ function Broker::__pop%(h: opaque of Broker::Store, k: any, e: interval%): bool
if ( ! key ) if ( ! key )
{ {
builtin_error("invalid Broker data conversion for key argument"); builtin_error("invalid Broker data conversion for key argument", k);
return val_mgr->False(); return val_mgr->False();
} }
@ -668,11 +662,10 @@ function Broker::__clear%(h: opaque of Broker::Store%): bool
if ( ! handle ) if ( ! handle )
{ {
builtin_error("invalid Broker store handle"); builtin_error("invalid Broker store handle", h);
return val_mgr->False(); return val_mgr->False();
} }
handle->store.clear(); handle->store.clear();
return val_mgr->True(); return val_mgr->True();
%} %}

View file

@ -1,2 +1,3 @@
error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/broker.store.invalid-handle/invalid-handle.zeek, line 6: invalid Broker store handle (Broker::keys(a)) expression error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/broker.store.invalid-handle/invalid-handle.zeek, line 18: invalid Broker store handle (0), during call: (Broker::is_closed(a))
keys, F error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/broker.store.invalid-handle/invalid-handle.zeek, line 6: invalid Broker store handle (Broker::keys(a) and 0)
keys, [status=Broker::FAILURE, result=[data=<uninitialized>]]

View file

@ -13,8 +13,21 @@ function print_keys(a: any)
} }
} }
function checkit(a: any)
{
if ( Broker::is_closed(a) )
print "this shouldn't get printed";
else
print "this shouldn't get printed either";
}
global a: int = 0; global a: int = 0;
event zeek_init() &priority=10
{
checkit(a);
}
event zeek_init() event zeek_init()
{ {
print_keys(a); print_keys(a);