Merge remote-tracking branch 'origin/topic/awelzel/4177-4178-custom-event-metadata-prework'

* origin/topic/awelzel/4177-4178-custom-event-metadata-prework:
  Event/EventMgr: protected to private
  EventHandler: Deprecate Call(args, no_remote, ts), add Call(args)
  EventHandler: Header cleanup
  EventMgr: Do not cache current event attributes
  EventMgr: Drop src_val
  Event: Header cleanup

CHANGES

@@ -1,3 +1,37 @@
+7.2.0-dev.447 | 2025-03-31 14:21:51 +0200
+
+  * Event/EventMgr: protected to private (Arne Welzel, Corelight)
+
+    These classes are final, so deriving isn't possible. No reason to have
+    protected members.
+
+  * EventHandler: Deprecate Call(args, no_remote, ts), add Call(args) (Arne Welzel, Corelight)
+
+    The ts parameter was only added to Call() for the Broker::auto_publish()
+    functionality and propagating the network timestamp. By now, the auto-publish
+    functionality is deprecated, so it'd be good to cleanup that signature. There
+    won't be any need for no_remote in the future either.
+
+    Allow users to just use Call() instead.
+
+  * EventHandler: Header cleanup (Arne Welzel, Corelight)
+
+  * EventMgr: Do not cache current event attributes (Arne Welzel, Corelight)
+
+    Avoid proliferation of various members on EventMgr by storing the
+    pointer of the current event instead.
+
+    This subtly changes the behavior of some builtin functions as they would
+    have returned the prior event's data when executed outside of event
+    draining (e.g. C++ level hook invocations), but I think that's actually
+    for the better.
+
+  * EventMgr: Drop src_val (Arne Welzel, Corelight)
+
+    This is a left over and hasn't been used since a while.
+
+  * Event: Header cleanup (Arne Welzel, Corelight)
+
 7.2.0-dev.439 | 2025-03-27 16:12:44 -0700
 
   * Make storage events take a tag for the backend instead of a string (Tim Wojtulewicz, Corelight)

NEWS

@@ -9,6 +9,11 @@ Zeek 7.2.0
 Breaking Changes
 ----------------
 
+- The ``is_remote_event()``, ``current_analyzer()`` and ``current_event_time()``
+  builtin functions do not return the previous event's values anymore when event
+  draining has completed. The same applies to the corresponding C++ accessors on
+  the ``EventMgr`` class. The functions now return false, 0 or the zero time instead.
+
 New Functionality
 -----------------
 

VERSION

@@ -1 +1 @@
-7.2.0-dev.439
+7.2.0-dev.447

src/Event.cc

@@ -2,17 +2,14 @@
 
 #include "zeek/Event.h"
 
-#include "zeek/zeek-config.h"
-
 #include "zeek/Desc.h"
-#include "zeek/Func.h"
-#include "zeek/NetVar.h"
 #include "zeek/Trigger.h"
 #include "zeek/Val.h"
 #include "zeek/iosource/Manager.h"
-#include "zeek/iosource/PktSrc.h"
 #include "zeek/plugin/Manager.h"
 
+#include "event.bif.netvar_h"
+
 zeek::EventMgr zeek::event_mgr;
 
 namespace zeek {
@@ -52,7 +49,11 @@ void Event::Dispatch(bool no_remote) {
         reporter->BeginErrorHandler();
 
     try {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+        // Replace in v8.1 with handler->Call(&args).
         handler->Call(&args, no_remote, ts);
+#pragma GCC diagnostic pop
     }
 
     catch ( InterpreterException& e ) {
@@ -67,23 +68,12 @@ void Event::Dispatch(bool no_remote) {
         reporter->EndErrorHandler();
 }
 
-EventMgr::EventMgr() {
-    head = tail = nullptr;
-    current_src = util::detail::SOURCE_LOCAL;
-    current_aid = 0;
-    current_ts = 0;
-    src_val = nullptr;
-    draining = false;
-}
-
 EventMgr::~EventMgr() {
     while ( head ) {
         Event* n = head->NextEvent();
         Unref(head);
         head = n;
     }
-
-    Unref(src_val);
 }
 
 void EventMgr::Enqueue(const EventHandlerPtr& h, Args vl, util::detail::SourceID src, analyzer::ID aid, Obj* obj,
@@ -109,10 +99,10 @@ void EventMgr::QueueEvent(Event* event) {
 }
 
 void EventMgr::Dispatch(Event* event, bool no_remote) {
-    current_src = event->Source();
+    Event* old_current = current;
-    current_aid = event->Analyzer();
+    current = event;
-    current_ts = event->Time();
     event->Dispatch(no_remote);
+    current = old_current;
     Unref(event);
 }
 
@@ -122,8 +112,6 @@ void EventMgr::Drain() {
 
     PLUGIN_HOOK_VOID(HOOK_DRAIN_EVENTS, HookDrainEvents());
 
-    draining = true;
-
     // Past Zeek versions drained as long as there events, including when
     // a handler queued new events during its execution. This could lead
     // to endless loops in case a handler kept triggering its own event.
@@ -132,27 +120,25 @@ void EventMgr::Drain() {
     // that expect the old behavior to trigger something quickly.
 
     for ( int round = 0; head && round < 2; round++ ) {
-        Event* current = head;
+        Event* event = head;
         head = nullptr;
         tail = nullptr;
 
-        while ( current ) {
+        while ( event ) {
-            Event* next = current->NextEvent();
+            Event* next = event->NextEvent();
 
-            current_src = current->Source();
+            current = event;
-            current_aid = current->Analyzer();
+            event->Dispatch();
-            current_ts = current->Time();
+            Unref(event);
-            current->Dispatch();
-            Unref(current);
 
             ++event_mgr.num_events_dispatched;
-            current = next;
+            event = next;
         }
     }
 
     // Note: we might eventually need a general way to specify things to
     // do after draining events.
-    draining = false;
+    current = nullptr;
 
     // Make sure all of the triggers get processed every time the events
     // drain.

src/Event.h

@@ -5,12 +5,10 @@
 #include <tuple>
 #include <type_traits>
 
-#include "zeek/Flare.h"
-#include "zeek/IntrusivePtr.h"
 #include "zeek/ZeekArgs.h"
-#include "zeek/ZeekList.h"
 #include "zeek/analyzer/Analyzer.h"
 #include "zeek/iosource/IOSource.h"
+#include "zeek/util.h"
 
 namespace zeek {
 
@@ -36,7 +34,7 @@ public:
 
     void Describe(ODesc* d) const override;
 
-protected:
+private:
     friend class EventMgr;
 
     // This method is protected to make sure that everybody goes through
@@ -54,7 +52,6 @@ protected:
 
 class EventMgr final : public Obj, public iosource::IOSource {
 public:
-    EventMgr();
     ~EventMgr() override;
 
     /**
@@ -86,21 +83,23 @@ public:
     void Dispatch(Event* event, bool no_remote = false);
 
     void Drain();
-    bool IsDraining() const { return draining; }
+    bool IsDraining() const { return current != nullptr; }
 
     bool HasEvents() const { return head != nullptr; }
 
-    // Returns the source ID of last raised event.
+    // Returns the source ID of the current event.
-    util::detail::SourceID CurrentSource() const { return current_src; }
+    util::detail::SourceID CurrentSource() const { return current ? current->Source() : util::detail::SOURCE_LOCAL; }
 
-    // Returns the ID of the analyzer which raised the last event, or 0 if
+    // Returns the ID of the analyzer which raised the current event, or 0 if
     // non-analyzer event.
-    analyzer::ID CurrentAnalyzer() const { return current_aid; }
+    analyzer::ID CurrentAnalyzer() const { return current ? current->Analyzer() : 0; }
 
-    // Returns the timestamp of the last raised event. The timestamp reflects the network time
+    // Returns the timestamp of the current event. The timestamp reflects the network time
     // the event was intended to be executed. For scheduled events, this is the time the event
     // was scheduled to. For any other event, this is the time when the event was created.
-    double CurrentEventTime() const { return current_ts; }
+    //
+    // If no event is being processed, returns 0.0.
+    double CurrentEventTime() const { return current ? current->Time() : 0.0; }
 
     int Size() const { return num_events_queued - num_events_dispatched; }
 
@@ -117,16 +116,12 @@ public:
     uint64_t num_events_queued = 0;
     uint64_t num_events_dispatched = 0;
 
-protected:
+private:
     void QueueEvent(Event* event);
 
-    Event* head;
+    Event* current = nullptr;
-    Event* tail;
+    Event* head = nullptr;
-    util::detail::SourceID current_src;
+    Event* tail = nullptr;
-    analyzer::ID current_aid;
-    double current_ts;
-    RecordVal* src_val;
-    bool draining;
 };
 
 extern EventMgr event_mgr;

src/EventHandler.cc

@@ -6,7 +6,6 @@
 #include "zeek/Event.h"
 #include "zeek/Func.h"
 #include "zeek/ID.h"
-#include "zeek/NetVar.h"
 #include "zeek/Scope.h"
 #include "zeek/Var.h"
 #include "zeek/broker/Data.h"

src/EventHandler.h

@@ -4,13 +4,11 @@
 
 #pragma once
 
-#include <optional>
 #include <string>
 #include <unordered_set>
 
 #include "zeek/Type.h"
 #include "zeek/ZeekArgs.h"
-#include "zeek/ZeekList.h"
 
 namespace zeek {
 
@@ -47,8 +45,19 @@ public:
         auto_publish.erase(topic);
     }
 
+    [[deprecated(
+        "Remove in v8.1. The no_remote and ts parameters are AutoPublish() specific and won't have an effect "
+        "in the future. Use Call(args)")]]
     void Call(zeek::Args* vl, bool no_remote = false, double ts = run_state::network_time);
 
+    // Call the function associated with this handler.
+    void Call(zeek::Args* vl) {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+        Call(vl, false, run_state::network_time);
+#pragma GCC diagnostic pop
+    }
+
     // Returns true if there is at least one local or remote handler.
     explicit operator bool() const;
 

src/zeek.bif

@@ -4973,9 +4973,9 @@ function uninstall_dst_net_filter%(snet: subnet%) : bool
 	return zeek::val_mgr->Bool(packet_mgr->GetPacketFilter()->RemoveDst(snet));
 	%}
 
-## Checks whether the last raised event came from a remote peer.
+## Checks whether the current event came from a remote peer.
 ##
-## Returns: True if the last raised event came from a remote peer.
+## Returns: True if the current event came from a remote peer.
 function is_remote_event%(%) : bool
 	%{
 	return zeek::val_mgr->Bool(zeek::event_mgr.CurrentSource() != zeek::util::detail::SOURCE_LOCAL);