diff --git a/CHANGES b/CHANGES index 2edfffec07..9d7fce9f08 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,12 @@ +3.3.0-dev.221 | 2020-08-31 17:31:21 -0700 + + * GH-174: Treat ambiguous attribute duplication as an error (Tim Wojtulewicz, Corelight) + + For example, a &default=1 and a &default=2 attribute are not valid when + used together, but two duplicate &log attributes together are acceptable + although redundant. + 3.3.0-dev.216 | 2020-08-31 14:57:57 -0700 * Simplify a broker btest (Jon Siwek, Corelight) diff --git a/VERSION b/VERSION index a5a9901035..baaa7e85c0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.3.0-dev.216 +3.3.0-dev.221 diff --git a/src/Attr.cc b/src/Attr.cc index ada2d5a737..6639879220 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -193,8 +193,35 @@ Attributes::Attributes(std::vector a, AddAttr(std::move(attr)); } -void Attributes::AddAttr(AttrPtr attr) +void Attributes::AddAttr(AttrPtr attr, bool is_redef) { + auto acceptable_duplicate_attr = [](const AttrPtr& attr, const AttrPtr& existing) -> bool + { + AttrTag new_tag = attr->Tag(); + + if ( new_tag == ATTR_DEPRECATED ) + { + if ( ! attr->DeprecationMessage().empty() || + ( existing && ! existing->DeprecationMessage().empty() ) ) + return false; + + return true; + } + + return new_tag == ATTR_LOG || new_tag == ATTR_OPTIONAL || new_tag == ATTR_REDEF || + new_tag == ATTR_BROKER_STORE_ALLOW_COMPLEX || new_tag == ATTR_RAW_OUTPUT || + new_tag == ATTR_ERROR_HANDLER; + }; + + // A `redef` is allowed to overwrite an existing attribute instead of + // flagging it as ambiguous. + if ( ! is_redef ) + { + auto existing = Find(attr->Tag()); + if ( existing && ! acceptable_duplicate_attr(attr, existing) ) + reporter->Error("Duplicate %s attribute is ambiguous", attr_name(attr->Tag())); + } + // We overwrite old attributes by deleting them first. RemoveAttr(attr->Tag()); attrs_list.push_back(attr.get()); @@ -224,16 +251,16 @@ void Attributes::AddAttr(AttrPtr attr) } } -void Attributes::AddAttrs(const AttributesPtr& a) +void Attributes::AddAttrs(const AttributesPtr& a, bool is_redef) { for ( const auto& attr : a->GetAttrs() ) - AddAttr(attr); + AddAttr(attr, is_redef); } -void Attributes::AddAttrs(Attributes* a) +void Attributes::AddAttrs(Attributes* a, bool is_redef) { for ( const auto& attr : a->GetAttrs() ) - AddAttr(attr); + AddAttr(attr, is_redef); Unref(a); } diff --git a/src/Attr.h b/src/Attr.h index 7c4866ebbb..13d8f26c9b 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -115,12 +115,12 @@ public: ~Attributes() override = default; - void AddAttr(AttrPtr a); + void AddAttr(AttrPtr a, bool is_redef = false); - void AddAttrs(const AttributesPtr& a); + void AddAttrs(const AttributesPtr& a, bool is_redef = false); [[deprecated("Remove in v4.1. Pass IntrusivePtr instead.")]] - void AddAttrs(Attributes* a); // Unref's 'a' when done + void AddAttrs(Attributes* a, bool is_redef = false); // Unref's 'a' when done [[deprecated("Remove in v4.1. Use Find().")]] Attr* FindAttr(AttrTag t) const; diff --git a/src/ID.cc b/src/ID.cc index 5a54457b25..841241fcd3 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -316,10 +316,10 @@ std::string ID::GetDeprecationWarning() const return util::fmt("deprecated (%s): %s", Name(), result.c_str()); } -void ID::AddAttrs(AttributesPtr a) +void ID::AddAttrs(AttributesPtr a, bool is_redef) { if ( attrs ) - attrs->AddAttrs(a); + attrs->AddAttrs(a, is_redef); else attrs = std::move(a); diff --git a/src/ID.h b/src/ID.h index 98e8bf0677..d3f07e0066 100644 --- a/src/ID.h +++ b/src/ID.h @@ -121,7 +121,7 @@ public: bool IsRedefinable() const; void SetAttrs(AttributesPtr attr); - void AddAttrs(AttributesPtr attr); + void AddAttrs(AttributesPtr attr, bool is_redef = false); void RemoveAttr(AttrTag a); void UpdateValAttrs(); diff --git a/src/Var.cc b/src/Var.cc index 448a8e049a..18642beb25 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -207,7 +207,7 @@ static void make_var(const IDPtr& id, TypePtr t, InitClass c, ExprPtr init, id->SetType(t); if ( attr ) - id->AddAttrs(make_intrusive(std::move(*attr), t, false, id->IsGlobal())); + id->AddAttrs(make_intrusive(std::move(*attr), t, false, id->IsGlobal()), dt == VAR_REDEF); if ( init ) { diff --git a/testing/btest/Baseline/language.attr-default-global-set-error/out b/testing/btest/Baseline/language.attr-default-global-set-error/out index ddcc6a8345..29ab67ac35 100644 --- a/testing/btest/Baseline/language.attr-default-global-set-error/out +++ b/testing/btest/Baseline/language.attr-default-global-set-error/out @@ -1,5 +1,6 @@ -error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 4: &default is not valid for global variables except for tables (&default=0) -error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: &default is not valid for global variables except for tables (&default=10) -error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: &default is not valid for global variables except for tables (&default=9) -error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: &optional is not valid for global variables (&default=9, &optional) -error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 10: &default is not valid for global variables except for tables (&default=set()) +error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 4: &default is not valid for global variables except for tables (&default=0) +error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: &default is not valid for global variables except for tables (&default=10) +error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: Duplicate &default attribute is ambiguous +error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: &default is not valid for global variables except for tables (&default=9) +error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: &optional is not valid for global variables (&default=9, &optional) +error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 10: &default is not valid for global variables except for tables (&default=set()) diff --git a/testing/btest/Baseline/language.duplicate-attributes/out b/testing/btest/Baseline/language.duplicate-attributes/out new file mode 100644 index 0000000000..d5816fe4c4 --- /dev/null +++ b/testing/btest/Baseline/language.duplicate-attributes/out @@ -0,0 +1,3 @@ +error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.duplicate-attributes/duplicate-attributes.zeek, line 4: Duplicate &default attribute is ambiguous +error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.duplicate-attributes/duplicate-attributes.zeek, line 6: Duplicate &deprecated attribute is ambiguous +error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.duplicate-attributes/duplicate-attributes.zeek, line 7: Duplicate &deprecated attribute is ambiguous diff --git a/testing/btest/language/duplicate-attributes.zeek b/testing/btest/language/duplicate-attributes.zeek new file mode 100644 index 0000000000..f8114c5f77 --- /dev/null +++ b/testing/btest/language/duplicate-attributes.zeek @@ -0,0 +1,8 @@ +# @TEST-EXEC-FAIL: zeek -b %INPUT >out 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out + +global a: table[count] of count &default = 10 &default = 9; +global b: table[count] of count &deprecated &deprecated; +global c: table[count] of count &deprecated="a" &deprecated; +global d: table[count] of count &deprecated &deprecated="a"; +global e: table[count] of count &redef &redef;