From d6c22295bd33db8534fe6f401e4b7b6a8de4255c Mon Sep 17 00:00:00 2001
From: Benjamin Bannier <benjamin.bannier@corelight.com>
Date: Fri, 4 Jul 2025 13:05:13 +0200
Subject: [PATCH] [Spicy] Let `zeek::protocol_handle_close()` send a TCP EOF.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Zeek's analyzer API makes it hard to determine during analyzer
shutdown whether a regular end-of-data has been reached, or if we're
aborting in the middle of a session (e.g., because Zeek missed the
remaining packets): the corresponding analyzer method, `EndOfData()`
gets called in both cases.

In an earlier change, we had stopped signaling Spicy analyzers a
regular finish when that `EndOfData()` method executes, because doing
so could trigger a parse error if it wasn't a regular shutdown—-which
isn't desired, a user request was to just silently stop processing in
this case.

However, that behavior now seems unfortunate in the case that one
deliberately calls `zeek::protocol_handle_close()` to terminate an
analyzer: this feels like a regular shutdown that should just
immediately happen. We achieve this now in this function by
additionally signaling the shutdown at the TCP layer as an "end of
file", which, for Spicy analyzers, happens to run the final, orderly
tear-down.

Not exactly great, but ti seems to thread the needle to achieve the
desired semantics in both cases.
---
 src/spicy/runtime-support.cc                  |  9 +++-
 .../spicy.tcp-eod-behavior-child/output       |  3 ++
 .../btest/spicy/tcp-eod-behavior-child.zeek   | 51 +++++++++++++++++++
 3 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 testing/btest/Baseline/spicy.tcp-eod-behavior-child/output
 create mode 100644 testing/btest/spicy/tcp-eod-behavior-child.zeek

diff --git a/src/spicy/runtime-support.cc b/src/spicy/runtime-support.cc
index 76cf8c1f59..abc97b5027 100644
--- a/src/spicy/runtime-support.cc
+++ b/src/spicy/runtime-support.cc
@@ -818,7 +818,14 @@ void rt::protocol_handle_close(const ProtocolHandle& handle) {
             if ( child->IsFinished() || child->Removing() )
                 throw ValueUnavailable(hilti::rt::fmt("child analyzer %s no longer exist", handle));
 
-            child->NextEndOfData(true);
+            auto* tcp_child = dynamic_cast<analyzer::tcp::TCP_ApplicationAnalyzer*>(child);
+            if ( ! tcp_child )
+                throw ValueUnavailable(hilti::rt::fmt("child analyzer %s is not a TCP application analyzer", handle));
+
+            tcp_child->EndpointEOF(true); // For Spicy analyzers, this will trigger Finish() ...
+            child->NextEndOfData(true);   // ... whereas this won't.
+
+            tcp_child->EndpointEOF(false);
             child->NextEndOfData(false);
 
             c->analyzer->RemoveChildAnalyzer(handle.id());
diff --git a/testing/btest/Baseline/spicy.tcp-eod-behavior-child/output b/testing/btest/Baseline/spicy.tcp-eod-behavior-child/output
new file mode 100644
index 0000000000..3231517d95
--- /dev/null
+++ b/testing/btest/Baseline/spicy.tcp-eod-behavior-child/output
@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+F, S, 1
+T, S, 1
diff --git a/testing/btest/spicy/tcp-eod-behavior-child.zeek b/testing/btest/spicy/tcp-eod-behavior-child.zeek
new file mode 100644
index 0000000000..1427d8a618
--- /dev/null
+++ b/testing/btest/spicy/tcp-eod-behavior-child.zeek
@@ -0,0 +1,51 @@
+# @TEST-REQUIRES: have-spicy
+#
+# @TEST-EXEC: spicyz -d -o x.hlto x.spicy x.evt
+# @TEST-EXEC: zeek -r ${TRACES}/ssh/single-conn.trace Zeek::Spicy x.hlto x.zeek >output 2>&1
+# @TEST-EXEC: btest-diff output
+#
+# @TEST-DOC: Checks that a analyzer is properly finished when a protocol handle is closed.
+
+# We use a child analyzer since this particular issue does not trigger for the root analyzer.
+
+# @TEST-START-FILE x.spicy
+module Foo;
+import zeek;
+
+public type X = unit {
+    data: bytes &size=2 {
+        local h = zeek::protocol_handle_get_or_create("Y");
+        zeek::protocol_data_in(zeek::is_orig(), $$, h);
+        zeek::protocol_handle_close(h);
+    }
+};
+
+public type Y = unit {
+    a: bytes &size=1;
+    b: bytes &eod;
+};
+# @TEST-END-FILE
+
+# @TEST-START-FILE x.evt
+import Foo;
+
+protocol analyzer X over TCP:
+    parse with Foo::X;
+
+protocol analyzer Y over TCP:
+    parse with Foo::Y;
+
+export Foo::Y;
+on Foo::Y -> event foo($is_orig, self);
+# @TEST-END-FILE
+
+
+# @TEST-START-FILE x.zeek
+event zeek_init() {
+    Analyzer::register_for_port(Analyzer::ANALYZER_X, 22/tcp);
+}
+
+event foo(is_orig: bool, y: Foo::Y) {
+    print is_orig, y$a, |y$b|;
+}
+# @TEST-END-FILE