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

Clean up and optimise Gravity #1631

Merged
merged 24 commits into from Sep 21, 2017

Conversation

Projects
None yet
6 participants
@WaLLy3K
Member

WaLLy3K commented Jul 24, 2017

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

9


Some pictures of Gravity running can be found here.

Changes for gravity.sh

  • Shellcheck validation
  • Made variable names, function names, comments and output clearer to understand
  • Quoted and braced variables and conditionals
  • Fix adlists.list handling logic, and remove any CR line endings
  • Make CP/MV/RM provide user-friendly output upon failure
  • Change adlists.list retrieval logic
  • Moved and fixed adlists.list domain parsing logic
  • Create gravity_ParseFileAsDomains() function to handle parsing of source files
  • If no changes to a list is detected, print no output
  • Ensure each source blocklist has a final newline
  • Format number output as currency
  • Make array of adlists domain sources unique to prevent redundant whitelisting
  • Merged bash IPv4/IPv6 hosts formatting IF statement into an awk one-liner
  • Trap Ctrl-C cancellations and run gravity_Cleanup()
  • Use new gravity_Cleanup() function on errors and script completion
  • Ensure that dnsmasq uses force-reload when gravity is invoked
  • Add --wildcard option to ensure dnsmasq is restarted upon b/wlisting of a wildcard

Changes for list.sh

  • Add "--blacklist-only" to only run essential gravity functions
  • Pass "--wildcard" option to gravity.sh to ensure dnsmasq is restarted

Changes for pihole

  • Made use of $PI_HOLE_SCRIPT_DIR
  • Fix #1610 & #1624
  • Add "$2" to restartDNS options

Changes for webpage.sh

  • Make RestartDNS() use pihole restartdns

WaLLy3K added some commits Jul 24, 2017

Clean up and optimise Gravity
* Shellcheck validation
* Made variable names, function names, comments and output clearer to understand
* Quoted and braced variables and conditionals
* Fix adlists.list handling logic, and remove any CR line endings
* Make CP/MV/RM provide user-friendly output upon failure
* Change adlists.list retrieval logic
* Moved and fixed adlists.list domain parsing logic
* Create gravity_ParseFileAsDomains() function to handle parsing of source files
* If no changes to a list is detected, print no output
* Ensure each source blocklist has a final newline
* Format number output as currency
* Make array of adlists domain sources unique to prevent redundant whitelisting
* Merged bash IPv4/IPv6 hosts formatting IF statement into an awk one-liner
* Trap Ctrl-C cancellations and run gravity_Cleanup()
* Use new gravity_Cleanup() function on errors and script completion
* Ensure that dnsmasq uses force-reload when gravity is invoked
* Add --wildcard option to ensure dnsmasq is restarted upon b/wlisting of a wildcard

Signed-off-by: WaLLy3K <wally3k@pi-hole.net>
Add dnsmasq "force-reload" option
* Made use of $PI_HOLE_SCRIPT_DIR
* Fix #1610 & #1624
* Add "$2" to restartDNS options

Signed-off-by: WaLLy3K <wally3k@pi-hole.net>
Speed up refresh time
* Add "--blacklist-only" to only run essential gravity functions
* Pass "--wildcard" option to `gravity.sh` to ensure dnsmasq is restarted

Signed-off-by: WaLLy3K <wally3k@pi-hole.net>
Remove duplicate code
* Make RestartDNS() use `pihole restartdns`

Signed-off-by: WaLLy3K <wally3k@pi-hole.net>
@WaLLy3K

This comment has been minimized.

Member

WaLLy3K commented Jul 24, 2017

The gravity.sh code is essentially complete, bar a couple of last-minute fixes that I'll need to push. In the meantime, test away, and try your best to break the parsing logic!

WaLLy3K added some commits Jul 27, 2017

Pass correct options to gravity.sh
* Optimise $validDomain function by using bashism and `grep`
* Add black/white/wildcard variables to pass to Reload()
* Revert reload variable behaviour
* Ensure Reload() function passes correct options to gravity.sh

Signed-off-by: WaLLy3K <wally3k@pi-hole.net>
WIP cleanup
* Changed supernova/eventHorizon variables to match their purpose
* Add gravity_DNSLookup() function
* Ensure all comments are clear and relevant
* Use && instead of || in gravity_Collapse()
* Renamed existing functions, and placed them in order of script execution
* Use \t instead of literal tab in gravity_ParseFileIntoDomains()
* Replace instances of "truncate" with : > (e.g: gravity_Schwarzschild())
* Ensure correct variables are local'd
* Use phrase "Cleaning up stray matter" when gravity_Cleanup() is called
* Add black/white/wildcard switches for list.sh
* Ensure necessary functions are called when modifying black/white/wildcards

Signed-off-by: WaLLy3K <wally3k@pi-hole.net>

@pi-hole pi-hole deleted a comment from codacy-bot Jul 27, 2017

@PromoFaux

This comment has been minimized.

Member

PromoFaux commented Jul 29, 2017

LGTM so far.

@WaLLy3K What else do you have planned for this PR?

@WaLLy3K WaLLy3K changed the title from [WIP] Clean up and optimise Gravity to Clean up and optimise Gravity Aug 1, 2017

@WaLLy3K WaLLy3K requested a review from jacobsalmela Aug 1, 2017

@WaLLy3K

This comment has been minimized.

Member

WaLLy3K commented Aug 1, 2017

Nothing else planned for now! 😄

@Mcat12

This comment has been minimized.

Member

Mcat12 commented Aug 1, 2017

  • Add "--blacklist-only" to only run essential gravity functions
  • Pass "--wildcard" option to gravity.sh to ensure dnsmasq is restarted

Maybe we want to change the names to better fit their functions?

@WaLLy3K

This comment has been minimized.

Member

WaLLy3K commented Aug 1, 2017

I modified them slightly here, but what did you have in mind?

@Mcat12

This comment has been minimized.

Member

Mcat12 commented Aug 1, 2017

Just that from reading the PR post the names weren't representative of what they are supposed to do.

@WaLLy3K WaLLy3K removed the request for review from jacobsalmela Aug 27, 2017

Update resolver test & added more comments
* Add/update code comments
* Change resolver check to test for pi.hole
* Make resolver check timeout after 10 seconds
* Use > instead of &> where appropriate
* Make resolver check sleep for 30 seconds (effectively waiting up to 50s for dnsmasq to be resolvable)
* Provide confirmation upon success of resolver check availability
* Quotes and Braced remaining variables as appropriate
* Removed duplicate local
@dschaper

This comment has been minimized.

Member

dschaper commented Aug 31, 2017

Tag this with the Ready for review when it's good to check for merge.

@Mcat12

I found a few issues, including one preventing new installs from working.

@@ -138,7 +140,7 @@ AddDomain() {
if [[ "${verbose}" == true ]]; then
echo -e " ${INFO} Adding $1 to wildcard blacklist..."
fi
reload=true
reload="restart"

This comment has been minimized.

@Mcat12

Mcat12 Sep 2, 2017

Member

When is this value used, and why set it to "restart" instead of true?

This comment has been minimized.

@WaLLy3K

WaLLy3K Sep 3, 2017

Member

Changing it from true to restart gives the variable a second purpose - we pass it through to restartDNS so that a wildcard update restarts dnsmasq instead of defaulting to "force-reload" (which in actual fact is just a SIGHUP).

pihole Outdated
fi
# Print output to Terminal, not Web Admin
str="${svcOption^}ing DNS service"

This comment has been minimized.

@Mcat12

Mcat12 Sep 2, 2017

Member

What does the ^ do in this case? Also, why only print to the terminal and not the web?

This comment has been minimized.

@WaLLy3K

WaLLy3K Sep 3, 2017

Member

^ is a bashism that'll capitalise the first character of $svcOption, which will either be "restart" (-> "Restarting DNS service") or "force-reload" (-> "Force-reloading DNS service").

dnsmasq is restarted for various reasons throughout Web Admin, and it's unnecessary to print that the DNS service was restarted - which keeps the output in line with current master behaviour.

gravity.sh Outdated
echo -e "${OVER} ${TICK} ${str}"
[[ -n "${error}" ]] && echo ""

This comment has been minimized.

@Mcat12

Mcat12 Sep 2, 2017

Member

Perhaps a comment would help explain this line.

"${PIHOLE_COMMAND}" restartdns
Options:
-f, --force Force the download of all specified blocklists
-h, --help Show this help dialog"

This comment has been minimized.

@Mcat12

Mcat12 Sep 2, 2017

Member

Should some of the below options be added to this list?

This comment has been minimized.

@WaLLy3K

WaLLy3K Sep 3, 2017

Member

I don't follow?

This comment has been minimized.

@Mcat12

Mcat12 Sep 3, 2017

Member

There's several undocumented flags below which users might find helpful such as --skip-download

This comment has been minimized.

@WaLLy3K

WaLLy3K Sep 3, 2017

Member

Ah! I've definitely considered adding those flags, but opted not to because I couldn't think of any use case where they'd be needed.

gravity.sh Outdated
gravity_Trap
# Ensure dnsmasq is restarted when modifying wildcards
[[ "${listType}" == "wildcard" ]] && dnsRestart="restart"

This comment has been minimized.

@Mcat12

Mcat12 Sep 2, 2017

Member

Could this be added to the switch statement case?

This comment has been minimized.

@WaLLy3K

WaLLy3K Sep 3, 2017

Member

I think the only reason I separated it out was to comment on its purpose.

gravity.sh Outdated
else
sources+=(${line})
# Determine if pi.hole can be resolved
if ! timeout 10 nslookup pi.hole > /dev/null; then

This comment has been minimized.

@Mcat12

Mcat12 Sep 2, 2017

Member

On a fresh install, this will always fail since pi.hole will return NXDOMAIN

pihole Outdated
@@ -15,6 +15,9 @@ readonly colfile="${PI_HOLE_SCRIPT_DIR}/COL_TABLE"
source ${colfile}
colfile="${PI_HOLE_SCRIPT_DIR}/COL_TABLE"

This comment has been minimized.

@Mcat12

Mcat12 Sep 2, 2017

Member

This is a readonly variable, and has already been set.

  [i] User-abort detected
  [✓] Cleaning up stray matter
/usr/local/bin/pihole: line 18: colfile: readonly variable
  [✓] DNS service is running
  [✓] Pi-hole blocking is Enabled
@PromoFaux

This comment has been minimized.

Member

PromoFaux commented Sep 5, 2017

Note for future reference (going for dinner, I will look in a bit if nobody beats me to it!)

On fresh install using this branch:
image

Which makes sense, as I'm pretty sure dnsmasq has not been brought back up at this stage in the install script

Never mind. @Mcat12 already pointed out the issue here

Issues mentioned in review are resolved

@PromoFaux PromoFaux added this to the v3.2 milestone Sep 14, 2017

@WaLLy3K WaLLy3K added the WIP label Sep 15, 2017

Optimised parsing of domains on IPv6 servers
* Remove WHITELIST_COMMAND
* Place IPv4/IPv6 availability test underneath setupVars.conf source
* Improved clarity on comments
* Define default lookupDomain on local line
* Use `getent hosts` instead of nslookup (faster)
* Make gravity_DNSLookup() function more readable
* Use bold on "Neutrino emissions detected"
* Swap conditionals around on adlists file handling
* Add comments to both gravity_Collapse() `awk`s
* Removed unnecessary "${str}" from gravity_Pull()
* Merge function variables into local line
* Place .phgbp suffice on mktemp, so patternbuffers can be cleaned up all at once in gravity_Cleanup()
* Removed success="false" from $httpCode case, placed empty success var in local
* Reordered $httpCode case numerically because I can
* Provide error if Dnsmasq format list is being parsed
* Remove IPv4 check when determining URL list (too slow on large lists)
* Check ${#sources[@]} to ensure we're checking the number of entries and not the character count
* Define empty plural in local line, removing unnecessary plural=;
* Optimised readability of gravity_Whitelist()
* Removed uninformative "Nothing to blacklist"/"No wildcards used" text
* Optimised parsing of domains into hosts format on IPv6 enabled servers
* Ensure /etc/hostname is non-zero
* Use `: >` instead of `rm` as consistent with the rest of the script
* Ensured that gravity_Cleanup() removes ${localList}.tmp
* Optimised readability of gravity_ParseUserDomains()
* Moved dnsRestart to ${var} case statement, renaming it to dnsRestartType for readability
* Set default $listType to ensure script passes "bash strict mode"

@WaLLy3K WaLLy3K added pr: Approval Required and removed WIP labels Sep 15, 2017

@WaLLy3K

This comment has been minimized.

Member

WaLLy3K commented Sep 15, 2017

Time tests against a Raspberry Pi 3B underclocked at 600Mhz:
Dev Gravity, default adlists: 17.646s
PR Gravity, default adlists: 12.865s

Dev Gravity, single adlist w/ 1m unique domains: 52.672s
PR Gravity, single adlist w/ 1m unique domains: 42.416s

Improve non-standard list parsing
* Add 504 status (Gateway connection timed out)
* Add text for non-standard list parsing
* Improve adblock parsing
* Ensure adblock exception rules are removed from file
* Ensure "www." is not treated as a URL-format list
* Corrected typo
* Ensure script does not fail if "-f" is used when there are no blocklists generated

Signed off by WaLLy3K <wally3k@pi-hole.net>
@PromoFaux

This comment has been minimized.

Member

PromoFaux commented Sep 19, 2017

Appears to only be doing hosts-file.net, and not the rest...

root@debian-512mb-lon1-01:/etc/.pihole# pihole -g
  [i] Neutrino emissions detected...
  [✓] Pulling blocklist source list into range

  [i] Target: hosts-file.net (ad_servers.txt)
  [✓] Status: Retrieval successful

  [✓] Consolidating blocklists
  [✓] Extracting domains from blocklists
  [i] 38,325 domains being pulled in by gravity
  [✓] Removing duplicate domains
  [i] 38,323 unique domains trapped in the Event Horizon

  [✓] Adding blocklist domain source to the whitelist
  [✓] Whitelisting 1 domain
  [✓] Parsing domains into hosts format
  [✓] Cleaning up stray matter

  [✓] Force-reloading DNS service
  [✓] DNS service is running
  [✓] Pi-hole blocking is Enabled
@PromoFaux

This comment has been minimized.

Member

PromoFaux commented Sep 19, 2017

Bash -x ouput:

root@debian-512mb-lon1-01:/etc/.pihole# bash -x /opt/pihole/gravity.sh
+ coltable=/opt/pihole/COL_TABLE
+ source /opt/pihole/COL_TABLE
++ [[ -t 1 ]]
+++ tput colors
++ [[ 256 -ge 8 ]]
++ COL_BOLD=''
++ COL_ULINE=''
++ COL_NC=''
++ COL_GRAY=''
++ COL_RED=''
++ COL_GREEN=''
++ COL_YELLOW=''
++ COL_BLUE=''
++ COL_PURPLE=''
++ COL_CYAN=''
++ COL_WHITE=''
++ COL_BLACK=''
++ COL_LIGHT_BLUE=''
++ COL_LIGHT_GREEN=''
++ COL_LIGHT_CYAN=''
++ COL_LIGHT_RED=''
++ COL_URG_RED=''
++ COL_LIGHT_PURPLE=''
++ COL_BROWN=''
++ COL_LIGHT_GRAY=''
++ COL_DARK_GRAY=''
++ TICK='[✓]'
++ CROSS='[✗]'
++ INFO='[i]'
++ QST='[?]'
++ DONE=' done!'
++ OVER='\r'
+ basename=pihole
+ PIHOLE_COMMAND=/usr/local/bin/pihole
+ piholeDir=/etc/pihole
+ piholeRepo=/etc/.pihole
+ adListFile=/etc/pihole/adlists.list
+ adListDefault=/etc/pihole/adlists.default
+ adListRepoDefault=/etc/.pihole/adlists.default
+ whitelistFile=/etc/pihole/whitelist.txt
+ blacklistFile=/etc/pihole/blacklist.txt
+ wildcardFile=/etc/dnsmasq.d/03-pihole-wildcard.conf
+ adList=/etc/pihole/gravity.list
+ blackList=/etc/pihole/black.list
+ localList=/etc/pihole/local.list
+ VPNList=/etc/openvpn/ipp.txt
+ domainsExtension=domains
+ matterAndLight=pihole.0.matterandlight.txt
+ parsedMatter=pihole.1.parsedmatter.txt
+ whitelistMatter=pihole.2.whitelistmatter.txt
+ accretionDisc=pihole.3.accretionDisc.txt
+ preEventHorizon=list.preEventHorizon
+ skipDownload=false
+ setupVars=/etc/pihole/setupVars.conf
+ [[ -f /etc/pihole/setupVars.conf ]]
+ source /etc/pihole/setupVars.conf
++ PIHOLE_INTERFACE=eth0
++ IPV4_ADDRESS=138.68.181.125/20
++ IPV6_ADDRESS=
++ PIHOLE_DNS_1=8.8.8.8
++ PIHOLE_DNS_2=8.8.4.4
++ QUERY_LOGGING=true
++ INSTALL_WEB=true
++ LIGHTTPD_ENABLED=1
++ WEBPASSWORD=5c3d70f930d5826c23cd7b1f9fd94c409f2362eb9e165530fa6d69354bf90c12
+ IPV4_ADDRESS=138.68.181.125
+ IPV6_ADDRESS=
+ [[ -z 138.68.181.125 ]]
+ [[ -r /etc/pihole/pihole.conf ]]
+ gravity_Trap
+ trap '{ echo -e "\\n\\n  ${INFO} ${COL_LIGHT_RED}User-abort detected${COL_NC}"; gravity_Cleanup "error"; }' INT
+ [[ '' == true ]]
+ [[ false == false ]]
+ gravity_DNSLookup
+ local lookupDomain=pi.hole plural=
+ [[ ! -e /etc/pihole/local.list ]]
+ timeout 5 getent hosts pi.hole
+ [[ -n '' ]]
+ return 0
+ gravity_Collapse
+ echo -e '  [i] Neutrino emissions detected...'
  [i] Neutrino emissions detected...
+ [[ ! -f /etc/pihole/adlists.list ]]
+ [[ -f /etc/pihole/adlists.default ]]
+ local 'str=Pulling blocklist source list into range'
+ echo -ne '  [i] Pulling blocklist source list into range...'
  [i] Pulling blocklist source list into range...+ mapfile -t sources
++ awk '!/^[#@;!\[]/ {
      # Remove windows CR line endings
      gsub(/\r$/, "", $0)
      # Print non-empty line
      if ($1) { print $1 }
    }' /etc/pihole/adlists.list
+ mapfile -t sourceDomains
++ awk -F '[/:]' '{
      # Remove URL protocol & optional username:password@
      gsub(/(.*:\/\/|.*:.*@)/, "", $0)
      print $1
    }'
+++ printf '%s\n' 'https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts https://mirror1.malwaredomains.com/files/justdomains http://sysctl.org/cameleon/hosts https://zeustracker.abuse.ch/blocklist.php?download=domainblocklist https://s3.amazonaws.com/lists.disconnect.me/simple_tracking.txt https://s3.amazonaws.com/lists.disconnect.me/simple_ad.txt https://hosts-file.net/ad_servers.txt'
+ [[ -n https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts https://mirror1.malwaredomains.com/files/justdomains http://sysctl.org/cameleon/hosts https://zeustracker.abuse.ch/blocklist.php?download=domainblocklist https://s3.amazonaws.com/lists.disconnect.me/simple_tracking.txt https://s3.amazonaws.com/lists.disconnect.me/simple_ad.txt https://hosts-file.net/ad_servers.txt ]]
+ [[ -n hosts-file.net ]]
+ echo -e '\r  [✓] Pulling blocklist source list into range'
  [✓] Pulling blocklist source list into range
+ gravity_Supernova
+ local url domain agent cmd_ext str
+ echo ''

+ (( i = 0 ))
+ (( i < 1 ))
+ url='https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts https://mirror1.malwaredomains.com/files/justdomains http://sysctl.org/cameleon/hosts https://zeustracker.abuse.ch/blocklist.php?download=domainblocklist https://s3.amazonaws.com/lists.disconnect.me/simple_tracking.txt https://s3.amazonaws.com/lists.disconnect.me/simple_ad.txt https://hosts-file.net/ad_servers.txt'
+ domain=hosts-file.net
+ saveLocation=/etc/pihole/list.0.hosts-file.net.domains
+ activeDomains[$i]=/etc/pihole/list.0.hosts-file.net.domains
+ agent='Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2227.0 Safari/537.36'
+ case "${domain}" in
+ cmd_ext=
+ [[ false == false ]]
+ echo -e '  [i] Target: hosts-file.net (ad_servers.txt)'
  [i] Target: hosts-file.net (ad_servers.txt)
+ gravity_Pull 'https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts https://mirror1.malwaredomains.com/files/justdomains http://sysctl.org/cameleon/hosts https://zeustracker.abuse.ch/blocklist.php?download=domainblocklist https://s3.amazonaws.com/lists.disconnect.me/simple_tracking.txt https://s3.amazonaws.com/lists.disconnect.me/simple_ad.txt https://hosts-file.net/ad_servers.txt' '' 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2227.0 Safari/537.36'
+ local 'url=https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts https://mirror1.malwaredomains.com/files/justdomains http://sysctl.org/cameleon/hosts https://zeustracker.abuse.ch/blocklist.php?download=domainblocklist https://s3.amazonaws.com/lists.disconnect.me/simple_tracking.txt https://s3.amazonaws.com/lists.disconnect.me/simple_ad.txt https://hosts-file.net/ad_servers.txt' cmd_ext= 'agent=Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2227.0 Safari/537.36' heisenbergCompensator= patternBuffer str httpCode success=
++ mktemp -p /tmp --suffix=.phgpb
+ patternBuffer=/tmp/tmp.HYJN9CXcqU.phgpb
+ [[ -r /etc/pihole/list.0.hosts-file.net.domains ]]
+ heisenbergCompensator='-z /etc/pihole/list.0.hosts-file.net.domains'
+ str=Status:
+ echo -ne '  [i] Status: Pending...'
  [i] Status: Pending...++ curl -s -L -z /etc/pihole/list.0.hosts-file.net.domains -w '%{http_code}' -A 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2227.0 Safari/537.36' 'https://raw.githubusercontent.com/StevenBlack/hosts/master/hosts https://mirror1.malwaredomains.com/files/justdomains http://sysctl.org/cameleon/hosts https://zeustracker.abuse.ch/blocklist.php?download=domainblocklist https://s3.amazonaws.com/lists.disconnect.me/simple_tracking.txt https://s3.amazonaws.com/lists.disconnect.me/simple_ad.txt https://hosts-file.net/ad_servers.txt' -o /tmp/tmp.HYJN9CXcqU.phgpb
+ httpCode=200
+ case "${httpCode}" in
+ echo -e '\r  [✓] Status: Retrieval successful'
  [✓] Status: Retrieval successful
+ success=true
+ [[ true == true ]]
+ [[ 200 == \3\0\4 ]]
+ [[ -s /tmp/tmp.HYJN9CXcqU.phgpb ]]
+ gravity_ParseFileIntoDomains /tmp/tmp.HYJN9CXcqU.phgpb /etc/pihole/list.0.hosts-file.net.domains
+ local source=/tmp/tmp.HYJN9CXcqU.phgpb destination=/etc/pihole/list.0.hosts-file.net.domains commentPattern firstLine abpFilter
+ [[ /tmp/tmp.HYJN9CXcqU.phgpb == \/\e\t\c\/\p\i\h\o\l\e\/\p\i\h\o\l\e\.\0\.\m\a\t\t\e\r\a\n\d\l\i\g\h\t\.\t\x\t ]]
+ read -r firstLine
+ [[ # this hosts file is a merged collection of hosts from reputable sources, =~ (adblock|ublock|^!) ]]
+ grep -q '^address=/' /tmp/tmp.HYJN9CXcqU.phgpb
+ grep -q -E '^https?://' /tmp/tmp.HYJN9CXcqU.phgpb
+ output='++ mv /tmp/tmp.HYJN9CXcqU.phgpb /etc/pihole/list.0.hosts-file.net.domains'
+ [[ ! -e /etc/pihole/list.0.hosts-file.net.domains ]]
+ echo ''

+ (( i++ ))
+ (( i < 1 ))
+ gravity_Schwarzschild
+ local str lastLine
+ str='Consolidating blocklists'
+ echo -ne '  [i] Consolidating blocklists...'
  [i] Consolidating blocklists...+ :
+ for i in '"${activeDomains[@]}"'
+ [[ -r /etc/pihole/list.0.hosts-file.net.domains ]]
+ tr -d '\r'
++ tail -1 /etc/pihole/pihole.0.matterandlight.txt
+ lastLine=
+ [[ 0 -gt 0 ]]
+ echo -e '\r  [✓] Consolidating blocklists'
  [✓] Consolidating blocklists
+ gravity_Filter
+ local str num
+ str='Extracting domains from blocklists'
+ echo -ne '  [i] Extracting domains from blocklists...'
  [i] Extracting domains from blocklists...+ gravity_ParseFileIntoDomains /etc/pihole/pihole.0.matterandlight.txt /etc/pihole/pihole.1.parsedmatter.txt
+ local source=/etc/pihole/pihole.0.matterandlight.txt destination=/etc/pihole/pihole.1.parsedmatter.txt commentPattern firstLine abpFilter
+ [[ /etc/pihole/pihole.0.matterandlight.txt == \/\e\t\c\/\p\i\h\o\l\e\/\p\i\h\o\l\e\.\0\.\m\a\t\t\e\r\a\n\d\l\i\g\h\t\.\t\x\t ]]
+ commentPattern='[#;@![\/]'
+ awk '!/^[#;@![\/]/ {
      # Determine if there are multiple words seperated by a space
      if(NF>1) {
        # Remove comments (including prefixed spaces/tabs)
        if($0 ~ /[#;@![\/]/) { gsub("( |\t)[#;@![\/].*", "", $0) }
        # Determine if there are aliased domains
        if($3) {
          # Remove IP address
          $1=""
          # Remove space which is left in $0 when removing $1
          gsub("^ ", "", $0)
          print $0
        } else if($2) {
          # Print single domain without IP
          print $2
        }
      # If there are no words seperated by space
      } else if($1) {
        print $1
      }
    }' /etc/pihole/pihole.0.matterandlight.txt
+ return 0
+++ wc -l
++ printf '%'\''.0f' 38325
+ num=38,325
+ echo -e '\r  [✓] Extracting domains from blocklists
  [i] 38,325 domains being pulled in by gravity'
  [✓] Extracting domains from blocklists
  [i] 38,325 domains being pulled in by gravity
+ str='Removing duplicate domains'
+ echo -ne '  [i] Removing duplicate domains...'
  [i] Removing duplicate domains...+ sort -u /etc/pihole/pihole.1.parsedmatter.txt
+ echo -e '\r  [✓] Removing duplicate domains'
  [✓] Removing duplicate domains
+++ wc -l
++ printf '%'\''.0f' 38323
+ num=38,323
+ echo -e '  [i] 38,323 unique domains trapped in the Event Horizon'
  [i] 38,323 unique domains trapped in the Event Horizon
+ gravity_WhitelistBLD
+ local plural= str uniqDomains
+ echo ''

+ [[ 1 -ne 1 ]]
+ str='Adding blocklist domain source to the whitelist'
+ echo -ne '  [i] Adding blocklist domain source to the whitelist...'
  [i] Adding blocklist domain source to the whitelist...+ mapfile -t uniqDomains
++ awk '{ if(!a[$1]++) { print $1 } }'
+++ printf '%s\n' hosts-file.net
+ /usr/local/bin/pihole -w -nr -q hosts-file.net
+ echo -e '\r  [✓] Adding blocklist domain source to the whitelist'
  [✓] Adding blocklist domain source to the whitelist
+ [[ false == false ]]
+ gravity_Whitelist
+ local num plural= str
+ [[ ! -f /etc/pihole/whitelist.txt ]]
++ wc -l
+ num=1
+ [[ 1 -ne 1 ]]
+ str='Whitelisting 1 domain'
+ echo -ne '  [i] Whitelisting 1 domain...'
  [i] Whitelisting 1 domain...+ grep -F -x -v -f /etc/pihole/whitelist.txt /etc/pihole/list.preEventHorizon
+ echo -e '\r  [✓] Whitelisting 1 domain'
  [✓] Whitelisting 1 domain
+ gravity_ShowBlockCount
+ local num plural
+ [[ -f /etc/pihole/blacklist.txt ]]
+ [[ -f /etc/dnsmasq.d/03-pihole-wildcard.conf ]]
+ [[ false == false ]]
+ str='Parsing domains into hosts format'
+ echo -ne '  [i] Parsing domains into hosts format...'
  [i] Parsing domains into hosts format...+ gravity_ParseUserDomains
+ [[ ! -f /etc/pihole/blacklist.txt ]]
+ return 0
+ [[ ! '' == \b\l\a\c\k\l\i\s\t ]]
+ gravity_ParseLocalDomains
+ local hostname
+ [[ -s /etc/hostname ]]
+ hostname=debian-512mb-lon1-01
+ echo -e 'debian-512mb-lon1-01\npi.hole'
+ :
+ gravity_ParseDomainsIntoHosts /etc/pihole/local.list.tmp /etc/pihole/local.list
+ awk -v ipv4=138.68.181.125 -v ipv6= '{
    # Remove windows CR line endings
    sub(/\r$/, "")
    # Parse each line as "ipaddr domain"
    if(ipv6 && ipv4) {
      print ipv4" "$0"\n"ipv6" "$0
    } else if(!ipv6) {
      print ipv4" "$0
    } else {
      print ipv6" "$0
    }
  }'
+ [[ -f /etc/openvpn/ipp.txt ]]
+ gravity_ParseBlacklistDomains
+ local output status
+ :
+ gravity_ParseDomainsIntoHosts /etc/pihole/pihole.2.whitelistmatter.txt /etc/pihole/pihole.3.accretionDisc.txt
+ awk -v ipv4=138.68.181.125 -v ipv6= '{
    # Remove windows CR line endings
    sub(/\r$/, "")
    # Parse each line as "ipaddr domain"
    if(ipv6 && ipv4) {
      print ipv4" "$0"\n"ipv6" "$0
    } else if(!ipv6) {
      print ipv4" "$0
    } else {
      print ipv6" "$0
    }
  }'
+ output='++ mv /etc/pihole/pihole.3.accretionDisc.txt /etc/pihole/gravity.list'
+ status=0
+ [[ 0 -ne 0 ]]
+ echo -e '\r  [✓] Parsing domains into hosts format'
  [✓] Parsing domains into hosts format
+ [[ false == false ]]
+ gravity_Cleanup
+ local error=
+ str='Cleaning up stray matter'
+ echo -ne '  [i] Cleaning up stray matter...'
  [i] Cleaning up stray matter...+ rm /etc/pihole/pihole.0.matterandlight.txt /etc/pihole/pihole.1.parsedmatter.txt /etc/pihole/pihole.2.whitelistmatter.txt
+ rm /etc/pihole/local.list.tmp
+ rm /tmp/tmp.CXioaWm9kp.phgpb
+ for file in '${piholeDir}/*.${domainsExtension}'
+ [[ ! /etc/pihole/list.0.hosts-file.net.domains == *\/\e\t\c\/\p\i\h\o\l\e\/\l\i\s\t\.\0\.\h\o\s\t\s\-\f\i\l\e\.\n\e\t\.\d\o\m\a\i\n\s* ]]
+ echo -e '\r  [✓] Cleaning up stray matter'
  [✓] Cleaning up stray matter
+ pidof dnsmasq
+ [[ -n '' ]]
+ echo ''

+ [[ -z '' ]]
+ /usr/local/bin/pihole restartdns force-reload
  [✓] Force-reloading DNS service
+ /usr/local/bin/pihole status
  [✓] DNS service is running
  [✓] Pi-hole blocking is Enabled
gravity.sh Outdated
fi
local str="Pulling blocklist source list into range"
echo -ne " ${INFO} ${str}..."
# Retrieve source URLs from $adListFile
# Awk Logic: Remove comments (#@;![), CR (windows) line endings and empty lines
mapfile -t sources < <(awk '!/^[#@;!\[]/ {gsub(/\r$/, "", $0); if ($1) { print $1 } }' "${adListFile}" 2> /dev/null)
mapfile -t sources <<< $(

This comment has been minimized.

@PromoFaux

PromoFaux Sep 19, 2017

Member

Confimed combination of this change and...

gravity.sh Outdated
# Parse source domains from $sources
# Awk Logic: Split by folder/port, remove URL protocol & optional username:password@
mapfile -t sourceDomains < <(
mapfile -t sourceDomains <<< $(

This comment has been minimized.

@PromoFaux

PromoFaux Sep 19, 2017

Member

this one causing only one adlist (the last one) to be downloaded.

Tested with the following changes locally:

#  # Retrieve source URLs from $adListFile
#  mapfile -t sources <<< $(
#    # Logic: Remove comments (#@;![)
#    awk '!/^[#@;!\[]/ {
#      # Remove windows CR line endings
#      gsub(/\r$/, "", $0)
#      # Print non-empty line
#      if ($1) { print $1 }
#    }' "${adListFile}" 2> /dev/null
#  )

mapfile -t sources < <(awk '!/^[#@;!\[]/ {gsub(/\r$/, "", $0); if ($1) { print $1 } }' "${adListFile}" 2> /dev/null)

  # Parse source domains from $sources
#  mapfile -t sourceDomains <<< $(
mapfile -t sourceDomains < <(
    # Logic: Split by folder/port
    awk -F '[/:]' '{
      # Remove URL protocol & optional username:password@
      gsub(/(.*:\/\/|.*:.*@)/, "", $0)
      print $1
    }' <<< "$(printf '%s\n' "${sources[@]}")" 2> /dev/null
  )

  if [[ -n "${sources[*]}" ]] && [[ -n "${sourceDomains[*]}" ]]; then
    echo -e "${OVER}  ${TICK} ${str}"
  else
    echo -e "${OVER}  ${CROSS} ${str}"
    gravity_Cleanup "error"
  fi
}
`<<<$()` back to `< <()`
Signed-off-by: Adam Warner <adamw@rner.email>

Fixed in 8d8482d, though this may not be the correct course of action

WaLLy3K and others added some commits Sep 20, 2017

Fix gravity from only parsing one adlist URL
* Redirect `grep` correctly to $sources (instead of using `awk`)
* Redirect $sourceDomains correctly
* Replace use of ${COL_LIGHT_BLUE}
* Add numeric count informing user of unique source domains being whitelisted

@WaLLy3K WaLLy3K added WIP and removed pr: Approval Required labels Sep 21, 2017

@WaLLy3K WaLLy3K removed the WIP label Sep 21, 2017

@DL6ER

This comment has been minimized.

Member

DL6ER commented Sep 21, 2017

This PR fixes a bug that prevents whitelisting from working correctly. The mentioned bug was introduced in f24ab85.

Ensure resolver test occurs each second
* Ensure that gravity will run the second the resolver is available
* Increase timeout to 120s
@PromoFaux

This comment has been minimized.

Member

PromoFaux commented Sep 21, 2017

Approved

Approved with PullApprove

@PromoFaux PromoFaux merged commit ef1ce7d into development Sep 21, 2017

5 checks passed

codacy/pr Good work! A positive pull request.
Details
code-review/pullapprove Approved by PromoFaux, WaLLy3K
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@PromoFaux PromoFaux deleted the tweak/gravity branch Sep 21, 2017

@WaLLy3K WaLLy3K referenced this pull request Sep 22, 2017

Merged

Ensure domains files are not deleted upon w/blist #1702

5 of 5 tasks complete

WaLLy3K added a commit that referenced this pull request Sep 26, 2017

echo -e " ${INFO} Received empty file, ${COL_LIGHT_GREEN}using cached one${COL_NC} (list not updated!)"
fi
# Determine error output message
if pidof dnsmasq &> /dev/null; then

This comment has been minimized.

@jacobsalmela

jacobsalmela Oct 28, 2017

Member

@WaLLy3K correct me if I'm wrong, but this is saying if dnsmasq has a PID, then the message DNS resolution is currently unavailable is displayed to the user. But if dnsmasq is running, then resolution would be possible.

This comment has been minimized.

@WaLLy3K

WaLLy3K Oct 28, 2017

Member

pi-hole/gravity.sh

Lines 79 to 85 in 47099e2

# Determine if $lookupDomain is resolvable
if timeout 1 getent hosts "${lookupDomain}" &> /dev/null; then
# Print confirmation of resolvability if it had previously failed
if [[ -n "${secs:-}" ]]; then
echo -e "${OVER} ${TICK} DNS resolution is now available\\n"
fi
return 0

If resolution is available, the function is exited using return 0, otherwise the function continues as if resolution is not available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment