Skip to content

Commit d0ed63a

Browse files
committed
feat(context): add more information to errors
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
1 parent 303f62b commit d0ed63a

4 files changed

Lines changed: 134 additions & 3 deletions

File tree

src/secrules_parsing/parser.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ def process_rules(files, verbose=False, debug=False):
4040
"line": e.line,
4141
"col": e.col,
4242
"message": e.message,
43+
"context": e.context,
4344
}
4445
models.append(model)
4546
return models
@@ -86,7 +87,7 @@ def get_correctness(files, output_type, models):
8687
)
8788
else:
8889
print(
89-
f"Syntax invalid: Syntax error in line {e['line']} col {e['col']}: {e['message']}"
90+
f"Syntax invalid: Syntax error in line {e['line']} col {e['col']}: {e['message']} - {e['context']}"
9091
)
9192
exitcode = 1
9293
else:
@@ -109,5 +110,6 @@ def process_from_str(str, verbose=False, debug=False):
109110
"line": e.line,
110111
"col": e.col,
111112
"message": e.message,
113+
"context": e.context,
112114
}
113115
return model

tests/conftest.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,14 @@ def assert_parse_success(parsed_result: Union[Any, Dict]) -> None:
6868
assert_parse_success(result)
6969
# Now safe to use result.rules
7070
"""
71-
assert not isinstance(parsed_result, dict), \
72-
f"Parse failed: {parsed_result.get('message', 'Unknown error')} at line {parsed_result.get('line', '?')}"
71+
if isinstance(parsed_result, dict):
72+
context = parsed_result.get('context', '')
73+
context_str = f" - Context: {context}" if context else ""
74+
raise AssertionError(
75+
f"Parse failed: {parsed_result.get('message', 'Unknown error')} "
76+
f"at line {parsed_result.get('line', '?')} "
77+
f"col {parsed_result.get('col', '?')}{context_str}"
78+
)
7379

7480

7581
def get_rules_by_type(parsed_result: Any, rule_type: str) -> List[Any]:

tests/test_api.py

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,3 +408,124 @@ def test_setenv_without_value() -> None:
408408
matches += 1
409409
assert matches == 1
410410

411+
412+
# Parse Error Context Tests
413+
414+
415+
def test_parse_error_includes_context_field() -> None:
416+
"""
417+
Test that parse errors include the 'context' field
418+
This verifies that error responses contain context information
419+
"""
420+
rule_text = """
421+
SecRule ARGS @rx "missing quotes
422+
"""
423+
result = parser.process_from_str(rule_text)
424+
425+
# Should return error dict, not a model
426+
assert isinstance(result, dict), "Expected parse error to return dict"
427+
428+
# Verify all error fields are present
429+
assert 'line' in result, "Error should include line number"
430+
assert 'col' in result, "Error should include column number"
431+
assert 'message' in result, "Error should include error message"
432+
assert 'context' in result, "Error should include context field"
433+
434+
# Verify context is not None or empty
435+
assert result['context'] is not None, "Context should not be None"
436+
assert len(str(result['context'])) > 0, "Context should not be empty"
437+
438+
439+
def test_parse_error_context_with_invalid_directive() -> None:
440+
"""
441+
Test that invalid directive syntax provides context
442+
"""
443+
rule_text = """
444+
InvalidDirective ARGS "@rx test"
445+
"""
446+
result = parser.process_from_str(rule_text)
447+
448+
assert isinstance(result, dict), "Expected parse error"
449+
assert 'context' in result, "Error should include context"
450+
assert result['line'] > 0, "Should have line number"
451+
assert result['col'] > 0, "Should have column number"
452+
453+
454+
def test_parse_error_context_with_missing_operator() -> None:
455+
"""
456+
Test that incomplete operator syntax provides context
457+
"""
458+
rule_text = """
459+
SecRule ARGS @ "id:1,deny"
460+
"""
461+
result = parser.process_from_str(rule_text)
462+
463+
assert isinstance(result, dict), "Expected parse error"
464+
assert 'context' in result, "Error should include context"
465+
assert 'message' in result, "Error should include message"
466+
467+
468+
def test_parse_error_context_with_malformed_actions() -> None:
469+
"""
470+
Test that malformed actions provide context
471+
"""
472+
rule_text = """
473+
SecRule ARGS "@rx test" "id:,phase:2"
474+
"""
475+
result = parser.process_from_str(rule_text)
476+
477+
assert isinstance(result, dict), "Expected parse error"
478+
assert 'context' in result, "Error should include context"
479+
assert result['line'] > 0, "Should have line number"
480+
481+
482+
def test_parse_error_context_with_unclosed_quotes() -> None:
483+
"""
484+
Test that unclosed quotes provide context
485+
"""
486+
rule_text = """
487+
SecRule ARGS "@rx test" "id:1,msg:'unclosed message
488+
"""
489+
result = parser.process_from_str(rule_text)
490+
491+
assert isinstance(result, dict), "Expected parse error"
492+
assert 'context' in result, "Error should include context"
493+
assert 'line' in result, "Error should include line"
494+
assert 'col' in result, "Error should include col"
495+
496+
497+
def test_successful_parse_does_not_return_dict() -> None:
498+
"""
499+
Test that successful parses return model, not dict
500+
This ensures we can distinguish between success and error
501+
"""
502+
rule_text = """
503+
SecRule ARGS "@rx test" "id:1,phase:2,deny"
504+
"""
505+
result = parser.process_from_str(rule_text)
506+
507+
# Successful parse should NOT return a dict
508+
assert not isinstance(result, dict), "Successful parse should return model, not dict"
509+
510+
# Should have rules attribute
511+
assert hasattr(result, 'rules'), "Model should have rules attribute"
512+
assert len(result.rules) > 0, "Should have at least one rule"
513+
514+
515+
def test_parse_error_context_multiline() -> None:
516+
"""
517+
Test that parse errors in multiline rules include context
518+
"""
519+
rule_text = """
520+
SecRule ARGS "@rx test" \\
521+
"id:1,\\
522+
phase:2,\\
523+
invalid_action_here,\\
524+
deny"
525+
"""
526+
result = parser.process_from_str(rule_text)
527+
528+
assert isinstance(result, dict), "Expected parse error"
529+
assert 'context' in result, "Error should include context"
530+
assert result['line'] > 0, "Should have line number"
531+

tests/test_conftest_examples.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ def test_example_parse_error_handling():
123123
if isinstance(result, dict):
124124
assert 'message' in result
125125
assert 'line' in result
126+
assert 'col' in result
127+
assert 'context' in result # NEW: context field added in recent update
126128
# Test passes - we expected a parse error
127129
return
128130

0 commit comments

Comments
 (0)