Skip to content

fix dissector curly braces problem #921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ekneg54 wants to merge 5 commits into
base: main
Choose a base branch
from
+106 −17
Open

fix dissector curly braces problem #921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
35ce44a
add testcase for curly braces handling
ekneg54 Jan 9, 2026
8ae3ca3
add handling of curly braces before end of section
ekneg54 Jan 9, 2026
c82a25a
fix ng dissector tests
ekneg54 Jan 9, 2026
fb12d1e
update CHANGELOG
ekneg54 Jan 9, 2026
8c7dffc
remove redundant curly brace
ekneg54 Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* fix incorrect default-logger lookup by consistently resolving defaults from `DEFAULT_LOG_CONFIG["loggers"]`.
* fix a possible race condition in the `geoip_enricher`
* fix possible memory leaks in configuration refresh when processors set up scheduled jobs which were not cleaned up
* fix a bug in `dissector` not handling curly braces on the end of a dissect section properly

## 18.0.0
### Breaking
Expand Down
4 changes: 2 additions & 2 deletions logprep/processor/dissector/rule.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@

START = r"%\{"
END = r"\}"
VALID_TARGET_FIELD = r"[^\}\%\{\}\+\/\|]*"
VALID_TARGET_FIELD = r"[^\%\{\}\+\/\|]*"
APPEND_WITH_SEPERATOR = r"(\+\([^%]+\))"
APPEND_WITHOUT_SEPERATOR = r"(\+(?!\([^%]))"
INDIRECT_FIELD_NOTATION = r"([&\?]))"
Expand All @@ -113,7 +113,7 @@
ACTION = r"(?P<action>[+])?"
STRIP_CHAR = r"(-\((?P<strip>.)\))?"
SEPERATOR = r"(\((?P<separator>\\\)|[^)]+)\))?"
TARGET_FIELD = r"(?P<target_field>[^\/\|-]*)"
TARGET_FIELD = r"(?P<target_field>[^\/\|\}-]*)"
Copy link
Collaborator

@mhoff mhoff Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekneg54 Maybe I have not properly understood the implementation, but shouldn't TARGET_FIELD and VALID_TARGET_FIELD be semantically equivalent?

Maybe not exactly, but something like this?:

Suggested change
TARGET_FIELD = r"(?P<target_field>[^\/\|\}-]*)"
TARGET_FIELD = rf"(?P<target_field>[^\/\|\}}-]{VALID_TARGET_FIELD}*)"

Copy link
Collaborator Author

@ekneg54 ekneg54 Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from semantics side you should be right. but from code site this doesn't work. And to be honest I have no clue why this ist written this way. I only came here to fix this bug. I have no time to dig deep into the implementation. Sorry for that.

The only thing I remember is that I had to design the validation regex a little bit broader. It is used in rule validation via attrs. But I guess it is worth it to have a look into. But not in conjunction with this bugfix.

POSITION = r"(\/(?P<position>\d*))?"
DATATYPE = r"(\|(?P<datatype>int|float|bool))?"
SECTION_MATCH = rf"{START}{ACTION}{SEPERATOR}{TARGET_FIELD}{STRIP_CHAR}{POSITION}{DATATYPE}{END}(?P<delimiter>.*)"
Expand Down
58 changes: 51 additions & 7 deletions tests/unit/ng/processor/dissector/test_dissector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,27 +382,26 @@
"filter": "message",
"dissector": {"mapping": {"message": "%{field1} %{field2} %{field3} %{+field4}"}},
},
{"message": "This is \\a + message"},
{"message": "This is \\a + mess}age"},
{
"message": "This is \\a + message",
"message": "This is \\a + mess}age",
"field1": "This",
"field2": "is",
"field3": "\\a",
"field4": "+ message",
"field4": "+ mess}age",
},
),
(
"handles special chars in captured content and target field names",
{
"filter": "message",
"dissector": {"mapping": {"message": "%{~field1} %{fie ld2} %{$fie}ld3} %{+field4}"}},
"dissector": {"mapping": {"message": "%{~field1} %{fie ld2} %{+field4}"}},
},
{"message": "&This is\2 a mess}age /1"},
{"message": "&This is\2 mess}age /1"},
{
"message": "&This is\2 a mess}age /1",
"message": "&This is\2 mess}age /1",
"~field1": "&This",
"fie ld2": "is\2",
"$fie}ld3": "a",
"field4": "mess}age /1",
},
),
Expand Down Expand Up @@ -638,6 +637,51 @@
"sys_type": "system_monitor",
},
),
(
"handle curly braces in message simple case",
{
"filter": "message",
"dissector": {
"mapping": {
"message": "proxy{addr=%{destination.address}}",
},
},
},
{"message": "proxy{addr=10.99.172.10:4191}"},
{
"destination": {
"address": "10.99.172.10:4191",
},
"message": "proxy{addr=10.99.172.10:4191}",
},
),
(
"handle curly braces in message full case",
{
"filter": "message",
"dissector": {
"mapping": {
"message": "proxy{addr=%{destination.address}}:service{ns=linkerd-multicluster name=%{destination.domain} port=4191}:endpoint{addr=%{source.address}}: %{log.logger}: %{message}",
},
},
},
{
"message": "proxy{addr=10.99.172.10:4191}:service{ns=linkerd-multicluster name=probe-gateway-bbb port=4191}:endpoint{addr=192.8.177.98:4191}: linkerd_reconnect: Failed to connect error=connect timed out after 1s"
},
{
"destination": {
"address": "10.99.172.10:4191",
"domain": "probe-gateway-bbb",
},
"log": {
"logger": "linkerd_reconnect",
},
"message": "Failed to connect error=connect timed out after 1s",
"source": {
"address": "192.8.177.98:4191",
},
},
),
]
failure_test_cases = [ # testcase, rule, event, expected
(
Expand Down
60 changes: 52 additions & 8 deletions tests/unit/processor/dissector/test_dissector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# pylint: disable=missing-docstring

# pylint: disable=duplicate-code
import pytest

from logprep.processor.base.exceptions import ProcessingWarning
Expand Down Expand Up @@ -381,27 +381,26 @@
"filter": "message",
"dissector": {"mapping": {"message": "%{field1} %{field2} %{field3} %{+field4}"}},
},
{"message": "This is \\a + message"},
{"message": "This is \\a + mess}age"},
{
"message": "This is \\a + message",
"message": "This is \\a + mess}age",
"field1": "This",
"field2": "is",
"field3": "\\a",
"field4": "+ message",
"field4": "+ mess}age",
},
),
(
"handles special chars in captured content and target field names",
{
"filter": "message",
"dissector": {"mapping": {"message": "%{~field1} %{fie ld2} %{$fie}ld3} %{+field4}"}},
"dissector": {"mapping": {"message": "%{~field1} %{fie ld2} %{+field4}"}},
},
{"message": "&This is\2 a mess}age /1"},
{"message": "&This is\2 mess}age /1"},
{
"message": "&This is\2 a mess}age /1",
"message": "&This is\2 mess}age /1",
"~field1": "&This",
"fie ld2": "is\2",
"$fie}ld3": "a",
"field4": "mess}age /1",
},
),
Expand Down Expand Up @@ -637,6 +636,51 @@
"sys_type": "system_monitor",
},
),
(
"handle curly braces in message simple case",
{
"filter": "message",
"dissector": {
"mapping": {
"message": "proxy{addr=%{destination.address}}",
},
},
},
{"message": "proxy{addr=10.99.172.10:4191}"},
{
"destination": {
"address": "10.99.172.10:4191",
},
"message": "proxy{addr=10.99.172.10:4191}",
},
),
(
"handle curly braces in message full case",
{
"filter": "message",
"dissector": {
"mapping": {
"message": "proxy{addr=%{destination.address}}:service{ns=linkerd-multicluster name=%{destination.domain} port=4191}:endpoint{addr=%{source.address}}: %{log.logger}: %{message}",
},
},
},
{
"message": "proxy{addr=10.99.172.10:4191}:service{ns=linkerd-multicluster name=probe-gateway-bbb port=4191}:endpoint{addr=192.8.177.98:4191}: linkerd_reconnect: Failed to connect error=connect timed out after 1s"
},
{
"destination": {
"address": "10.99.172.10:4191",
"domain": "probe-gateway-bbb",
},
"log": {
"logger": "linkerd_reconnect",
},
"message": "Failed to connect error=connect timed out after 1s",
"source": {
"address": "192.8.177.98:4191",
},
},
),
]
failure_test_cases = [ # testcase, rule, event, expected
(
Expand Down