In raising this issue, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your issue:
How familiar are you with the codebase?:
1
[BUG] Expected Behaviour:
Domain Blocklist Count be correctly displayed.
[BUG] Actual Behaviour:
Domain Blocklist Count does not appear to be displayed correctly. In my case, it appears the number is doubled.
[BUG] Steps to reproduce:
Log file output [if available]
pihole -c -j
{"domains_being_blocked":245730,"dns_queries_today":238,"ads_blocked_today":28,"ads_percentage_today":11.764706}
When using the old method of counting lines in the blocklist files:
david@pihole:/etc/pihole$ test1=$(grep -c ^ /etc/pihole/list.preEventHorizon)
david@pihole:/etc/pihole$ echo $test1
122865
david@pihole:/etc/pihole$ test2=$(grep -c ^ /etc/pihole/blacklist.txt)
david@pihole:/etc/pihole$ echo $test2
0
Device specifics
Hardware Type: Old 32bit netbook
OS: Ubuntu 17.10
_This template was created based on the work of udemy-dl._
With FTLDNS, we changed the method we obtain the number of blocked domains. It seems, that this was not very wise.
Previously (FTL < v4.0), we counted the number of lines in list.preEventHorizon, just as you did. Now (FTLDNS, v4.0+), we get the number of blocked domains directly from the inbuilt resolver.
The advantage of the new method is that domains that could not be interpreted by the resolver (e.g. due to invalid UTF-8 characters in the domains) are also not counted. The obvious disadvantage is now that we count every domain twice if we block it once for IPv4 and once for IPv6...
I think we should revert to counting the number of domains in list.preEventHorizon.
Can FTL not determine whether or not the user has both IPv4 and v6? Similar to how the web interface does it by checking for an IPv6 address in /etc/pihole/setupvars.conf?
But is it guaranteed to have doubled entries in gravity.list if there is an IPv6 address in setupvars.conf (the user could just have added it in manually without running gravity). Also, when going the NXDOMAIN path, there won't be duplicated entries in the config. Counting the numbers in preEventHorizon might be better/simpler.
preventHorizon, remember, does not have blacklist/whitelist applied to it (as far as I recall), so if a user has a whitelist consisting of 1000 domains, then the count will be off by 1000
Hmm, the old logic was number(preEventHorizon) + number(black.list). I'd have to check if we forgot the whitelist or if it is included in preEventHorizon...
Would it be a fair assumption that if the user has ipv4 and 6 enabled, the number coming back could simply be divided by 2? Or is that a bit oversimplified? In the case where something didn't make it due to invalid characters, it wouldn't get added to ipv4 or 6, right?
Or is that a bit oversimplified?
I'm not so sure. Users might have added configuration though other files which might have been misinterpreted as blocked domains (although it might be valid assignments of e.g. artificial local domains). I think it is better like we did it before. I already pushed an update to the beta branch and you might want to try it in a few seconds (when the CI finished compiling the binaries and uploaded them). Edit: Done.
Before:

After:

@PromoFaux Not sure how to handle whitelisted domains, we did it always wrong (by ignoring them altogether). The problem is that if we just subtract them off, we might get an incorrect number as well, because users might have whitelisted some page that was anyways never blocked. This line wouldn't be counted for anything, then. I'm thinking about storing the correct number after each gravity run in some file, which FTL could simply read in. What do you think?
I think reverting to the non-preEventHorizon method and detecting if the IPv6 address is set in setupvars would be OK, you know.
Even if the user has added the IPv6, but not yet run gravity, it will correct itself when gravity autoruns the following Sunday. AFAIK, there is no other use for the IPv6 address being stored in the setupVars, so I can't imagine that it should cause any issues...
It is a bit more difficult than this as we would have to differentiate between the A/AAAA blocking entries mode and the NXDOMAIN mode...
would it be possible to merely use the numbers provided as part of gravity at the end ?
@technicalpyro this is exactly what I was aiming at. I think it would be the cleanest solution
Implemented like this:
/etc/pihole/numBlocked. If this file is avail- and readable, we use this number for displaying on the dashboard.can confirm running the updated does indeed populated numBlocked correctly
I'll close this issue as it should be resolved. Feel free to re-open if the issue persists.
Thanks everyone!