Merge remote-tracking branch 'origin/topic/awelzel/3045-no-holes-in-vectors'

* origin/topic/awelzel/3045-no-holes-in-vectors:
  broker/Data/data_to_val: Fail on vectors/lists with holes

CHANGES

@@ -1,3 +1,19 @@
+8.0.0-dev.4 | 2025-04-28 18:24:09 +0200
+
+  * GH-3045: broker/Data/data_to_val: Fail on vectors/lists with holes (Arne Welzel, Corelight)
+
+    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
+
 8.0.0-dev.2 | 2025-04-25 16:11:36 -0700
 
   * Fix use-after-move in recent broker changes (Tim Wojtulewicz, Corelight)

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
 ---------------------
 

VERSION

@@ -1 +1 @@
-8.0.0-dev.2
+8.0.0-dev.4

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 )
-                    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<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 )
-                    continue;
+                if ( ! item_val ) {
+                    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