From 6572324b8c2dc717a17461dbc9e0aeb307f06d4d Mon Sep 17 00:00:00 2001
From: Arne Welzel <arne.welzel@corelight.com>
Date: Mon, 4 Mar 2024 14:16:06 +0100
Subject: [PATCH] Stmt: Fix assert evaluating cond twice

Since 81a9745fb36fb6705873921423364bd2c438bfc4, the assert condition is
evaluated twice. This leads to unexpected behavior when cond has a side
effect like publishing a message or creating a log stream or filter.

Found while using the following in ad-hoc testing code and wondering
why two messages were published.

    assert publish(Cluster::worker_topic, hello, "abc")
---
 src/Stmt.cc                                   |  2 +-
 .../Baseline/language.assert-hook-8/.stderr   |  1 +
 .../btest/Baseline/language.assert-hook-8/out |  6 ++++
 testing/btest/language/assert-hook.zeek       | 28 +++++++++++++++++++
 4 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 testing/btest/Baseline/language.assert-hook-8/.stderr
 create mode 100644 testing/btest/Baseline/language.assert-hook-8/out

diff --git a/src/Stmt.cc b/src/Stmt.cc
index 9c2a593b91..6c304b2bc2 100644
--- a/src/Stmt.cc
+++ b/src/Stmt.cc
@@ -1644,7 +1644,7 @@ ValPtr AssertStmt::Exec(Frame* f, StmtFlowType& flow) {
     bool run_result_hook = assertion_result_hook && assertion_result_hook->HasEnabledBodies();
     auto assert_result = cond->Eval(f)->AsBool();
 
-    if ( ! cond->Eval(f)->AsBool() || run_result_hook ) {
+    if ( ! assert_result || run_result_hook ) {
         zeek::StringValPtr msg_val = zeek::val_mgr->EmptyString();
 
         if ( msg ) {
diff --git a/testing/btest/Baseline/language.assert-hook-8/.stderr b/testing/btest/Baseline/language.assert-hook-8/.stderr
new file mode 100644
index 0000000000..49d861c74c
--- /dev/null
+++ b/testing/btest/Baseline/language.assert-hook-8/.stderr
@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
diff --git a/testing/btest/Baseline/language.assert-hook-8/out b/testing/btest/Baseline/language.assert-hook-8/out
new file mode 100644
index 0000000000..01f1c34267
--- /dev/null
+++ b/testing/btest/Baseline/language.assert-hook-8/out
@@ -0,0 +1,6 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+zeek_init
+returning true
+assertion_result, T, always_true(), always true, <...>/assert-hook.zeek, 23
+returning false
+assertion_result, F, always_false(), always false, <...>/assert-hook.zeek, 24
diff --git a/testing/btest/language/assert-hook.zeek b/testing/btest/language/assert-hook.zeek
index 36909dec80..1f107ff70e 100644
--- a/testing/btest/language/assert-hook.zeek
+++ b/testing/btest/language/assert-hook.zeek
@@ -196,3 +196,31 @@ event zeek_done()
 	assert 2 + 2 == 5, "this is false";
 	print "not reached";
 	}
+
+@TEST-START-NEXT
+# Ensure cond is only evaluated once.
+hook assertion_result(result: bool, cond: string, msg: string, bt: Backtrace)
+	{
+	print "assertion_result", result, cond, msg, bt[0]$file_location, bt[0]$line_location;
+	break;
+	}
+
+function always_true(): bool
+	{
+	print "returning true";
+	return T;
+	}
+
+function always_false(): bool
+	{
+	print "returning false";
+	return F;
+	}
+
+event zeek_init()
+	{
+	print "zeek_init";
+	assert always_true(), "always true";
+	assert always_false(), "always false";
+	print "not reached";
+	}