Merge remote-tracking branch 'origin/topic/timw/dict-templates-redux'

* origin/topic/timw/dict-templates-redux:
  Update NEWS with breakage/deprecation notes about Dictionary changes [nomail] [skip ci]
  Remove unnecssary includes of Dict.h
  Turn PDict into a type alias
  Convert Dictionary types to be templated classes
  Fix signedness comparison warning in ssl analyzer

CHANGES

@@ -1,3 +1,33 @@
+5.1.0-dev.186 | 2022-07-05 22:18:52 +0000
+
+  * Remove unnecssary includes of Dict.h (Tim Wojtulewicz, Corelight)
+
+  * Turn PDict into a type alias (Tim Wojtulewicz, Corelight)
+
+  * Convert Dictionary types to be templated classes (Tim Wojtulewicz, Corelight)
+
+    This has the fortunate side-effect of also making it so we can store
+    the value objects as typed pointers, instead of void*.
+
+  * Fix signedness comparison warning in ssl analyzer (Tim Wojtulewicz, Corelight)
+
+  * Management framework: make artifact filenames Github-compatible (Christian Kreibich, Corelight)
+
+    The upload-artifact action refuses to process certain filenames:
+
+      "Invalid characters include: Double quote ", Colon :, Less than <,
+       Greater than >, Vertical bar |, Asterisk *, Question mark ?, Carriage
+       return \r, Line feed \n"
+
+    On failure, this iterates over the btest artifacts in the testsuite's .tmp
+    directory and renames any of these to dots.
+
+  * Management framework: mask testsuite result (Christian Kreibich, Corelight)
+
+    There's currently a race condition in the agent-supervisor interaction that can
+    cause requests to the Supervisor to get lost. This temporarily masks the
+    testsuite result until I've had a chance to investigate.
+
 5.1.0-dev.177 | 2022-07-05 15:03:12 +0100
 
   * SSL Analyzer: track connection direction by messages (Johanna Amann)

NEWS

@@ -14,10 +14,13 @@ Breaking Changes
   variable was added to cover the transport layer. See this GitHub issue for
   more detail: https://github.com/zeek/zeek/issues/2183.
 
-- The STREAM mode of the ASCII reader now behaves like `tail -F`: when file is 
+- The STREAM mode of the ASCII reader now behaves like `tail -F`: when file is
   removed/replaced, it will start tracking the new file. See
   https://github.com/zeek/zeek/pull/2097 for more detail
 
+- The Dictionary and PDict classes are now C++ templates. This may cause
+  plugin/package builds to fail due to needing to modify uses of them to match.
+
 New Functionality
 -----------------
 
@@ -38,6 +41,9 @@ Changed Functionality
 Deprecated Functionality
 ------------------------
 
+- The PDict class is now an alias to Dictionary and has been deprecated. Use Dictionary
+  directly, passing a pointer type to the template.
+
 Zeek 5.0.0
 ==========
 

VERSION

@@ -1 +1 @@
-5.1.0-dev.177
+5.1.0-dev.186

src/CompHash.cc

@@ -8,7 +8,6 @@
 #include <map>
 #include <vector>
 
-#include "zeek/Dict.h"
 #include "zeek/Func.h"
 #include "zeek/IPAddr.h"
 #include "zeek/RE.h"

src/Conn.h

@@ -7,7 +7,6 @@
 #include <tuple>
 #include <type_traits>
 
-#include "zeek/Dict.h"
 #include "zeek/IPAddr.h"
 #include "zeek/IntrusivePtr.h"
 #include "zeek/Rule.h"

src/EventTrace.h

@@ -3,6 +3,7 @@
 #pragma once
 
 #include "zeek/Val.h"
+#include "zeek/ZeekArgs.h"
 
 namespace zeek::detail
 	{

src/ID.cc

@@ -6,7 +6,6 @@
 
 #include "zeek/Attr.h"
 #include "zeek/Desc.h"
-#include "zeek/Dict.h"
 #include "zeek/EventRegistry.h"
 #include "zeek/Expr.h"
 #include "zeek/File.h"

src/Stmt.cc

@@ -1331,7 +1331,7 @@ ValPtr ForStmt::DoExec(Frame* f, Val* v, StmtFlowType& flow)
 		for ( const auto& lve : *loop_vals )
 			{
 			auto k = lve.GetHashKey();
-			auto* current_tev = lve.GetValue<TableEntryVal*>();
+			auto* current_tev = lve.value;
 			auto ind_lv = tv->RecreateIndex(*k);
 
 			if ( value_var )

src/Val.cc

@@ -493,13 +493,10 @@ static void BuildJSON(threading::formatter::JSON::NullDoubleWriter& writer, Val*
 			else
 				writer.StartObject();
 
-			std::unique_ptr<detail::HashKey> k;
-			TableEntryVal* entry;
-
 			for ( const auto& te : *table )
 				{
-				entry = te.GetValue<TableEntryVal*>();
+				auto* entry = te.value;
-				k = te.GetHashKey();
+				auto k = te.GetHashKey();
 
 				auto lv = tval->RecreateIndex(*k);
 				Val* entry_key = lv->Length() == 1 ? lv->Idx(0).get() : lv.get();
@@ -1465,7 +1462,7 @@ int TableVal::RecursiveSize() const
 
 	for ( const auto& ve : *table_val )
 		{
-		auto* tv = ve.GetValue<TableEntryVal*>();
+		auto* tv = ve.value;
 		if ( tv->GetVal() )
 			n += tv->GetVal()->AsTableVal()->RecursiveSize();
 		}
@@ -1634,7 +1631,7 @@ bool TableVal::AddTo(Val* val, bool is_first_init, bool propagate_ops) const
 	for ( const auto& tble : *table_val )
 		{
 		auto k = tble.GetHashKey();
-		auto* v = tble.GetValue<TableEntryVal*>();
+		auto* v = tble.value;
 
 		if ( is_first_init && t->AsTable()->Lookup(k.get()) )
 			{
@@ -2255,7 +2252,7 @@ std::unordered_map<ValPtr, ValPtr> TableVal::ToMap() const
 	for ( const auto& iter : *table_val )
 		{
 		auto k = iter.GetHashKey();
-		auto v = iter.GetValue<TableEntryVal*>();
+		auto v = iter.value;
 		auto vl = table_hash->RecoverVals(*k);
 
 		res[std::move(vl)] = v->GetVal();
@@ -2298,7 +2295,7 @@ void TableVal::Describe(ODesc* d) const
 			reporter->InternalError("hash table underflow in TableVal::Describe");
 
 		auto k = iter->GetHashKey();
-		auto* v = iter->GetValue<TableEntryVal*>();
+		auto* v = iter->value;
 
 		auto vl = table_hash->RecoverVals(*k);
 		int dim = vl->Length();
@@ -2445,7 +2442,7 @@ void TableVal::DoExpire(double t)
 	      i < zeek::detail::table_incremental_step && *expire_iterator != table_val->end_robust();
 	      ++i, ++(*expire_iterator) )
 		{
-		auto v = (*expire_iterator)->GetValue<TableEntryVal*>();
+		auto v = (*expire_iterator)->value;
 
 		if ( v->ExpireAccessTime() == 0 )
 			{
@@ -2624,7 +2621,7 @@ ValPtr TableVal::DoClone(CloneState* state)
 	for ( const auto& tble : *table_val )
 		{
 		auto key = tble.GetHashKey();
-		auto* val = tble.GetValue<TableEntryVal*>();
+		auto* val = tble.value;
 		TableEntryVal* nval = val->Clone(state);
 		tv->table_val->Insert(key.get(), nval);
 
@@ -2664,7 +2661,7 @@ unsigned int TableVal::ComputeFootprint(std::unordered_set<const Val*>* analyzed
 		{
 		auto k = iter.GetHashKey();
 		auto vl = table_hash->RecoverVals(*k);
-		auto v = iter.GetValue<TableEntryVal*>()->GetVal();
+		auto v = iter.value->GetVal();
 
 		fp += vl->Footprint(analyzed_vals);
 		if ( v )
@@ -2711,7 +2708,7 @@ TableVal::ParseTimeTableState TableVal::DumpTableState()
 	for ( const auto& tble : *table_val )
 		{
 		auto key = tble.GetHashKey();
-		auto* val = tble.GetValue<TableEntryVal*>();
+		auto* val = tble.value;
 
 		rval.emplace_back(RecreateIndex(*key), val->GetVal());
 		}

src/Val.h

@@ -8,7 +8,6 @@
 #include <unordered_map>
 #include <vector>
 
-#include "zeek/Dict.h"
 #include "zeek/IntrusivePtr.h"
 #include "zeek/Notifier.h"
 #include "zeek/Reporter.h"
@@ -38,6 +37,10 @@ class RE_Matcher;
 class File;
 using FilePtr = zeek::IntrusivePtr<File>;
 
+template <typename T> class RobustDictIterator;
+template <typename T> class Dictionary;
+template <typename T> using PDict = Dictionary<T>;
+
 namespace detail
 	{
 
@@ -1044,7 +1047,7 @@ protected:
 	detail::ExprPtr expire_time;
 	detail::ExprPtr expire_func;
 	TableValTimer* timer;
-	RobustDictIterator* expire_iterator;
+	RobustDictIterator<TableEntryVal>* expire_iterator;
 	detail::PrefixTable* subnets;
 	ValPtr def_val;
 	detail::ExprPtr change_func;

src/analyzer/Manager.h

@@ -23,7 +23,6 @@
 #include <queue>
 #include <vector>
 
-#include "zeek/Dict.h"
 #include "zeek/IP.h"
 #include "zeek/Tag.h"
 #include "zeek/analyzer/Analyzer.h"

src/analyzer/protocol/ssl/SSL.cc

@@ -374,7 +374,7 @@ bool SSL_Analyzer::TryDecryptApplicationData(int len, const u_char* data, bool i
 		EVP_DecryptUpdate(ctx, NULL, &decrypted_len, s_aead_tag.data(), s_aead_tag.size());
 		EVP_DecryptUpdate(ctx, decrypted.data(), &decrypted_len, (const u_char*)encrypted,
 		                  encrypted_len);
-		assert(decrypted_len <= decrypted.size());
+		assert(static_cast<decltype(decrypted.size())>(decrypted_len) <= decrypted.size());
 		decrypted.resize(decrypted_len);
 
 		int res = 0;

src/broker/Data.cc

@@ -924,8 +924,6 @@ broker::expected<broker::data> val_to_data(const Val* v)
 			for ( const auto& te : *table )
 				{
 				auto hk = te.GetHashKey();
-				auto* entry = te.GetValue<TableEntryVal*>();
-
 				auto vl = table_val->RecreateIndex(*hk);
 
 				broker::vector composite_key;
@@ -952,7 +950,7 @@ broker::expected<broker::data> val_to_data(const Val* v)
 					get<broker::set>(rval).emplace(move(key));
 				else
 					{
-					auto val = val_to_data(entry->GetVal().get());
+					auto val = val_to_data(te.value->GetVal().get());
 
 					if ( ! val )
 						return broker::ec::invalid_data;

src/broker/messaging.bif

@@ -37,7 +37,6 @@ std::set<std::string> val_to_topic_set(zeek::Val* val)
 		for ( const auto& te : *tbl )
 			{
 			auto k = te.GetHashKey();
-			auto* v = te.GetValue<zeek::TableEntryVal*>();
 
 			auto index = val->AsTableVal()->RecreateIndex(*k);
 			rval.emplace(index->Idx(0)->AsString()->CheckString());

src/file_analysis/AnalyzerSet.h

@@ -96,7 +96,8 @@ public:
 	void DrainModifications();
 
 	// Iterator support
-	using iterator = zeek::DictIterator;
+	using iterator = zeek::DictIterator<file_analysis::Analyzer>;
+	;
 	using const_iterator = const iterator;
 	using reverse_iterator = std::reverse_iterator<iterator>;
 	using const_reverse_iterator = std::reverse_iterator<const_iterator>;

src/file_analysis/File.cc

@@ -377,7 +377,7 @@ void File::DeliverStream(const u_char* data, uint64_t len)
 
 	for ( const auto& entry : analyzers )
 		{
-		auto* a = entry.GetValue<file_analysis::Analyzer*>();
+		auto* a = entry.value;
 
 		DBG_LOG(DBG_FILE_ANALYSIS, "stream delivery to analyzer %s",
 		        file_mgr->GetComponentName(a->Tag()).c_str());
@@ -475,7 +475,7 @@ void File::DeliverChunk(const u_char* data, uint64_t len, uint64_t offset)
 
 	for ( const auto& entry : analyzers )
 		{
-		auto* a = entry.GetValue<file_analysis::Analyzer*>();
+		auto* a = entry.value;
 
 		DBG_LOG(DBG_FILE_ANALYSIS, "chunk delivery to analyzer %s",
 		        file_mgr->GetComponentName(a->Tag()).c_str());
@@ -539,7 +539,7 @@ void File::EndOfFile()
 
 	for ( const auto& entry : analyzers )
 		{
-		auto* a = entry.GetValue<file_analysis::Analyzer*>();
+		auto* a = entry.value;
 
 		if ( ! a->EndOfFile() )
 			analyzers.QueueRemove(a->Tag(), a->GetArgs());
@@ -574,7 +574,7 @@ void File::Gap(uint64_t offset, uint64_t len)
 
 	for ( const auto& entry : analyzers )
 		{
-		auto* a = entry.GetValue<file_analysis::Analyzer*>();
+		auto* a = entry.value;
 
 		if ( ! a->Undelivered(offset, len) )
 			analyzers.QueueRemove(a->Tag(), a->GetArgs());

src/input/Manager.cc

@@ -272,7 +272,7 @@ bool Manager::CreateStream(Stream* info, RecordVal* description)
 		for ( const auto& icte : *info_config_table )
 			{
 			auto k = icte.GetHashKey();
-			auto* v = icte.GetValue<TableEntryVal*>();
+			auto* v = icte.value;
 
 			auto index = info->config->RecreateIndex(*k);
 			string key = index->Idx(0)->AsString()->CheckString();
@@ -1402,7 +1402,7 @@ void Manager::EndCurrentSend(ReaderFrontend* reader)
 	for ( auto it = stream->lastDict->begin_robust(); it != stream->lastDict->end_robust(); ++it )
 		{
 		auto lastDictIdxKey = it->GetHashKey();
-		InputHash* ih = it->GetValue<InputHash*>();
+		InputHash* ih = it->value;
 
 		ValPtr val;
 		ValPtr predidx;

src/logging/Manager.cc

@@ -889,7 +889,7 @@ bool Manager::Write(EnumVal* id, RecordVal* columns_arg)
 			for ( const auto& fcte : *filter_config_table )
 				{
 				auto k = fcte.GetHashKey();
-				auto* v = fcte.GetValue<TableEntryVal*>();
+				auto* v = fcte.value;
 
 				auto index = filter->config->RecreateIndex(*k);
 				string key = index->Idx(0)->AsString()->CheckString();

src/packet_analysis/Analyzer.cc

@@ -3,7 +3,6 @@
 #include "zeek/packet_analysis/Analyzer.h"
 
 #include "zeek/DebugLogger.h"
-#include "zeek/Dict.h"
 #include "zeek/Event.h"
 #include "zeek/RunState.h"
 #include "zeek/session/Manager.h"

src/reporter.bif

@@ -186,7 +186,6 @@ function Reporter::set_weird_sampling_whitelist%(weird_sampling_whitelist: strin
 	for ( const auto& tble : *wl_table )
 		{
 		auto k = tble.GetHashKey();
-		auto* v = tble.GetValue<TableEntryVal*>();
 
 		auto index = wl_val->RecreateIndex(*k);
 		string key = index->Idx(0)->AsString()->CheckString();

src/script_opt/ZAM/IterInfo.h

@@ -65,7 +65,7 @@ public:
 	// For the current iteration, returns the corresponding value.
 	ZVal IterValue()
 		{
-		auto tev = (*tbl_iter)->GetValue<TableEntryVal*>();
+		auto tev = (*tbl_iter)->value;
 		return ZVal(tev->GetVal(), aux->value_var_type);
 		}
 
@@ -88,8 +88,8 @@ private:
 	// Associated auxiliary information.
 	ZInstAux* aux = nullptr;
 
-	std::optional<DictIterator> tbl_iter;
+	std::optional<DictIterator<TableEntryVal>> tbl_iter;
-	std::optional<DictIterator> tbl_end;
+	std::optional<DictIterator<TableEntryVal>> tbl_end;
 	};
 
 // Class for simple step-wise iteration across an integer range.

src/supervisor/Supervisor.cc

@@ -25,7 +25,6 @@ extern "C"
 	}
 
 #include "zeek/DebugLogger.h"
-#include "zeek/Dict.h"
 #include "zeek/Event.h"
 #include "zeek/EventHandler.h"
 #include "zeek/ID.h"
@@ -1271,7 +1270,7 @@ Supervisor::NodeConfig Supervisor::NodeConfig::FromRecord(const RecordVal* node)
 	for ( const auto& ee : *env_table )
 		{
 		auto k = ee.GetHashKey();
-		auto* v = ee.GetValue<TableEntryVal*>();
+		auto* v = ee.value;
 
 		auto key = env_table_val->RecreateIndex(*k);
 		auto name = key->Idx(0)->AsStringVal()->ToStdString();
@@ -1286,7 +1285,7 @@ Supervisor::NodeConfig Supervisor::NodeConfig::FromRecord(const RecordVal* node)
 	for ( const auto& cte : *cluster_table )
 		{
 		auto k = cte.GetHashKey();
-		auto* v = cte.GetValue<TableEntryVal*>();
+		auto* v = cte.value;
 
 		auto key = cluster_table_val->RecreateIndex(*k);
 		auto name = key->Idx(0)->AsStringVal()->ToStdString();

src/telemetry/telemetry.bif

@@ -55,7 +55,7 @@ std::vector<zeek::telemetry::LabelView> sv_tbl(zeek::TableVal* xs)
 		{
 		for ( auto& val : *xs->Get() )
 			{
-			auto val_ptr = val.GetValue<zeek::TableEntryVal*>()->GetVal();
+			auto val_ptr = val.value->GetVal();
 			result.emplace_back(std::string_view{val.GetKey(), val.key_size},
 			                    sv(val_ptr->AsStringVal()));
 			}

src/util.cc

@@ -46,7 +46,6 @@
 #include "zeek/3rdparty/ConvertUTF.h"
 #include "zeek/3rdparty/doctest.h"
 #include "zeek/Desc.h"
-#include "zeek/Dict.h"
 #include "zeek/Hash.h"
 #include "zeek/NetVar.h"
 #include "zeek/Obj.h"