Skip to content

Commit

Permalink
Revert not exists changes from #480, but keep refactorings in filter_…
Browse files Browse the repository at this point in the history
…expression (#515)

* Revert not exists changes from #480, but keep refactorings in filter_expression

* Re-add refactoring of rule_parser

* Update changelog
  • Loading branch information
ppcad authored Feb 1, 2024
1 parent 54623b2 commit 9e19caa
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 59 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* a do nothing option do dummy output to ensure dummy does not fill memory
* make the s3 connector raise `FatalOutputError` instead of warnings
* make the s3 connector blocking by removing threading
* revert the change from v9.0.0 to always check the existence of a field for negated key-value based lucene filter expressions

### Bugfix

Expand Down
34 changes: 4 additions & 30 deletions logprep/framework/rule_tree/rule_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@
behavior, allowing a simpler construction of the rule tree.
"""
from typing import TYPE_CHECKING, Optional
from typing import TYPE_CHECKING

from logprep.filter.expression.filter_expression import (
Always,
Exists,
Not,
KeyValueBasedFilterExpression,
FilterExpression,
)

from logprep.framework.rule_tree.demorgan_resolver import DeMorganResolver
Expand Down Expand Up @@ -118,8 +116,6 @@ def _add_exists_filter(parsed_rules: list):
checks if the given field even exists. Like this unnecessary comparisons can be prevented
when the tree would check each of the values the field can have even when the field does
not exist in the current event.
It also adds an exists filter to negated key value based expressions,
so that matching a value always requires the existence of the field.
Parameters
----------
Expand All @@ -131,33 +127,11 @@ def _add_exists_filter(parsed_rules: list):
temp_parsed_rule = parsed_rule.copy()
added_exists_filter_count = 0
for segment_idx, segment in enumerate(temp_parsed_rule):
if isinstance(segment, (Exists, Always)):
continue
exists_filter = RuleParser._get_exists_filter(segment)
if exists_filter is None:
if isinstance(segment, (Exists, Not, Always)):
continue

exists_filter = Exists(segment.key)
if exists_filter in parsed_rule:
continue
parsed_rule.insert(segment_idx + added_exists_filter_count, exists_filter)
added_exists_filter_count += 1

@staticmethod
def _get_exists_filter(segment: FilterExpression) -> Optional[Exists]:
"""Get Exists filter expression based in segment.
Parameters
----------
segment: FilterExpression
Filter expressions segment.
Returns
-------
Optional[Exists]
Returns an Exists FilterExpression from a KeyValueBased FilterExpression or None.
"""
if isinstance(segment, Not):
segment = segment.children[0]
if isinstance(segment, KeyValueBasedFilterExpression):
return Exists(segment.key)
return None
32 changes: 3 additions & 29 deletions tests/unit/framework/rule_tree/test_rule_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,10 @@
# pylint: disable=too-many-statements
import pytest

from logprep.filter.expression.filter_expression import (
StringFilterExpression,
Not,
Exists,
RegExFilterExpression,
)
from logprep.filter.expression.filter_expression import StringFilterExpression, Not, Exists
from logprep.framework.rule_tree.rule_parser import RuleParser
from logprep.processor.pre_detector.rule import PreDetectorRule


pytest.importorskip("logprep.processor.pre_detector")

string_filter_expression_1 = StringFilterExpression(["key1"], "value1")
Expand Down Expand Up @@ -169,7 +163,6 @@ class TestRuleParser:
[
Exists(["EventID"]),
StringFilterExpression(["EventID"], "17"),
Exists(["Image"]),
Not(StringFilterExpression(["Image"], "*\\powershell.exe")),
Exists(["PipeName"]),
StringFilterExpression(["PipeName"], "\\PSHost*"),
Expand Down Expand Up @@ -296,7 +289,6 @@ class TestRuleParser:
[
Exists(["bar"]),
StringFilterExpression(["bar"], "foo"),
Exists(["foo"]),
Not(StringFilterExpression(["foo"], "bar")),
]
],
Expand Down Expand Up @@ -340,12 +332,10 @@ class TestRuleParser:
{},
{},
[
[Exists(["foo"]), Not(StringFilterExpression(["foo"], "bar"))],
[Not(StringFilterExpression(["foo"], "bar"))],
[
Exists(["msg"]),
Not(StringFilterExpression(["msg"], "123")),
Not(StringFilterExpression(["msg"], "456")),
Exists(["test"]),
Not(StringFilterExpression(["test"], "ok")),
],
],
Expand Down Expand Up @@ -420,7 +410,6 @@ class TestRuleParser:
Not(Not(Exists(["AImphash"]))),
Exists(["EventID"]),
StringFilterExpression(["EventID"], "15"),
Exists(["Imphash"]),
Not(StringFilterExpression(["Imphash"], "000")),
]
],
Expand Down Expand Up @@ -650,22 +639,7 @@ def test_parse_rule_param(self, rule, priority_dict, tag_map, expected_expressio
[Exists(["key3"]), string_filter_expression_3],
],
),
(
[[Not(string_filter_expression_1)]],
[[Exists(["key1"]), Not(string_filter_expression_1)]],
),
(
[[Not(Exists(["foo"]))]],
[[Not(Exists(["foo"]))]],
),
(
[[RegExFilterExpression(["foo"], "bar")]],
[[Exists(["foo"]), RegExFilterExpression(["foo"], "bar")]],
),
(
[[Not(RegExFilterExpression(["foo"], "bar"))]],
[[Exists(["foo"]), Not(RegExFilterExpression(["foo"], "bar"))]],
),
([[Not(string_filter_expression_1)]], [[Not(string_filter_expression_1)]]),
(
[[string_filter_expression_1, Exists(["key1"])], [string_filter_expression_1]],
[
Expand Down

0 comments on commit 9e19caa

Please sign in to comment.