refactor: split cli methods to easy testing #3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Felipe Zipitria <[email protected]>
theseion
requested changes
Jan 22, 2025
| if len(flist) == 0: | ||
| errmsg("List of files is empty!") | ||
| sys.exit(1) | ||
| errmsg(" Can't open file for indent check: %s" % (f)) |
There was a problem hiding this comment.
Suggested change
| errmsg(" Can't open file for indent check: %s" % (f)) | |
| errmsg("Can't open file for indent check: %s" % (f)) |
There was a problem hiding this comment.
Just my 2 cents: the indentations (spaces before messages here and at other reviews) helped the readability of the output, eg:
rules/REQUEST-931-APPLICATION-ATTACK-RFI.conf
Ignore case check ok.
Action order check ok.
Indentation check ok.
no 'ctl:auditLogParts' action found.
no duplicate id's
paranoia-level tags are correct.
PL anomaly_scores are correct.
All TX variables are set.
No new tags added.
No t:lowercase and (?i) flag used.
No rule without OWASP_CRS tag.
No rule without correct ver action.
No rule uses TX.N without capture action.
rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf
Ignore case check ok.
Action order check ok.
...
because the script processes files in order. If there are no indentations, I think it's a bit harder to read the output.
There was a problem hiding this comment.
All those are more debug than informational. Meaning, they don't provide additional information.
The only ones that do are the failures, IMHO.
So I think these should be moved to logger.debug() instead, and if people wants to see that output just run it with --debug or similar.
| for f in flist: | ||
| diff = difflib.unified_diff(from_lines, output) | ||
| if from_lines == output: | ||
| msg(" Indentation check ok.") |
There was a problem hiding this comment.
Suggested change
| msg(" Indentation check ok.") | |
| msg("Indentation check ok.") |
| if from_lines == output: | ||
| msg(" Indentation check ok.") | ||
| else: | ||
| errmsg(" Indentation check found error(s)") |
There was a problem hiding this comment.
Suggested change
| errmsg(" Indentation check found error(s)") | |
| errmsg("Indentation check found error(s)") |
|
|
||
| ### check case usings | ||
| c.check_ignore_case() | ||
| if len(c.caseerror) == 0: | ||
| msg(" Ignore case check ok.") | ||
| logger.info(" Ignore case check ok.") |
There was a problem hiding this comment.
Suggested change
| logger.info(" Ignore case check ok.") | |
| logger.info("Ignore case check ok.") |
| if has_unused == False: | ||
| msg(" No unused TX variable") | ||
| if not has_unused: | ||
| logger.info(" No unused TX variable") |
There was a problem hiding this comment.
Suggested change
| logger.info(" No unused TX variable") | |
| logger.info("No unused TX variable") |
Co-authored-by: Max Leske <[email protected]>
Signed-off-by: Felipe Zipitria <[email protected]>
Signed-off-by: Felipe Zipitria <[email protected]>
Signed-off-by: Felipe Zipitria <[email protected]>
Signed-off-by: Felipe Zipitria <[email protected]>
Signed-off-by: Felipe Zipitria <[email protected]>
Signed-off-by: Felipe Zipitria <[email protected]>
Signed-off-by: Felipe Zipitria <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
what
why