Skip to content

Commit 6af8b9d

Browse files
fzipiclaude
andcommitted
fix: address SonarCloud findings in modsec_test.py
- POOL_DEBUG_RE: rewrite the two unbounded [^\n]+ groups around a literal as a lookahead + single unbounded group, removing the superlinear backtracking risk on inputs without a match (S8786). Verified byte-identical output across the original's edge cases. - Extract _check_match_spec(), shared by the match_response/match_log/ match_file loops in run_regression_test - cuts its cognitive complexity from 41 to under the limit and removes the duplicated "matched (expected no match)"/"failed to match" literals (S3776, S1192). - TestResult.log_matches: Dict[...] = None -> Optional[Dict[...]] (S5890). - _make_request: dict comprehension over `request["headers"]` pairs -> dict() (S7500). No behavior change: full regression suite (241 passed, 5 skipped) matches pre-refactor results exactly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 185ac0e commit 6af8b9d

1 file changed

Lines changed: 43 additions & 39 deletions

File tree

tests/modsec_test.py

Lines changed: 43 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class TestResult:
1616
success: bool
1717
message: str = ""
1818
response: Optional[requests.Response] = None
19-
log_matches: Dict[str, List[str]] = None
19+
log_matches: Optional[Dict[str, List[str]]] = None
2020
skipped: bool = False
2121

2222
def __post_init__(self):
@@ -50,7 +50,26 @@ def _force_multiline(pattern: Pattern[bytes]) -> Pattern[bytes]:
5050
return re.compile(pattern.pattern, pattern.flags | re.MULTILINE)
5151

5252

53-
POOL_DEBUG_RE = re.compile(rb"POOL DEBUG:[^\n]+PALLOC[^\n]+\n")
53+
def _check_match_spec(spec, get_match, describe) -> Optional[str]:
54+
"""Shared logic for match_response/match_log/match_file: walk a
55+
{key: pattern_or_[pattern,timeout]} spec, respecting a leading "-" on the
56+
key for negation, and return a failure message for the first mismatch (or
57+
None if everything matched as expected).
58+
59+
get_match(name, pattern, timeout) -> matched bytes or None.
60+
describe(name) -> message prefix, e.g. "response status" or "file <path>".
61+
"""
62+
for key, value in (spec or {}).items():
63+
negate, name = _split_negation(key)
64+
pattern, timeout = _split_match_value(value)
65+
match = get_match(name, pattern, timeout)
66+
if (negate and match is not None) or (not negate and match is None):
67+
verb = "matched (expected no match)" if negate else "failed to match"
68+
return f"{describe(name)} {verb}: {pattern.pattern!r}"
69+
return None
70+
71+
72+
POOL_DEBUG_RE = re.compile(rb"POOL DEBUG:(?=[^\n]+PALLOC)[^\n]+\n")
5473
BUFSIZ = 32768
5574

5675

@@ -312,49 +331,34 @@ def run_regression_test(self, entry: Dict[str, Any]) -> TestResult:
312331
if response is None:
313332
return TestResult(success=False, message="Failed to make HTTP request")
314333

315-
for key, value in (entry.get("match_response") or {}).items():
316-
negate, mtype = _split_negation(key)
317-
pattern, _ = _split_match_value(value)
318-
match = self.response_matcher.match(mtype, response, pattern) if response is not None else None
319-
fail = (negate and match is not None) or (not negate and match is None)
320-
if fail:
321-
verb = "matched (expected no match)" if negate else "failed to match"
322-
return TestResult(
323-
success=False,
324-
message=f"response {mtype} {verb}: {pattern.pattern!r}",
325-
response=response,
326-
)
334+
def get_response_match(mtype, pattern, _timeout):
335+
return self.response_matcher.match(mtype, response, pattern) if response is not None else None
327336

328-
for key, value in (entry.get("match_log") or {}).items():
329-
negate, log_name = _split_negation(key)
330-
pattern, timeout = _split_match_value(value)
331-
match = self.log_matcher.wait_for_pattern(log_name, _force_multiline(pattern), timeout=timeout)
332-
fail = (negate and match is not None) or (not negate and match is None)
333-
if fail:
334-
verb = "matched (expected no match)" if negate else "failed to match"
335-
return TestResult(
336-
success=False,
337-
message=f"{log_name} log {verb}: {pattern.pattern!r}",
338-
response=response,
339-
)
337+
failure = _check_match_spec(
338+
entry.get("match_response"), get_response_match, lambda mtype: f"response {mtype}"
339+
)
340+
if failure:
341+
return TestResult(success=False, message=failure, response=response)
342+
343+
def get_log_match(log_name, pattern, timeout):
344+
return self.log_matcher.wait_for_pattern(log_name, _force_multiline(pattern), timeout=timeout)
345+
346+
failure = _check_match_spec(entry.get("match_log"), get_log_match, lambda log_name: f"{log_name} log")
347+
if failure:
348+
return TestResult(success=False, message=failure, response=response)
340349

341350
match_file = entry.get("match_file") or {}
342351
if match_file:
343352
time.sleep(1) # matches runfile()'s own "make sure the file exists" delay
344-
for key, value in match_file.items():
345-
negate, file_name = _split_negation(key)
346-
pattern, timeout = _split_match_value(value)
347-
match = self.log_matcher.wait_for_pattern(
353+
354+
def get_file_match(file_name, pattern, timeout):
355+
return self.log_matcher.wait_for_pattern(
348356
file_name, _force_multiline(pattern), timeout=timeout, from_start=True
349357
)
350-
fail = (negate and match is not None) or (not negate and match is None)
351-
if fail:
352-
verb = "matched (expected no match)" if negate else "failed to match"
353-
return TestResult(
354-
success=False,
355-
message=f"file {file_name} {verb}: {pattern.pattern!r}",
356-
response=response,
357-
)
358+
359+
failure = _check_match_spec(match_file, get_file_match, lambda file_name: f"file {file_name}")
360+
if failure:
361+
return TestResult(success=False, message=failure, response=response)
358362

359363
return TestResult(success=True, message=f"Test '{comment}' passed", response=response)
360364

@@ -400,7 +404,7 @@ def _make_request(self, request: Dict[str, Any]) -> Optional[requests.Response]:
400404
(do_raw_request() in Perl - deliberately malformed/chunked requests
401405
that must go over a raw socket instead of a conforming HTTP client)."""
402406
if request["__type__"] == "http_request":
403-
headers = {name: value for name, value in request["headers"]}
407+
headers = dict(request["headers"])
404408
try:
405409
return self.http_client.request(
406410
request["method"], request["uri"], headers=headers, data=request["content"],

0 commit comments

Comments
 (0)