diff --git a/CHANGES b/CHANGES index 9fbfee0163..4ae5cda9df 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,31 @@ +7.2.0-dev.478 | 2025-04-04 18:36:37 +0200 + + * broker/main: Adapt enum values to agree with comm.bif (Arne Welzel, Corelight) + + Logic to detect this error already existed, but due to enum identifiers + not having a value set, it never triggered before. + + * btest/plugin: Add plugin testing enum identifiers (Arne Welzel, Corelight) + + This plugin has a generic name, but for now just tests the API around + enum types and enum vals at InitPostScript() time. + + * Var/add_type: Do not clone EnumType when declared for the first time (Arne Welzel, Corelight) + + EnumType receives the name into its constructor. Even for the first declaration + the name is not empty and instead the same as the identifier's name. Due to that, + add_type() previously took the else path and created a shallow clone of the + initial type instead of using it. This lead to buggy behavior where enum value + identifiers declared within an enum's first body have a different TypePtr + associated than the one that is found via `zeek::id::find_type()`. It also + means that enum identifiers added via redef later would have a different + TypePtr than those in the initial pointer. + + * type/id/zeekygen: Add EnumVal to enum identifiers (Arne Welzel, Corelight) + + Provide a direct way to go from a zeek:id::ID value to EnumVal without + needing to go through the type. + 7.2.0-dev.473 | 2025-04-03 13:10:43 +0200 * GH-4323: Traverse: Do not short-circuit traverse_all() if stmts is nullptr (Arne Welzel, Corelight) diff --git a/NEWS b/NEWS index 3d1dbf88da..ef76edcb44 100644 --- a/NEWS +++ b/NEWS @@ -120,6 +120,10 @@ Changed Functionality - The RDP analyzer now also parses connections that do not contain the cookie field, which were previously rejected. +- An enum's zeek::detail::ID instance now holds its ``EnumVal``. For example, + looking up the "Conn::LOG" identifier allows to directly query the ``EnumVal`` + using ``ID::GetVal()``. + Removed Functionality --------------------- diff --git a/VERSION b/VERSION index 13ff358a7a..e6b41d64de 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.2.0-dev.473 +7.2.0-dev.478 diff --git a/scripts/base/frameworks/broker/main.zeek b/scripts/base/frameworks/broker/main.zeek index 805f1f1e6c..1213555dc5 100644 --- a/scripts/base/frameworks/broker/main.zeek +++ b/scripts/base/frameworks/broker/main.zeek @@ -207,24 +207,26 @@ export { PEER_INVALID = 3, ## Remote peer not listening. PEER_UNAVAILABLE = 4, + ## Remote peer disconnected during the handshake. + PEER_DISCONNECT_DURING_HANDSHAKE = 5, ## A peering request timed out. - PEER_TIMEOUT = 5, + PEER_TIMEOUT = 6, ## Master with given name already exists. - MASTER_EXISTS = 6, + MASTER_EXISTS = 7, ## Master with given name does not exist. - NO_SUCH_MASTER = 7, + NO_SUCH_MASTER = 8, ## The given data store key does not exist. - NO_SUCH_KEY = 8, + NO_SUCH_KEY = 9, ## The store operation timed out. - REQUEST_TIMEOUT = 9, + REQUEST_TIMEOUT = 10, ## The operation expected a different type than provided. - TYPE_CLASH = 10, + TYPE_CLASH = 11, ## The data value cannot be used to carry out the desired operation. - INVALID_DATA = 11, + INVALID_DATA = 12, ## The storage backend failed to execute the operation. - BACKEND_FAILURE = 12, + BACKEND_FAILURE = 13, ## The storage backend failed to execute the operation. - STALE_DATA = 13, + STALE_DATA = 14, ## Catch-all for a CAF-level problem. CAF_ERROR = 100 }; diff --git a/src/ID.cc b/src/ID.cc index 0031d833bc..10719cdaca 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -481,6 +481,10 @@ void ID::DescribeReST(ODesc* d, bool roles_only) const { } if ( val && type && type->Tag() != TYPE_FUNC && type->InternalType() != TYPE_INTERNAL_VOID && + // Do not include a default value for enum const identifiers, + // as their value can't be changed. + ! IsEnumConst() && + // Values within Version module are likely to include a // constantly-changing version number and be a frequent // source of error/desynchronization, so don't include them. diff --git a/src/Type.cc b/src/Type.cc index a5d57d187c..451bb9934e 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -1567,7 +1567,7 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name, zeek if ( deprecation ) id->MakeDeprecated({NewRef{}, deprecation}); - detail::zeekygen_mgr->Identifier(std::move(id), from_redef); + detail::zeekygen_mgr->Identifier(id, from_redef); } else { // We allow double-definitions if matching exactly. This is so that @@ -1591,6 +1591,12 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name, zeek if ( vals.find(val) == vals.end() ) vals[val] = make_intrusive(IntrusivePtr{NewRef{}, this}, val); + if ( ! id->HasVal() ) + id->SetVal(vals[val]); + else if ( id->GetVal()->AsEnum() != val ) + reporter->InternalError("inconsistent enum integer value for '%s' (old %" PRId64 " new %" PRId64 ")", + fullname.c_str(), id->GetVal()->AsEnum(), val); + const auto& types = Type::Aliases(GetName()); for ( const auto& t : types ) diff --git a/src/Var.cc b/src/Var.cc index e17e2dc15a..44ced49665 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -369,9 +369,18 @@ void add_type(ID* id, TypePtr t, std::unique_ptr> attr) { TypePtr tnew; - if ( (t->Tag() == TYPE_RECORD || t->Tag() == TYPE_ENUM) && old_type_name.empty() ) + if ( (t->Tag() == TYPE_RECORD || t->Tag() == TYPE_ENUM) && + (old_type_name.empty() || old_type_name == new_type_name) ) { // An extensible type (record/enum) being declared for first time. + // + // Enum types are initialized with the same name as their identifier + // when declared for the first time, double check that here. + if ( t->Tag() == TYPE_ENUM && new_type_name != old_type_name ) + reporter->InternalError("enum type has unexpected names: '%s' and '%s'", old_type_name.c_str(), + new_type_name.c_str()); + tnew = std::move(t); + } else { // If the old type is an error or the old type doesn't exist, then return // an error instead of trying to clone it. diff --git a/testing/btest/Baseline/plugins.api-plugin/output b/testing/btest/Baseline/plugins.api-plugin/output new file mode 100644 index 0000000000..f21c6a7f90 --- /dev/null +++ b/testing/btest/Baseline/plugins.api-plugin/output @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +TestEnumIdentifiers successful diff --git a/testing/btest/plugins/api-plugin.zeek b/testing/btest/plugins/api-plugin.zeek new file mode 100644 index 0000000000..f318cab1d7 --- /dev/null +++ b/testing/btest/plugins/api-plugin.zeek @@ -0,0 +1,26 @@ +# @TEST-DOC: A plugin testing some parts Zeek's C++ API +# @TEST-EXEC: ${DIST}/auxil/zeek-aux/plugin-support/init-plugin -u . Demo API +# @TEST-EXEC: cp -r %DIR/api-plugin/* . +# +# @TEST-EXEC: ./configure --zeek-dist=${DIST} && make +# +# @TEST-EXEC: ZEEK_PLUGIN_PATH=`pwd` zeek %INPUT >output +# +# @TEST-EXEC: TEST_DIFF_CANONIFIER= btest-diff output + +module DemoAPI; + +export { + type Severity: enum { + CRITICAL = 1, + ERROR = 2, + WARNING = 3, + INFO = 4, + }; +} + +module User; + +redef enum DemoAPI::Severity += { + USER_DEBUG = 50, +}; diff --git a/testing/btest/plugins/api-plugin/.btest-ignore b/testing/btest/plugins/api-plugin/.btest-ignore new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testing/btest/plugins/api-plugin/src/Plugin.cc b/testing/btest/plugins/api-plugin/src/Plugin.cc new file mode 100644 index 0000000000..40ed229c66 --- /dev/null +++ b/testing/btest/plugins/api-plugin/src/Plugin.cc @@ -0,0 +1,88 @@ + +#include "Plugin.h" + +#include +#include +#include +#include +#include + +namespace btest::plugin::Demo_API { +Plugin plugin; +} + +using namespace btest::plugin::Demo_API; + +zeek::plugin::Configuration Plugin::Configure() { + zeek::plugin::Configuration config; + config.name = "Demo::API"; + config.description = "Use some of Zeek's API for testing"; + config.version.major = 1; + config.version.minor = 0; + config.version.patch = 0; + return config; +} + +namespace { + +// Test zeek::id::find for enums an their types. +void TestEnumIdentifiers() { + auto severity_type = zeek::id::find_type("DemoAPI::Severity"); + if ( ! severity_type ) + zeek::reporter->FatalError("DemoAPI::Severity not found"); + + // Expect 5 entries! + if ( severity_type->Names().size() != 5 ) + zeek::reporter->FatalError("Wrong number of severities %" PRId64, severity_type->Names().size()); + + // Ensure CRITICAL and USER_DEBUG identifiers have the same enum type as severity_type. + auto critical_id = zeek::id::find("DemoAPI::CRITICAL"); + auto critical_type = critical_id->GetType(); + auto user_debug_id = zeek::id::find("User::USER_DEBUG"); + auto user_debug_type = user_debug_id->GetType(); + + if ( critical_type != user_debug_type ) + zeek::reporter->FatalError("CRITICAL and USER_DEBUG have different types (%p and %p)", critical_type.get(), + user_debug_type.get()); + + // Ensure the critical_id and user_debug_type IDs have an EnumVal attached + // and that the value is the same as in script land. + auto critical_val = critical_id->GetVal(); + auto user_debug_val = user_debug_id->GetVal(); + + if ( ! critical_val || ! user_debug_val ) + zeek::reporter->FatalError("Missing values on enum value identifiers %p %p", critical_val.get(), + user_debug_val.get()); + + if ( critical_val->AsEnum() != 1 ) + zeek::reporter->FatalError("Wrong value for CRITICAL: %" PRId64, critical_val->AsEnum()); + + if ( user_debug_val->AsEnum() != 50 ) + zeek::reporter->FatalError("Wrong value for USER_DEBUG: %" PRId64, user_debug_val->AsEnum()); + + // Ensure all the types (identifiers and values) agree with severity_type. + if ( critical_type != severity_type ) + zeek::reporter->FatalError("CRITICAL identifier has the wrong enum type %p vs %p", critical_type.get(), + severity_type.get()); + + if ( user_debug_type != severity_type ) + zeek::reporter->FatalError("USER_DEBUG identifier has the wrong enum type %p vs %p", user_debug_type.get(), + severity_type.get()); + + if ( critical_val->GetType() != severity_type ) + zeek::reporter->FatalError("CRITICAL value has the wrong enum type %p vs %p", critical_val->GetType().get(), + severity_type.get()); + + if ( user_debug_val->GetType() != severity_type ) + zeek::reporter->FatalError("USER_DEBUG value has the wrong enum type %p vs %p", user_debug_val->GetType().get(), + severity_type.get()); + + std::cout << "TestEnumIdentifiers successful" << std::endl; +} + +} // namespace + +void Plugin::InitPostScript() { + // Other API tests if wanted. + TestEnumIdentifiers(); +} diff --git a/testing/btest/plugins/api-plugin/src/Plugin.h b/testing/btest/plugins/api-plugin/src/Plugin.h new file mode 100644 index 0000000000..2c2740e97a --- /dev/null +++ b/testing/btest/plugins/api-plugin/src/Plugin.h @@ -0,0 +1,18 @@ + +#pragma once + +#include + +namespace btest::plugin::Demo_API { + +class Plugin : public zeek::plugin::Plugin { +protected: + // Overridden from zeek::plugin::Plugin. + zeek::plugin::Configuration Configure() override; + + void InitPostScript() override; +}; + +extern Plugin plugin; + +} // namespace btest::plugin::Demo_API