Skip to content
This repository has been archived by the owner on Nov 6, 2023. It is now read-only.

Report and remove overlapping targets #12320

Merged
merged 2 commits into from
Sep 7, 2017

Conversation

RReverser
Copy link
Contributor

I've noticed many XML have targets that actually overlap and so unnecessarily duplicate each other.

I've extended target checks to test and report this automatically as reducing them might partially help with #12232.

@RReverser
Copy link
Contributor Author

RReverser commented Sep 1, 2017

Hmm, this is too broad and doesn't take into account that * in the end is treated differently than in the beginning. Let me fix it. Done.

I've noticed many XML have targets that actually overlap and so unnecessarily duplicate each other.

I've extended target checks to test and report this automatically as reducing them might partially help with EFForg#12232.
@RReverser
Copy link
Contributor Author

Added automated attempt to remove targets that were detected as already covered by others in the same ruleset.

As I said in #12322 (comment), I'm not sure if this will always give correct result because sometimes it's the wildcard target that can be avoided, but I think it might be better for the start and can be corrected manually after.

@RReverser
Copy link
Contributor Author

(feel free to merge the check separately from XML changes if that's easier / makes more sense)

@@ -1,7 +1,6 @@
<ruleset name="3min" default_off="Invalid Certificate">
<target host="3min.de"/>
<target host="*.3min.de"/>
<target host="www.3min.de"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases we cold replace the target with a test url.

@Hainish
Copy link
Member

Hainish commented Sep 7, 2017

Reviewing this now.

@Hainish Hainish merged commit cdc90c4 into EFForg:master Sep 7, 2017
@Hainish
Copy link
Member

Hainish commented Sep 7, 2017

Looks great, thanks again!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants