diff --git a/NEWS b/NEWS index 50c4faf128..974556cac9 100644 --- a/NEWS +++ b/NEWS @@ -256,6 +256,12 @@ Changed Functionality our switch to use the C-Ares library back in the 5.0 release, but we never removed the requirement from CMake. +- Publishing remote events with vector arguments that contain holes is now + rejected. The receiver side never had a chance to figure out where these + holes would have been. There's a chance this breaks scripts that accidentally + published vectors with holes. A reporter error is produced at runtime when + serialization of vectors with holes is attempted. + Removed Functionality --------------------- diff --git a/src/broker/Data.cc b/src/broker/Data.cc index e9a6492b88..850f88219b 100644 --- a/src/broker/Data.cc +++ b/src/broker/Data.cc @@ -841,8 +841,10 @@ std::optional val_to_data(const Val* v) { for ( auto i = 0u; i < vec->Size(); ++i ) { auto item_val = vec->ValAt(i); - if ( ! item_val ) - continue; + if ( ! item_val ) { + reporter->Error("serialization of vectors with holes is unsupported"); + return std::nullopt; + } auto item = val_to_data(item_val.get()); @@ -864,8 +866,10 @@ std::optional val_to_data(const Val* v) { for ( auto i = 0; i < list->Length(); ++i ) { const auto& item_val = list->Idx(i); - if ( ! item_val ) - continue; + if ( ! item_val ) { + reporter->Error("serialization of lists with holes is unsupported"); + return std::nullopt; + } auto item = val_to_data(item_val.get()); diff --git a/testing/btest/Baseline/cluster.generic.publish-vec-hole/..manager..stderr b/testing/btest/Baseline/cluster.generic.publish-vec-hole/..manager..stderr new file mode 100644 index 0000000000..0138046c28 --- /dev/null +++ b/testing/btest/Baseline/cluster.generic.publish-vec-hole/..manager..stderr @@ -0,0 +1,3 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error in ../manager.zeek, line 12: serialization of vectors with holes is unsupported +received termination signal diff --git a/testing/btest/Baseline/cluster.generic.publish-vec-hole/..manager..stdout b/testing/btest/Baseline/cluster.generic.publish-vec-hole/..manager..stdout new file mode 100644 index 0000000000..15e138e0b6 --- /dev/null +++ b/testing/btest/Baseline/cluster.generic.publish-vec-hole/..manager..stdout @@ -0,0 +1,6 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +node_up, worker-1 +got pong, with, [1, 2, 3], 3 +got pong, with, [1, 2, 3], 3 +got pong, with, [4, 5, 6], 3 +got pong, with, [4, 5, 6], 3 diff --git a/testing/btest/Baseline/cluster.generic.publish-vec-hole/..worker-1..stderr b/testing/btest/Baseline/cluster.generic.publish-vec-hole/..worker-1..stderr new file mode 100644 index 0000000000..e3f6131b1d --- /dev/null +++ b/testing/btest/Baseline/cluster.generic.publish-vec-hole/..worker-1..stderr @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +received termination signal diff --git a/testing/btest/Baseline/cluster.generic.publish-vec-hole/..worker-1..stdout b/testing/btest/Baseline/cluster.generic.publish-vec-hole/..worker-1..stdout new file mode 100644 index 0000000000..99b4863cdd --- /dev/null +++ b/testing/btest/Baseline/cluster.generic.publish-vec-hole/..worker-1..stdout @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +got ping, vector of count, [1, 2, 3], 3 +got ping, vector of count, [4, 5, 6], 3 +got finish! diff --git a/testing/btest/cluster/generic/publish-vec-hole.zeek b/testing/btest/cluster/generic/publish-vec-hole.zeek new file mode 100644 index 0000000000..971b29e46d --- /dev/null +++ b/testing/btest/cluster/generic/publish-vec-hole.zeek @@ -0,0 +1,92 @@ +# @TEST-DOC: Attempt to send an event with holes. It should fail. +# +# @TEST-REQUIRES: have-zeromq +# +# @TEST-PORT: XPUB_PORT +# @TEST-PORT: XSUB_PORT +# @TEST-PORT: LOG_PULL_PORT +# +# @TEST-EXEC: cp $FILES/zeromq/cluster-layout-no-logger.zeek cluster-layout.zeek +# @TEST-EXEC: cp $FILES/zeromq/test-bootstrap.zeek zeromq-test-bootstrap.zeek +# +# @TEST-EXEC: zeek -b --parse-only common.zeek manager.zeek worker.zeek +# +# @TEST-EXEC: btest-bg-run manager "ZEEKPATH=$ZEEKPATH:.. && CLUSTER_NODE=manager zeek -b ../manager.zeek" +# @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. && CLUSTER_NODE=worker-1 zeek -b ../worker.zeek" +# +# @TEST-EXEC: btest-bg-wait 30 +# @TEST-EXEC: btest-diff ./manager/.stderr +# @TEST-EXEC: btest-diff ./manager/.stdout +# @TEST-EXEC: btest-diff ./worker-1/.stdout +# @TEST-EXEC: btest-diff ./worker-1/.stderr + +# @TEST-START-FILE common.zeek +@load ./zeromq-test-bootstrap.zeek + +redef Log::default_rotation_interval = 0sec; + +global finish: event() &is_used; +global ping: event(v: vector of count) &is_used; +global pong: event(v: vector of count) &is_used; +# @TEST-END-FILE + +# @TEST-START-FILE manager.zeek +@load ./common.zeek + +event send_pings() + { + local v1 = vector(1, 2, 3); + + assert Cluster::publish(Cluster::worker_topic, ping, v1); + + # Publish with a vector with a hole, fails! + local v2 = vector(1); + v2[2] = 3; + assert ! Cluster::publish(Cluster::worker_topic, ping, v2); + local v3 = vector(4, 5, 6); + assert Cluster::publish(Cluster::worker_topic, ping, v3); + } + +global pongs = 0; + +event pong(v: vector of count) + { + ++pongs; + print "got pong", "with", v, |v|; + + # Two of the three pings go through, the worker sends 2 pongs + # for each ping, so stop after 4. + if ( pongs == 4 ) + Cluster::publish(Cluster::worker_topic, finish); + } + +event Cluster::node_up(name: string, id: string) + { + print "node_up", name; + event send_pings(); + } + +event Cluster::node_down(name: string, id: string) + { + terminate(); + } +# @TEST-END-FILE + + +# @TEST-START-FILE worker.zeek +@load ./common.zeek + +event ping(v: vector of count) + { + print "got ping", type_name(v), cat(v), |v|; + Cluster::publish(Cluster::manager_topic, pong, v); + local e = Cluster::make_event(pong, v); + Cluster::publish(Cluster::manager_topic, e); + } + +event finish() + { + print "got finish!"; + terminate(); + } +# @TEST-END-FILE