broker/Data/data_to_val: Fail on vectors/lists with holes

Instead of simply removing holes from vectors or lists when converting
from Val to Broker format, error out as the receiver has no chance to
reconstruct where the hole might have been.

We could encode holes with broker::none, but this will put unnecessary
burden on language bindings and users due to the potential optionality.
Think a std::vector<uint64_t> that technically needs to be a
std::vector<std::optional<uint64_t>> to represent optional elements
properly.

Closes #3045

NEWS

@@ -17,6 +17,12 @@ New Functionality
 Changed Functionality
 ---------------------
 
+- 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
 ---------------------
 

src/broker/Data.cc

@@ -841,8 +841,10 @@ std::optional<broker::data> val_to_data(const Val* v) {
             for ( auto i = 0u; i < vec->Size(); ++i ) {
                 auto item_val = vec->ValAt(i);
 
-                if ( ! item_val )
+                if ( ! item_val ) {
-                    continue;
+                    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<broker::data> val_to_data(const Val* v) {
             for ( auto i = 0; i < list->Length(); ++i ) {
                 const auto& item_val = list->Idx(i);
 
-                if ( ! item_val )
+                if ( ! item_val ) {
-                    continue;
+                    reporter->Error("serialization of lists with holes is unsupported");
+                    return std::nullopt;
+                }
 
                 auto item = val_to_data(item_val.get());
 

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

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

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

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!

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