Skip to content

Commit d873811

Browse files
Fix mutable default argument in merge_logs_to_list (#1497)
* Fix mutable default argument in merge_logs_to_list Replace log_list=[] with log_list=None and initialize inside the function body. The mutable default was shared across all calls, causing log entries from previous invocations to leak into subsequent results and growing without bound. Fixes #1464 * Add docstring and tests for merge_logs_to_list Address review feedback: add docstring to meet coverage threshold and add tests verifying correct behavior including no shared state between consecutive calls (the core bug this PR fixes). --------- Co-authored-by: Sam Stepanyan <sam.stepanyan@owasp.org>
1 parent 50c0be2 commit d873811

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

nettacker/core/utils/common.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,19 @@ def replace_dependent_response(log, response_dependent):
3131
return log
3232

3333

34-
def merge_logs_to_list(result, log_list=[]):
34+
def merge_logs_to_list(result, log_list=None):
35+
"""Recursively extract all 'log' values from a nested dict into a flat deduplicated list.
36+
37+
Args:
38+
result: A dict (possibly nested) containing 'log' keys to extract.
39+
log_list: Accumulator list for recursive calls. Defaults to a new empty list
40+
on each top-level call to avoid mutable default argument pitfalls.
41+
42+
Returns:
43+
A deduplicated list of extracted log values.
44+
"""
45+
if log_list is None:
46+
log_list = []
3547
if isinstance(result, dict):
3648
if "json_event" in list(result.keys()):
3749
if not isinstance(result["json_event"], dict):

tests/core/utils/test_common.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,43 @@ def test_select_maximum_cpu_core(cpu_count_mock):
9696
assert common_utils.select_maximum_cpu_core("invalid") == 1
9797

9898

99+
def test_merge_logs_to_list_simple():
100+
result = {"log": "error occurred"}
101+
assert common_utils.merge_logs_to_list(result) == ["error occurred"]
102+
103+
104+
def test_merge_logs_to_list_nested():
105+
result = {
106+
"log": "outer",
107+
"nested": {"log": "inner"},
108+
}
109+
logs = common_utils.merge_logs_to_list(result)
110+
assert sorted(logs) == ["inner", "outer"]
111+
112+
113+
def test_merge_logs_to_list_no_log_key():
114+
result = {"status": "ok", "data": {"value": 42}}
115+
assert common_utils.merge_logs_to_list(result) == []
116+
117+
118+
def test_merge_logs_to_list_deduplicates():
119+
result = {
120+
"log": "same",
121+
"nested": {"log": "same"},
122+
}
123+
assert common_utils.merge_logs_to_list(result) == ["same"]
124+
125+
126+
def test_merge_logs_to_list_no_shared_state_between_calls():
127+
"""Verify that consecutive calls without explicit log_list don't leak state."""
128+
result_a = {"log": "first"}
129+
result_b = {"log": "second"}
130+
logs_a = common_utils.merge_logs_to_list(result_a)
131+
logs_b = common_utils.merge_logs_to_list(result_b)
132+
assert logs_a == ["first"]
133+
assert logs_b == ["second"]
134+
135+
99136
def test_wait_for_threads_to_finish_all_dead():
100137
"""All threads already finished -- should return True immediately."""
101138
t = MagicMock(spec=threading.Thread)

0 commit comments

Comments
 (0)