Prevent redundant entries from being added to Adlists.list #1954
Conversation
Updated webpage.sh to include conditional forwarding options.
Grep ${args[3]} and only add if grep -c -eq 0
Signed-off-by: Ryan Knapper <ryanknapper@gmail.com>
Reduced differences. Signed-off-by: Ryan Knapper <ryanknapper@gmail.com>
|
Does this look for exact matches? I don't think so. The likeliness of the following might be small, but assume I have a list http://blocking-list-provider.com/tracker Assume now, they also offer a list like http://blocking-list-provider.com/tracker-german I think with your proposed solution, the latter one cannot be added if the former is already present. I might be mistaken here (I'm not in the position to test this right now), but it is something I would like to be through about. |
|
I didn't think of this, but it still appears to work with in the situation you described. |
dschaper
added
PR: Code Review Required
advanced/Scripts/webpage.sh
Outdated
| @@ -346,7 +346,9 @@ CustomizeAdLists() { | |||
| elif [[ "${args[2]}" == "disable" ]]; then | |||
| sed -i "\\@${args[3]}@s/^http/#http/g" "${list}" | |||
| elif [[ "${args[2]}" == "add" ]]; then | |||
| echo "${args[3]}" >> ${list} | |||
| if [[ $(grep -c "${args[3]}" "${list}") -eq 0 ]] ; then | |||
There was a problem hiding this comment.
You should probably be doing an exact comparison here to avoid the situation I described above from happening. Just use the proper anchors here:
grep -c "^${args[3]}$" "${list}"Updated to require an exact match to reduce false-positives, as suggested by DL6ER. Signed-off-by: Ryan Knapper <ryanknapper@gmail.com>
ryanknapper
dismissed stale reviews from AzureMarker and jacobsalmela
via
5ecfc58
|
Suggested corrections made. |
|
Do we want to finish this PR? If so, @DL6ER, please re-review. |
By submitting this pull request, I confirm the following:
{please fill any appropriate checkboxes, e.g: [X]}{Please ensure that your pull request is for the 'development' branch!}git rebase)git commit --signoff)What does this PR aim to accomplish?:
Presently a user can add redundant entries to adlists.list, each of which Gravity will check for updates.
How does this PR accomplish the above?:
A simple grep for the proposed change. If true, do nothing.
What documentation changes (if any) are needed to support this PR?:
None needed.