Merge remote-tracking branch 'origin/topic/robin/gh-3573-replaces-cleanup'

* origin/topic/robin/gh-3573-replaces-cleanup:
  Fix packet analyzer replacement.
  Spicy: Wenn replacing an analyzer add a component mapping.
  Add component API to transparently remap one component to another one.
  Move enabled/disabled functionality from analyzers into `Component` base class API.

src/analyzer/Analyzer.cc

@@ -801,4 +801,30 @@ TEST_SUITE("Analyzer management") {
         CHECK(conn->FindAnalyzer("IMAP"));
         conn->Done();
     }
+
+    TEST_CASE("Analyzer mapping") {
+        REQUIRE(zeek::analyzer_mgr);
+
+        zeek::Packet p;
+        zeek::ConnTuple t;
+        auto conn = std::make_unique<zeek::Connection>(zeek::detail::ConnKey(t), 0, &t, 0, &p);
+
+        auto ssh = zeek::analyzer_mgr->InstantiateAnalyzer("SSH", conn.get());
+        REQUIRE(ssh);
+        auto imap = zeek::analyzer_mgr->InstantiateAnalyzer("IMAP", conn.get());
+        REQUIRE(imap);
+
+        zeek::analyzer_mgr->AddComponentMapping(ssh->GetAnalyzerTag(), imap->GetAnalyzerTag());
+        zeek::analyzer_mgr->DisableAnalyzer(ssh->GetAnalyzerTag()); // needs to be disabled for mapping to take effect
+        auto ssh_is_imap = zeek::analyzer_mgr->InstantiateAnalyzer("SSH", conn.get());
+        CHECK_EQ(ssh_is_imap->GetAnalyzerTag(), imap->GetAnalyzerTag()); // SSH is now IMAP
+
+        // orderly cleanup through connection
+        auto* tcp = new zeek::packet_analysis::TCP::TCPSessionAdapter(conn.get());
+        conn->SetSessionAdapter(tcp, nullptr);
+        tcp->AddChildAnalyzer(ssh);
+        tcp->AddChildAnalyzer(imap);
+        tcp->AddChildAnalyzer(ssh_is_imap);
+        conn->Done();
+    }
 }

src/analyzer/Component.cc

@@ -13,8 +13,8 @@ Component::Component(const std::string& name, factory_callback arg_factory, zeek
     : plugin::Component(arg_adapter ? plugin::component::SESSION_ADAPTER : plugin::component::ANALYZER, name,
                         arg_subtype, analyzer_mgr->GetTagType()) {
     factory = arg_factory;
-    enabled = arg_enabled;
     partial = arg_partial;
+    SetEnabled(arg_enabled);
 }
 
 void Component::Initialize() {
@@ -29,7 +29,7 @@ void Component::DoDescribe(ODesc* d) const {
         d->Add(", ");
     }
 
-    d->Add(enabled ? "enabled" : "disabled");
+    d->Add(Enabled() ? "enabled" : "disabled");
 }
 
 } // namespace zeek::analyzer

src/analyzer/Component.h

@@ -85,20 +85,6 @@ public:
      */
     bool Partial() const { return partial; }
 
-    /**
-     * Returns true if the analyzer is currently enabled and hence
-     * available for use.
-     */
-    bool Enabled() const { return enabled; }
-
-    /**
-     * Enables or disables this analyzer.
-     *
-     * @param arg_enabled True to enabled, false to disable.
-     *
-     */
-    void SetEnabled(bool arg_enabled) { enabled = arg_enabled; }
-
 protected:
     /**
      * Overridden from plugin::Component.
@@ -108,7 +94,6 @@ protected:
 private:
     factory_callback factory; // The analyzer's factory callback.
     bool partial;             // True if the analyzer supports partial connections.
-    bool enabled;             // True if the analyzer is enabled.
 };
 
 } // namespace analyzer

src/analyzer/Manager.cc

@@ -110,7 +110,7 @@ void Manager::DumpDebug() {
 void Manager::Done() {}
 
 bool Manager::EnableAnalyzer(const zeek::Tag& tag) {
-    Component* p = Lookup(tag);
+    Component* p = Lookup(tag, false);
 
     if ( ! p )
         return false;
@@ -122,7 +122,7 @@ bool Manager::EnableAnalyzer(const zeek::Tag& tag) {
 }
 
 bool Manager::EnableAnalyzer(EnumVal* val) {
-    Component* p = Lookup(val);
+    Component* p = Lookup(val, false);
 
     if ( ! p )
         return false;
@@ -134,7 +134,7 @@ bool Manager::EnableAnalyzer(EnumVal* val) {
 }
 
 bool Manager::DisableAnalyzer(const zeek::Tag& tag) {
-    Component* p = Lookup(tag);
+    Component* p = Lookup(tag, false);
 
     if ( ! p )
         return false;

src/file_analysis/Component.cc

@@ -11,7 +11,7 @@ namespace zeek::file_analysis {
 Component::Component(const std::string& name, factory_function arg_factory, Tag::subtype_t subtype, bool arg_enabled)
     : plugin::Component(plugin::component::FILE_ANALYZER, name, subtype, file_mgr->GetTagType()) {
     factory_func = arg_factory;
-    enabled = arg_enabled;
+    SetEnabled(arg_enabled);
 }
 
 void Component::Initialize() {
@@ -26,7 +26,7 @@ void Component::DoDescribe(ODesc* d) const {
         d->Add(", ");
     }
 
-    d->Add(enabled ? "enabled" : "disabled");
+    d->Add(Enabled() ? "enabled" : "disabled");
 }
 
 } // namespace zeek::file_analysis

src/file_analysis/Component.h

@@ -70,20 +70,6 @@ public:
      */
     factory_function FactoryFunction() const { return factory_func; }
 
-    /**
-     * Returns true if the analyzer is currently enabled and hence
-     * available for use.
-     */
-    bool Enabled() const { return enabled; }
-
-    /**
-     * Enables or disables this analyzer.
-     *
-     * @param arg_enabled True to enabled, false to disable.
-     *
-     */
-    void SetEnabled(bool arg_enabled) { enabled = arg_enabled; }
-
 protected:
     /**
      * Overridden from plugin::Component.
@@ -94,7 +80,6 @@ private:
     friend class Manager;
 
     factory_function factory_func; // The analyzer's factory callback.
-    bool enabled;                  // True if the analyzer is enabled.
 };
 
 } // namespace file_analysis

src/file_analysis/file_analysis.bif

@@ -41,7 +41,7 @@ function Files::__set_reassembly_buffer%(file_id: string, max: count%): bool
 ## :zeek:see:`Files::enable_analyzer`.
 function Files::__enable_analyzer%(tag: Files::Tag%) : bool
 	%{
-	auto c = zeek::file_mgr->Lookup(tag->AsEnumVal());
+	auto c = zeek::file_mgr->Lookup(tag->AsEnumVal(), false);
 
 	if ( ! c )
 		return zeek::val_mgr->False();
@@ -53,7 +53,7 @@ function Files::__enable_analyzer%(tag: Files::Tag%) : bool
 ## :zeek:see:`Files::disable_analyzer`.
 function Files::__disable_analyzer%(tag: Files::Tag%) : bool
 	%{
-	auto c = zeek::file_mgr->Lookup(tag->AsEnumVal());
+	auto c = zeek::file_mgr->Lookup(tag->AsEnumVal(), false);
 
 	if ( ! c )
 		return zeek::val_mgr->False();

src/packet_analysis/Component.cc

@@ -18,11 +18,23 @@ void Component::Initialize() {
 }
 
 void Component::SetEnabled(bool arg_enabled) {
-    enabled = arg_enabled;
+    auto analyzer = packet_mgr->GetAnalyzer(Tag().AsVal().get());
+    if ( analyzer ) {
+        // We can only toggle the analyzer if it's not replacing another one,
+        // otherwise our dispatching tables would be wrong.
+        if ( packet_mgr->ProvidesComponentMapping(Tag()) ) {
+            reporter->Warning(
+                "attempt to toggle packet analyzer %s, which replaces another one; toggling replacement analyzers is "
+                "not supported",
+                analyzer->GetAnalyzerName());
+            return;
+        }
 
-    // If we already have instantiated an analyzer, update its state.
+        // Update the existing analyzer's state.
-    if ( auto analyzer = packet_mgr->GetAnalyzer(Tag().AsVal().get()) )
+        analyzer->SetEnabled(arg_enabled);
-        analyzer->SetEnabled(enabled);
+    }
+
+    plugin::Component::SetEnabled(arg_enabled);
 }
 
 void Component::DoDescribe(ODesc* d) const {
@@ -32,5 +44,5 @@ void Component::DoDescribe(ODesc* d) const {
         d->Add(", ");
     }
 
-    d->Add(enabled ? "enabled" : "disabled");
+    d->Add(Enabled() ? "enabled" : "disabled");
 }

src/packet_analysis/Component.h

@@ -34,19 +34,7 @@ public:
      */
     factory_callback Factory() const { return factory; }
 
-    /**
+    void SetEnabled(bool arg_enabled) override;
-     * Returns true if the analyzer is currently enabled and hence
-     * available for use.
-     */
-    bool Enabled() const { return enabled; }
-
-    /**
-     * Enables or disables this analyzer.
-     *
-     * @param arg_enabled True to enabled, false to disable.
-     *
-     */
-    void SetEnabled(bool arg_enabled);
 
 protected:
     /**
@@ -56,7 +44,6 @@ protected:
 
 private:
     factory_callback factory; // The analyzer's factory callback.
-    bool enabled = true;      // True if the analyzer is enabled.
 };
 
 } // namespace zeek::packet_analysis

src/packet_analysis/Dispatcher.cc

@@ -42,7 +42,7 @@ void Dispatcher::Register(uint32_t identifier, AnalyzerPtr analyzer) {
     }
 
     int64_t index = identifier - lowest_identifier;
-    if ( table[index] != nullptr )
+    if ( table[index] != nullptr && table[index] != analyzer )
         reporter->Info("Overwriting packet analyzer mapping %#8" PRIx64 " => %s with %s", index + lowest_identifier,
                        table[index]->GetAnalyzerName(), analyzer->GetAnalyzerName());
     table[index] = std::move(analyzer);

src/packet_analysis/Manager.cc

@@ -82,14 +82,14 @@ AnalyzerPtr Manager::GetAnalyzer(const std::string& name) {
 }
 
 bool Manager::EnableAnalyzer(EnumVal* tag) {
-    Component* c = Lookup(tag);
+    Component* c = Lookup(tag, false);
     c->SetEnabled(true);
 
     return true;
 }
 
 bool Manager::DisableAnalyzer(EnumVal* tag) {
-    Component* c = Lookup(tag);
+    Component* c = Lookup(tag, false);
     c->SetEnabled(false);
 
     return true;
@@ -159,7 +159,7 @@ AnalyzerPtr Manager::InstantiateAnalyzer(const Tag& tag) {
         return nullptr;
     }
 
-    if ( tag != a->GetAnalyzerTag() ) {
+    if ( tag != a->GetAnalyzerTag() && ! HasComponentMapping(tag) ) {
         reporter->InternalError(
             "Mismatch of requested analyzer %s and instantiated analyzer %s. "
             "This usually means that the plugin author made a mistake.",

src/plugin/Component.cc

@@ -59,12 +59,30 @@ void Component::InitializeTag() {
     tag = zeek::Tag(etype, ++type_counter, tag_subtype);
 }
 
-/**
- * @return The component's tag.
- */
 zeek::Tag Component::Tag() const {
     assert(tag_initialized);
     return tag;
 }
 
+void Component::SetEnabled(bool arg_enabled) {
+    switch ( type ) {
+        case component::ANALYZER:
+        case component::PACKET_ANALYZER:
+        case component::FILE_ANALYZER:
+        case component::SESSION_ADAPTER:
+            // For these types we have logic in place to ignore the component
+            // if disabled.
+            enabled = arg_enabled;
+            break;
+
+        default:
+            // It wouldn't be hard to add support for other component types. We
+            // just need to make sure the enabled flag is checked somewhere to
+            // skip using the component if off.
+            ODesc d;
+            Describe(&d);
+            reporter->InternalError("SetEnabled() called on unsupported component (%s)", d.Description());
+            break;
+    }
+}
 } // namespace zeek::plugin

src/plugin/Component.h

@@ -118,6 +118,25 @@ public:
      */
     zeek::Tag Tag() const;
 
+    /**
+     * Returns true if the component is currently enabled and hence
+     * available for use.
+     */
+    bool Enabled() const { return enabled; }
+
+    /**
+     * Enables or disables this component. Derived classes may override this if
+     * they need to initiate additional actions, but must then call the base
+     * class version.
+     *
+     * @param arg_enabled True to enabled, false to disable.
+     *
+     * Note: This method is currently supported for protocol, file, and packet
+     * analyzers, as well as session adapters. Using it on other types of
+     * component will result in an internal error.
+     */
+    virtual void SetEnabled(bool arg_enabled);
+
 protected:
     /**
      * Adds type specific information to the output of Describe().
@@ -139,6 +158,7 @@ private:
     EnumTypePtr etype;
     Tag::subtype_t tag_subtype;
     bool tag_initialized = false;
+    bool enabled = true;
 
     /** Used to generate globally unique tags */
     static Tag::type_t type_counter;

src/plugin/ComponentManager.h

@@ -119,24 +119,59 @@ public:
 
     /**
      * @param name The canonical name of a component.
+     * @param consider_remappings If true, component mappings will be honored
+     * if the original component is disabled.
      * @return The component associated with the name or a null pointer if no
      * such component exists.
      */
-    C* Lookup(const std::string& name) const;
+    C* Lookup(const std::string& name, bool consider_remappings = true) const;
 
     /**
      * @param name A component tag.
+     * @param consider_remappings If true, component mappings will be honored
+     * if the original component is disabled.
      * @return The component associated with the tag or a null pointer if no
      * such component exists.
      */
-    C* Lookup(const zeek::Tag& tag) const;
+    C* Lookup(const zeek::Tag& tag, bool consider_remappings = true) const;
 
     /**
      * @param name A component's enum value.
+     * @param consider_remappings If true, component mappings will be honored
+     * if the original component is disabled.
      * @return The component associated with the value or a null pointer if no
      * such component exists.
      */
-    C* Lookup(EnumVal* val) const;
+    C* Lookup(EnumVal* val, bool consider_remappings = true) const;
+
+    /**
+     * Registers a mapping of a component to another one that will be honored
+     * by the `Lookup()` methods if (and only if) the original is currently
+     * disabled.
+     *
+     * @param old The original component tag.
+     * @param new_ The new component tag.
+     */
+    void AddComponentMapping(const zeek::Tag& old, const zeek::Tag& new_) {
+        if ( old != new_ ) {
+            component_mapping_by_src[old] = new_;
+            component_mapping_by_dst[new_] = old;
+        }
+    }
+
+    /**
+     * Returns true if a given component has a mapping to different one in place.
+     *
+     * @param tag The component tag to check.
+     */
+    auto HasComponentMapping(const zeek::Tag& tag) const { return component_mapping_by_src.count(tag); }
+
+    /**
+     * Returns true if a given component is mapped to from a different one.
+     *
+     * @param tag The component tag to check.
+     */
+    bool ProvidesComponentMapping(const zeek::Tag& tag) const { return component_mapping_by_dst.count(tag); }
 
 private:
     /** Script layer module in which component tags live. */
@@ -150,6 +185,8 @@ private:
     std::map<std::string, C*> components_by_name;
     std::map<zeek::Tag, C*> components_by_tag;
     std::map<zeek_int_t, C*> components_by_val;
+    std::map<zeek::Tag, zeek::Tag> component_mapping_by_src;
+    std::map<zeek::Tag, zeek::Tag> component_mapping_by_dst;
 };
 
 template<class C>
@@ -204,7 +241,7 @@ const std::string& ComponentManager<C>::GetComponentName(zeek::Tag tag) const {
     if ( ! tag )
         return error;
 
-    if ( C* c = Lookup(tag) )
+    if ( C* c = Lookup(tag, false) ) // use actual, not remapped name
         return c->CanonicalName();
 
     reporter->InternalWarning("requested name of unknown component tag %s", tag.AsString().c_str());
@@ -266,21 +303,48 @@ zeek::Tag ComponentManager<C>::GetComponentTag(Val* v) const {
 }
 
 template<class C>
-C* ComponentManager<C>::Lookup(const std::string& name) const {
+C* ComponentManager<C>::Lookup(const std::string& name, bool consider_remappings) const {
-    typename std::map<std::string, C*>::const_iterator i = components_by_name.find(util::to_upper(name));
+    if ( auto i = components_by_name.find(util::to_upper(name)); i != components_by_name.end() ) {
-    return i != components_by_name.end() ? i->second : nullptr;
+        auto c = (*i).second;
+        if ( consider_remappings && ! c->Enabled() ) {
+            if ( auto j = component_mapping_by_src.find(c->Tag()); j != component_mapping_by_src.end() )
+                return Lookup(j->second, false);
+        }
+
+        return c;
+    }
+    else
+        return nullptr;
 }
 
 template<class C>
-C* ComponentManager<C>::Lookup(const zeek::Tag& tag) const {
+C* ComponentManager<C>::Lookup(const zeek::Tag& tag, bool consider_remappings) const {
-    typename std::map<zeek::Tag, C*>::const_iterator i = components_by_tag.find(tag);
+    if ( auto i = components_by_tag.find(tag); i != components_by_tag.end() ) {
-    return i != components_by_tag.end() ? i->second : nullptr;
+        auto c = (*i).second;
+        if ( consider_remappings && ! c->Enabled() ) {
+            if ( auto j = component_mapping_by_src.find(c->Tag()); j != component_mapping_by_src.end() )
+                return Lookup(j->second, false);
+        }
+
+        return c;
+    }
+    else
+        return nullptr;
 }
 
 template<class C>
-C* ComponentManager<C>::Lookup(EnumVal* val) const {
+C* ComponentManager<C>::Lookup(EnumVal* val, bool consider_remappings) const {
-    typename std::map<zeek_int_t, C*>::const_iterator i = components_by_val.find(val->InternalInt());
+    if ( auto i = components_by_val.find(val->InternalInt()); i != components_by_val.end() ) {
-    return i != components_by_val.end() ? i->second : nullptr;
+        auto c = (*i).second;
+        if ( consider_remappings && ! c->Enabled() ) {
+            if ( auto j = component_mapping_by_src.find(c->Tag()); j != component_mapping_by_src.end() )
+                return Lookup(j->second, false);
+        }
+
+        return c;
+    }
+    else
+        return nullptr;
 }
 
 template<class C>

src/spicy/manager.cc

@@ -119,9 +119,9 @@ void Manager::registerProtocolAnalyzer(const std::string& name, hilti::rt::Proto
 
     trackComponent(c, c->Tag().Type()); // Must come after Initialize().
 
-    info.type = c->Tag().Type();
+    info.tag = c->Tag();
-    _protocol_analyzers_by_type.resize(info.type + 1);
+    _protocol_analyzers_by_type.resize(info.tag.Type() + 1);
-    _protocol_analyzers_by_type[info.type] = info;
+    _protocol_analyzers_by_type[info.tag.Type()] = info;
 }
 
 void Manager::registerFileAnalyzer(const std::string& name, const hilti::rt::Vector<std::string>& mime_types,
@@ -165,9 +165,9 @@ void Manager::registerFileAnalyzer(const std::string& name, const hilti::rt::Vec
 
     trackComponent(c, c->Tag().Type()); // Must come after Initialize().
 
-    info.type = c->Tag().Type();
+    info.tag = c->Tag();
-    _file_analyzers_by_type.resize(info.type + 1);
+    _file_analyzers_by_type.resize(info.tag.Type() + 1);
-    _file_analyzers_by_type[info.type] = info;
+    _file_analyzers_by_type[info.tag.Type()] = info;
 }
 
 void Manager::registerPacketAnalyzer(const std::string& name, const std::string& parser, const std::string& replaces,
@@ -214,9 +214,9 @@ void Manager::registerPacketAnalyzer(const std::string& name, const std::string&
 
     trackComponent(c, c->Tag().Type()); // Must come after Initialize().
 
-    info.type = c->Tag().Type();
+    info.tag = c->Tag();
-    _packet_analyzers_by_type.resize(info.type + 1);
+    _packet_analyzers_by_type.resize(info.tag.Type() + 1);
-    _packet_analyzers_by_type[info.type] = info;
+    _packet_analyzers_by_type[info.tag.Type()] = info;
 }
 
 void Manager::registerType(const std::string& id, const TypePtr& type) {
@@ -337,7 +337,7 @@ bool Manager::toggleProtocolAnalyzer(const Tag& tag, bool enable) {
 
     const auto& analyzer = _protocol_analyzers_by_type[type];
 
-    if ( ! analyzer.type )
+    if ( ! analyzer.tag )
         // not set -> not ours
         return false;
 
@@ -371,12 +371,13 @@ bool Manager::toggleFileAnalyzer(const Tag& tag, bool enable) {
 
     const auto& analyzer = _file_analyzers_by_type[type];
 
-    if ( ! analyzer.type )
+    if ( ! analyzer.tag )
         // not set -> not ours
         return false;
 
-    file_analysis::Component* component = file_mgr->Lookup(tag);
+    file_analysis::Component* component = file_mgr->Lookup(tag, false);
-    file_analysis::Component* component_replaces = analyzer.replaces ? file_mgr->Lookup(analyzer.replaces) : nullptr;
+    file_analysis::Component* component_replaces =
+        analyzer.replaces ? file_mgr->Lookup(analyzer.replaces, false) : nullptr;
 
     if ( ! component ) {
         // Shouldn't really happen.
@@ -414,13 +415,13 @@ bool Manager::togglePacketAnalyzer(const Tag& tag, bool enable) {
 
     const auto& analyzer = _packet_analyzers_by_type[type];
 
-    if ( ! analyzer.type )
+    if ( ! analyzer.tag )
         // not set -> not ours
         return false;
 
-    packet_analysis::Component* component = packet_mgr->Lookup(tag);
+    packet_analysis::Component* component = packet_mgr->Lookup(tag, false);
     packet_analysis::Component* component_replaces =
-        analyzer.replaces ? packet_mgr->Lookup(analyzer.replaces) : nullptr;
+        analyzer.replaces ? packet_mgr->Lookup(analyzer.replaces, false) : nullptr;
 
     if ( ! component ) {
         // Shouldn't really happen.
@@ -452,21 +453,21 @@ bool Manager::togglePacketAnalyzer(const Tag& tag, bool enable) {
 
 bool Manager::toggleAnalyzer(EnumVal* tag, bool enable) {
     if ( tag->GetType() == analyzer_mgr->GetTagType() ) {
-        if ( auto analyzer = analyzer_mgr->Lookup(tag) )
+        if ( auto analyzer = analyzer_mgr->Lookup(tag, false) )
             return toggleProtocolAnalyzer(analyzer->Tag(), enable);
         else
             return false;
     }
 
     if ( tag->GetType() == file_mgr->GetTagType() ) {
-        if ( auto analyzer = file_mgr->Lookup(tag) )
+        if ( auto analyzer = file_mgr->Lookup(tag, false) )
             return toggleFileAnalyzer(analyzer->Tag(), enable);
         else
             return false;
     }
 
     if ( tag->GetType() == packet_mgr->GetTagType() ) {
-        if ( auto analyzer = packet_mgr->Lookup(tag) )
+        if ( auto analyzer = packet_mgr->Lookup(tag, false) )
             return togglePacketAnalyzer(analyzer->Tag(), enable);
         else
             return false;
@@ -686,7 +687,7 @@ void Manager::InitPostScript() {
     };
 
     for ( auto& p : _protocol_analyzers_by_type ) {
-        if ( p.type == 0 )
+        if ( ! p.tag )
             // vector element not set
             continue;
 
@@ -732,7 +733,7 @@ void Manager::InitPostScript() {
     }
 
     for ( auto& p : _file_analyzers_by_type ) {
-        if ( p.type == 0 )
+        if ( ! p.tag )
             // vector element not set
             continue;
 
@@ -767,7 +768,7 @@ void Manager::InitPostScript() {
     }
 
     for ( auto& p : _packet_analyzers_by_type ) {
-        if ( p.type == 0 )
+        if ( ! p.tag )
             // vector element not set
             continue;
 
@@ -893,7 +894,7 @@ void Manager::disableReplacedAnalyzers() {
 
         auto replaces = info.name_replaces.c_str();
 
-        if ( file_mgr->Lookup(replaces) || packet_mgr->Lookup(replaces) )
+        if ( file_mgr->Lookup(replaces, false) || packet_mgr->Lookup(replaces, false) )
             reporter->FatalError("cannot replace '%s' analyzer with a protocol analyzer", replaces);
 
         auto tag = analyzer_mgr->GetAnalyzerTag(replaces);
@@ -907,6 +908,7 @@ void Manager::disableReplacedAnalyzers() {
         SPICY_DEBUG(hilti::rt::fmt("%s replaces existing protocol analyzer %s", info.name_analyzer, replaces));
         info.replaces = tag;
         analyzer_mgr->DisableAnalyzer(tag);
+        analyzer_mgr->AddComponentMapping(tag, info.tag);
     }
 
     for ( auto& info : _file_analyzers_by_type ) {
@@ -915,10 +917,10 @@ void Manager::disableReplacedAnalyzers() {
 
         auto replaces = info.name_replaces.c_str();
 
-        if ( analyzer_mgr->Lookup(replaces) || packet_mgr->Lookup(replaces) )
+        if ( analyzer_mgr->Lookup(replaces, false) || packet_mgr->Lookup(replaces, false) )
             reporter->FatalError("cannot replace '%s' analyzer with a file analyzer", replaces);
 
-        auto component = file_mgr->Lookup(replaces);
+        auto component = file_mgr->Lookup(replaces, false);
         if ( ! component ) {
             SPICY_DEBUG(hilti::rt::fmt("%s is supposed to replace file analyzer %s, but that does not exist",
                                        info.name_analyzer, replaces));
@@ -929,6 +931,7 @@ void Manager::disableReplacedAnalyzers() {
         SPICY_DEBUG(hilti::rt::fmt("%s replaces existing file analyzer %s", info.name_analyzer, replaces));
         info.replaces = component->Tag();
         component->SetEnabled(false);
+        file_mgr->AddComponentMapping(component->Tag(), info.tag);
     }
 
     for ( auto& info : _packet_analyzers_by_type ) {
@@ -937,7 +940,7 @@ void Manager::disableReplacedAnalyzers() {
 
         auto replaces = info.name_replaces.c_str();
 
-        auto component = packet_mgr->Lookup(replaces);
+        auto component = packet_mgr->Lookup(replaces, false);
         if ( ! component ) {
             SPICY_DEBUG(hilti::rt::fmt("%s is supposed to replace packet analyzer %s, but that does not exist",
                                        info.name_analyzer, replaces));
@@ -948,6 +951,7 @@ void Manager::disableReplacedAnalyzers() {
         SPICY_DEBUG(hilti::rt::fmt("%s replaces existing packet analyzer %s", info.name_analyzer, replaces));
         info.replaces = component->Tag();
         component->SetEnabled(false);
+        packet_mgr->AddComponentMapping(component->Tag(), info.tag);
     }
 }
 

src/spicy/manager.h

@@ -264,9 +264,6 @@ public:
      * loaded will also be activated. By calling this method, an analyzer can
      * toggled.
      *
-     * @note This is currently not supported because Zeek does not provide the
-     * necessary API.
-     *
      * @param analyzer tag of analyzer
      * @param enable true to enable, false to disable
      */
@@ -352,7 +349,7 @@ private:
         // Computed and available once the analyzer has been registered.
         std::string name_zeek;
         std::string name_zeekygen;
-        Tag::type_t type;
+        Tag tag;
         const ::spicy::rt::Parser* parser_orig;
         const ::spicy::rt::Parser* parser_resp;
         Tag replaces;
@@ -378,7 +375,7 @@ private:
         // Computed and available once the analyzer has been registered.
         std::string name_zeek;
         std::string name_zeekygen;
-        Tag::type_t type;
+        Tag tag;
         const ::spicy::rt::Parser* parser;
         Tag replaces;
 
@@ -402,7 +399,7 @@ private:
         // Computed and available once the analyzer has been registered.
         std::string name_zeek;
         std::string name_zeekygen;
-        Tag::type_t type;
+        Tag tag;
         const ::spicy::rt::Parser* parser;
         Tag replaces;
 

testing/btest/Baseline/spicy.packet-analyzer-replaces/output

@@ -1,2 +1,3 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
 My Ethernet:, \x00\x10\xdcrL_\x00\xd0\xb7\x1e\xbe \x08\x00
+UDP:, 10.20.1.31, 53/udp, 207.158.192.40, 53/udp

testing/btest/Baseline/spicy.replaces/output

@@ -2,3 +2,6 @@
 Analyzer::ANALYZER_SSH, 3
 SSH banner, [orig_h=192.150.186.169, orig_p=49244/tcp, resp_h=131.159.14.23, resp_p=22/tcp], F, 1.99, OpenSSH_3.9p1
 SSH banner, [orig_h=192.150.186.169, orig_p=49244/tcp, resp_h=131.159.14.23, resp_p=22/tcp], T, 2.0, OpenSSH_3.8.1p1
+Analyzer::ANALYZER_SSH, 6
+SSH banner, [orig_h=172.16.238.1, orig_p=49656/tcp, resp_h=172.16.238.131, resp_p=80/tcp], F, 2.0, OpenSSH_5.8p1 Debian-1ubuntu3
+SSH banner, [orig_h=172.16.238.1, orig_p=49656/tcp, resp_h=172.16.238.131, resp_p=80/tcp], T, 2.0, OpenSSH_5.2

testing/btest/spicy/packet-analyzer-replaces.zeek

@@ -1,35 +1,17 @@
 # @TEST-REQUIRES: have-spicy
 #
 # @TEST-EXEC: spicyz -d -o my-ethernet.hlto my-ethernet.spicy my-ethernet.evt
-# @TEST-EXEC: zeek -r ${TRACES}/dns53.pcap my-ethernet.hlto %INPUT ENABLE=T >output-on
+# @TEST-EXEC: zeek -r ${TRACES}/dns53.pcap my-ethernet.hlto %INPUT >output
-# @TEST-EXEC: zeek -r ${TRACES}/dns53.pcap my-ethernet.hlto %INPUT ENABLE=F >output-off
+# @TEST-EXEC: btest-diff output
-# @TEST-EXEC: btest-diff output-on
-
 #
 # @TEST-DOC: Check that we can replace Zeek's Ethernet analyzer.
-#
-# Zeek logs look the same in both cases but we get some additional output
-# when our analyzer is running by raising a custom event.
-
-const ENABLE = T &redef;
 
 module MyEthernet;
 
 const DLT_EN10MB : count = 1;
 
-event zeek_init() &priority=-200
+event zeek_init()
 	{
-	if ( ENABLE )
-		Spicy::enable_file_analyzer(PacketAnalyzer::ANALYZER_SPICY_MYETHERNET);
-	else
-		Spicy::disable_file_analyzer(PacketAnalyzer::ANALYZER_SPICY_MYETHERNET);
-}
-
-# The priority here needs to be higher than the standard script registering the
-# built-in Ethernet analyzer.
-event zeek_init() &priority=-100
-	{
-	PacketAnalyzer::register_packet_analyzer(PacketAnalyzer::ANALYZER_ROOT, DLT_EN10MB, PacketAnalyzer::ANALYZER_SPICY_MYETHERNET);
 	PacketAnalyzer::register_packet_analyzer(PacketAnalyzer::ANALYZER_SPICY_MYETHERNET, 0x0800, PacketAnalyzer::ANALYZER_IP);
 	}
 
@@ -38,6 +20,11 @@ event MyEthernet::data(p: raw_pkt_hdr, data: string)
 	print "My Ethernet:", data;
 	}
 
+event udp_request(u: connection)
+	{
+	print "UDP:", u$id$orig_h, u$id$orig_p, u$id$resp_h, u$id$resp_p;
+	}
+
 # @TEST-START-FILE my-ethernet.spicy
 module MyEthernet;
 

testing/btest/spicy/replaces.zeek

@@ -2,28 +2,21 @@
 #
 # @TEST-EXEC: mkdir -p modules
 # @TEST-EXEC: spicyz -d -o modules/ssh.hlto ssh.spicy ./ssh.evt
-# @TEST-EXEC: ZEEK_SPICY_MODULE_PATH=$(pwd)/modules zeek -r ${TRACES}/ssh/single-conn.trace %INPUT | sort >output
+# @TEST-EXEC: ZEEK_SPICY_MODULE_PATH=$(pwd)/modules zeek -r ${TRACES}/ssh/single-conn.trace %INPUT | sort >>output
+# @TEST-EXEC: ZEEK_SPICY_MODULE_PATH=$(pwd)/modules zeek -r ${TRACES}/ssh/ssh-on-port-80.trace %INPUT | sort >>output
 # @TEST-EXEC: btest-diff output
 #
 # We use the module search path for loading here as a regression test for #137.
 # Note that this that problem only showed up when the Spicy plugin was built
 # into Zeek.
-#
+
-# XXX: Replaces is kin of borked. "replaces" probably should inherit/use
-#      ports previously registered through Analyzer::register_for_port() for
-#      the analyzer that is being replaced, but that doesn't seem to be
-#      happening. Having ports previosly in .evt "worked around it" mostly.
-#
-#      This seems pretty much #3573.
-#
 event zeek_init()
 	{
-	Analyzer::register_for_port(Analyzer::ANALYZER_SPICY_SSH, 22/tcp);
+	# Reuse existing analyzer's port.
+	Analyzer::register_for_port(Analyzer::ANALYZER_SSH, 22/tcp);
 
-	# The following should maybe "do the right thing" when using replaces
+	# Add our own port.
-	# if we fiddle with the underlying enum value?
+	Analyzer::register_for_port(Analyzer::ANALYZER_SPICY_SSH, 80/tcp);
-	#
-	# Analyzer::register_for_port(Analyzer::ANALYZER_SSH, 22/tcp);
 	}
 
 event ssh::banner(c: connection, is_orig: bool, version: string, software: string)