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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Markdown: Sanitizier Configuration #9075
Markdown: Sanitizier Configuration #9075
Changes from all commits
9025a69
5353d15
e582133
45963fc
5ecdfa5
a641266
314330b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't
haveRegexp
be ignored as your above code?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean by "as my above code"? The first check is for a completely empty section; there's nothing to do so exit with only a warning.
Per above
config-cheat-sheet
description forREGEXP
: "Must be present but may be empty for unconditional whitelisting of this attribute."So if it is present but empty,
haveRegexp = ini.Section.HasKey("REGEXP")
will returntrue
, butpattern = ini.Section.Key("REGEXP").ValueWithShadows()[i] == ""
for somei
.This check is thus for if
ELEMENT=
andHAVEATTR=
have been specified, butREGEXP
hasn't been. (Well -- strictly it checks for if any haven't been specified having earlier checked that all haven't been specified).This is because we aren't parsing the
ini
ourselves. We only have an external library that parses it for us, and gives us parsed results. Particularly, parsed results without any ordering between keys and without knowledge of where a missing key might be.If we allowed:
The user might later add:
We wouldn't be able to tell which element/attr pair the regex belongs to, because the ini module doesn't expose neighbor key information. That'd surprise the user (a working config + another working config should equal a working config, since we say that we support adding configs to get more rules).
So I maintain this is desired behavior unless there's something I'm missing?