Improve unnecessarily over-complicated regex #11813
Conversation
marcusburghardt
added
bugfix
|
Start a new ephemeral environment with changes proposed in this pull request: rhel8 (from CTF) Environment (using Fedora as testing environment) Fedora Testing Environment Oracle Linux 8 Environment |
|
This datastream diff is auto generated by the check Click here to see the full diffbash remediation for rule 'xccdf_org.ssgproject.content_rule_sudo_custom_logfile' differs.
--- xccdf_org.ssgproject.content_rule_sudo_custom_logfile
+++ xccdf_org.ssgproject.content_rule_sudo_custom_logfile
@@ -6,7 +6,7 @@
if /usr/sbin/visudo -qcf /etc/sudoers; then
cp /etc/sudoers /etc/sudoers.bak
- if ! grep -P '^[\s]*Defaults[\s]*\blogfile\s*=\s*("(?:\\"|\\\\|[^"\\\n])*"\B|[^"](?:(?:\\,|\\"|\\ |\\\\|[^", \\\n])*)\b)\b.*$' /etc/sudoers; then
+ if ! grep -P '^[\s]*Defaults[\s]*\blogfile\s*=\s*(?:"?([^",\s]+)"?)\b.*$' /etc/sudoers; then
# sudoers file doesn't define Option logfile
echo "Defaults logfile=${var_sudo_logfile}" >> /etc/sudoers
else |
|
🤖 A k8s content image for this PR is available at: Click here to see how to deploy itIf you alread have Compliance Operator deployed: Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and: |
|
Testing farms are failing now because we still need to fix some minor issues with #11796. |
There was a problem hiding this comment.
I have checkd the regex101.com and I have reviewed the regex. I have executed the test scenarios.
The PR looks good to me.
Please rebase this on the top of the latest upstream master branch which should bring in the sssd_enable_pam_service which should make the TF tests green.
By any reason the regex in this rule was unnecessarily super
overcomplicated. It was updated from:
'\s*=\s*("(?:\\"|\\\\|[^"\\\n])*"\B|[^"](?:(?:\\,|\\"|\\ |\\\\|[^", \\\n])*)\b)'
to:
'\s*=\s*(?:"?([^",\s]+)"?)'
with the same effect and very likely better performance.
marcusburghardt
force-pushed
the
quoted_logfile
branch
from
d6e1799
to
1c28e47
Compare
Done |
|
Code Climate has analyzed commit 1c28e47 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 59.2% (0.0% change). View more on Code Climate. |
|
It looks good. @jan-cerny can you approve and merge please? |
Description:
By any reason the regex in this rule was unnecessarily super over-complicated.
This PR updates it from:
'\s*=\s*("(?:\\"|\\\\|[^"\\\n])*"\B|[^"](?:(?:\\,|\\"|\\ |\\\\|[^", \\\n])*)\b)'to:
'\s*=\s*(?:"?([^",\s]+)"?)'with the same effect and very likely better performance.
Rationale:
Review Hints:
Automatus tests should be enough, but it would also be nice to play with regex101.com