From 8acf9953615701d628bfbfef5e581a5fe0e5eb3b Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Thu, 29 Sep 2016 16:06:43 -0700 Subject: [PATCH] Fixing Broxygen indexing confusion for plugins. Because plugins register their script directories with the BROPATH, Broxygen stripped them out from plugin script paths it was indexing. That then led to multiple plugins ending up with the same script paths, triggering warnings about duplicates. I fixed this by checking if a script comes out of a plugin. If so, it gets an artifcial index prefix ":", followed by the script's relative path inside the plugin's top-level directory. For example, "/opt/bro/lib/bro/plugins/Bro_Netmap/scripts/init.bro" now turns into "Bro::Netmap:scripts/init.bro" for Broxygen purposes (whereas it used to be just "init.bro"). Addresses BIT-1663. (Can't think of a good way to add a test for this unfortunately.) --- src/broxygen/Manager.cc | 32 +++++++++++++++++++++++--------- src/plugin/Manager.cc | 29 ++++++++++++++++++++++++++++- src/plugin/Manager.h | 10 ++++++++++ 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/src/broxygen/Manager.cc b/src/broxygen/Manager.cc index 3a07191f33..6be16f5cc3 100644 --- a/src/broxygen/Manager.cc +++ b/src/broxygen/Manager.cc @@ -1,6 +1,7 @@ // See the file "COPYING" in the main distribution directory for copyright. #include "Manager.h" +#include "plugin/Manager.h" #include "util.h" #include @@ -38,6 +39,19 @@ static string RemoveLeadingSpace(const string& s) return rval; } +// Turns a script's full path into a shortened, normalized version that we +// use for indexing. +static string NormalizeScriptPath(const string& path) + { + if ( auto p = plugin_mgr->LookupPluginByPath(path) ) + { + auto rval = normalize_path(path); + return p->Name() + ":" + rval.substr(p->PluginDirectory().size() + 1); + } + + return without_bropath_component(path); + } + Manager::Manager(const string& arg_config, const string& bro_command) : disabled(), comment_buffer(), comment_buffer_map(), packages(), scripts(), identifiers(), all_info(), last_identifier_seen(), incomplete_type(), @@ -108,7 +122,7 @@ void Manager::Script(const string& path) if ( disabled ) return; - string name = without_bropath_component(path); + string name = NormalizeScriptPath(path); if ( scripts.GetInfo(name) ) { @@ -149,8 +163,8 @@ void Manager::ScriptDependency(const string& path, const string& dep) return; } - string name = without_bropath_component(path); - string depname = without_bropath_component(dep); + string name = NormalizeScriptPath(path); + string depname = NormalizeScriptPath(dep); ScriptInfo* script_info = scripts.GetInfo(name); if ( ! script_info ) @@ -174,7 +188,7 @@ void Manager::ModuleUsage(const string& path, const string& module) if ( disabled ) return; - string name = without_bropath_component(path); + string name = NormalizeScriptPath(path); ScriptInfo* script_info = scripts.GetInfo(name); if ( ! script_info ) @@ -225,7 +239,7 @@ void Manager::StartType(ID* id) return; } - string script = without_bropath_component(id->GetLocationInfo()->filename); + string script = NormalizeScriptPath(id->GetLocationInfo()->filename); ScriptInfo* script_info = scripts.GetInfo(script); if ( ! script_info ) @@ -289,7 +303,7 @@ void Manager::Identifier(ID* id) return; } - string script = without_bropath_component(id->GetLocationInfo()->filename); + string script = NormalizeScriptPath(id->GetLocationInfo()->filename); ScriptInfo* script_info = scripts.GetInfo(script); if ( ! script_info ) @@ -318,7 +332,7 @@ void Manager::RecordField(const ID* id, const TypeDecl* field, return; } - string script = without_bropath_component(path); + string script = NormalizeScriptPath(path); idd->AddRecordField(field, script, comment_buffer); comment_buffer.clear(); DBG_LOG(DBG_BROXYGEN, "Document record field %s, identifier %s, script %s", @@ -343,7 +357,7 @@ void Manager::Redef(const ID* id, const string& path) return; } - string from_script = without_bropath_component(path); + string from_script = NormalizeScriptPath(path); ScriptInfo* script_info = scripts.GetInfo(from_script); if ( ! script_info ) @@ -365,7 +379,7 @@ void Manager::SummaryComment(const string& script, const string& comment) if ( disabled ) return; - string name = without_bropath_component(script); + string name = NormalizeScriptPath(script); ScriptInfo* info = scripts.GetInfo(name); if ( info ) diff --git a/src/plugin/Manager.cc b/src/plugin/Manager.cc index 9ce7fb27dd..e42da20d11 100644 --- a/src/plugin/Manager.cc +++ b/src/plugin/Manager.cc @@ -240,6 +240,8 @@ bool Manager::ActivateDynamicPluginInternal(const std::string& name, bool ok_if_ current_plugin->DoConfigure(); current_plugin->InitializeComponents(); + plugins_by_path.insert(std::make_pair(normalize_path(dir), current_plugin)); + if ( current_plugin->APIVersion() != BRO_PLUGIN_API_VERSION ) reporter->FatalError("plugin's API version does not match Bro (expected %d, got %d in %s)", BRO_PLUGIN_API_VERSION, current_plugin->APIVersion(), path); @@ -327,7 +329,7 @@ void Manager::RegisterPlugin(Plugin *plugin) if ( current_dir && current_sopath ) // A dynamic plugin, record its location. - plugin->SetPluginLocation(current_dir, current_sopath); + plugin->SetPluginLocation(normalize_path(current_dir), current_sopath); current_plugin = plugin; } @@ -462,6 +464,31 @@ Manager::bif_init_func_map* Manager::BifFilesInternal() return bifs; } +Plugin* Manager::LookupPluginByPath(std::string path) + { + path = normalize_path(path); + + if ( is_file(path) ) + path = SafeDirname(path).result; + + while ( path.size() ) + { + auto i = plugins_by_path.find(path); + + if ( i != plugins_by_path.end() ) + return i->second; + + auto j = path.rfind("/"); + + if ( j == std::string::npos ) + break; + + path.erase(j); + } + + return nullptr; + } + static bool hook_cmp(std::pair a, std::pair b) { if ( a.first == b.first ) diff --git a/src/plugin/Manager.h b/src/plugin/Manager.h index 04c632d61a..2a394b39ee 100644 --- a/src/plugin/Manager.h +++ b/src/plugin/Manager.h @@ -149,6 +149,13 @@ public: */ template std::list Components() const; + /** + * Returns the (dynamic) plugin associated with a given filesytem + * path. The path can be the plugin directory itself, or any path + * inside it. + */ + Plugin* LookupPluginByPath(std::string path); + /** * Returns true if there's at least one plugin interested in a given * hook. @@ -329,6 +336,9 @@ private: // of that type enabled. hook_list** hooks; + // A map of all the top-level plugin directories. + std::map plugins_by_path; + // Helpers providing access to current state during dlopen(). static Plugin* current_plugin; static const char* current_dir;