diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cf84a28a..ce948e09 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -55,7 +55,6 @@ jobs: ubuntu_22, centos_8, centos_9, - fedora_35, fedora_36, fedora_37, ] diff --git a/advanced/Templates/pihole-FTL.service b/advanced/Templates/pihole-FTL.service index 15096972..460339ae 100644 --- a/advanced/Templates/pihole-FTL.service +++ b/advanced/Templates/pihole-FTL.service @@ -23,6 +23,11 @@ is_running() { return 1 } +cleanup() { + # Run post-stop script, which does cleanup among runtime files + sh "${PI_HOLE_SCRIPT_DIR}/pihole-FTL-poststop.sh" +} + # Start the service start() { @@ -33,10 +38,16 @@ start() { sh "${PI_HOLE_SCRIPT_DIR}/pihole-FTL-prestart.sh" if setcap CAP_NET_BIND_SERVICE,CAP_NET_RAW,CAP_NET_ADMIN,CAP_SYS_NICE,CAP_IPC_LOCK,CAP_CHOWN+eip "/usr/bin/pihole-FTL"; then - su -s /bin/sh -c "/usr/bin/pihole-FTL" pihole || exit $? + su -s /bin/sh -c "/usr/bin/pihole-FTL" pihole else echo "Warning: Starting pihole-FTL as root because setting capabilities is not supported on this system" - /usr/bin/pihole-FTL || exit $? + /usr/bin/pihole-FTL + fi + rc=$? + # Cleanup if startup failed + if [ "${rc}" != 0 ]; then + cleanup + exit $rc fi echo fi @@ -65,8 +76,7 @@ stop() { else echo "Not running" fi - # Run post-stop script, which does cleanup among runtime files - sh "${PI_HOLE_SCRIPT_DIR}/pihole-FTL-poststop.sh" + cleanup echo } @@ -84,6 +94,9 @@ status() { ### main logic ### +# catch sudden termination +trap 'cleanup; exit 1' INT HUP TERM ABRT + # Get FTL's PID file path FTL_PID_FILE="$(getFTLPIDFile)" diff --git a/advanced/lighttpd.conf.debian b/advanced/lighttpd.conf.debian index 06c284fe..5a96c6cd 100644 --- a/advanced/lighttpd.conf.debian +++ b/advanced/lighttpd.conf.debian @@ -17,7 +17,6 @@ server.modules = ( "mod_access", - "mod_accesslog", "mod_auth", "mod_expire", "mod_redirect", @@ -34,8 +33,6 @@ server.groupname = "www-data" # For lighttpd version 1.4.46 or above, the port can be overwritten in `/etc/lighttpd/external.conf` using the := operator # e.g. server.port := 8000 server.port = 80 -accesslog.filename = "/var/log/lighttpd/access-pihole.log" -accesslog.format = "%{%s}t|%V|%r|%s|%b" # Allow streaming response # reference: https://redmine.lighttpd.net/projects/lighttpd/wiki/Server_stream-response-bodyDetails diff --git a/advanced/lighttpd.conf.fedora b/advanced/lighttpd.conf.fedora index 04f3ee01..6276bfcb 100644 --- a/advanced/lighttpd.conf.fedora +++ b/advanced/lighttpd.conf.fedora @@ -35,8 +35,6 @@ server.groupname = "lighttpd" # For lighttpd version 1.4.46 or above, the port can be overwritten in `/etc/lighttpd/external.conf` using the := operator # e.g. server.port := 8000 server.port = 80 -accesslog.filename = "/var/log/lighttpd/access-pihole.log" -accesslog.format = "%{%s}t|%V|%r|%s|%b" # Allow streaming response # reference: https://redmine.lighttpd.net/projects/lighttpd/wiki/Server_stream-response-bodyDetails diff --git a/advanced/pihole-admin.conf b/advanced/pihole-admin.conf index 2809d339..ef15c8fe 100644 --- a/advanced/pihole-admin.conf +++ b/advanced/pihole-admin.conf @@ -23,7 +23,7 @@ $HTTP["url"] =~ "^/admin/" { fastcgi.server = ( ".php" => ( "localhost" => ( - "socket" => "/tmp/pihole-php-fastcgi.socket", + "socket" => "/run/lighttpd/pihole-php-fastcgi.socket", "bin-path" => "/usr/bin/php-cgi", "min-procs" => 0, "max-procs" => 1, @@ -79,4 +79,4 @@ $HTTP["host"] == "pi.hole" { } # (keep this on one line for basic-install.sh filtering during install) -server.modules += ( "mod_access", "mod_redirect", "mod_fastcgi", "mod_setenv" ) +server.modules += ( "mod_access", "mod_accesslog", "mod_redirect", "mod_fastcgi", "mod_setenv" ) diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh index cf27e3ac..b1bd773e 100755 --- a/automated install/basic-install.sh +++ b/automated install/basic-install.sh @@ -1400,6 +1400,9 @@ installConfigs() { # If the user chose to install the dashboard, if [[ "${INSTALL_WEB_SERVER}" == true ]]; then + # set permissions on /etc/lighttpd/lighttpd.conf so pihole user (other) can read the file + chmod o+x /etc/lighttpd + chmod o+r "${lighttpdConfig}" if grep -q -F "FILE AUTOMATICALLY OVERWRITTEN BY PI-HOLE" "${lighttpdConfig}"; then # Attempt to preserve backwards compatibility with older versions install -D -m 644 -T ${PI_HOLE_LOCAL_REPO}/advanced/${LIGHTTPD_CFG} "${lighttpdConfig}" @@ -1432,17 +1435,26 @@ installConfigs() { elif [[ -d "/etc/lighttpd/conf-available" ]]; then conf=/etc/lighttpd/conf-available/15-pihole-admin.conf install -D -m 644 -T ${PI_HOLE_LOCAL_REPO}/advanced/pihole-admin.conf $conf - # disable server.modules += ( ... ) in $conf to avoid module dups - # (needed until Debian 10 no longer supported by pi-hole) - # (server.modules duplication is ignored in lighttpd 1.4.56+) - if awk '!/^server\.modules/{print}' $conf > $conf.$$ && mv $conf.$$ $conf; then + + # Get the version number of lighttpd + version=$(dpkg-query -f='${Version}\n' --show lighttpd) + # Test if that version is greater than or euqal to 1.4.56 + if dpkg --compare-versions "$version" "ge" "1.4.56"; then + # If it is, then we don't need to disable the modules + # (server.modules duplication is ignored in lighttpd 1.4.56+) : else - rm $conf.$$ + # disable server.modules += ( ... ) in $conf to avoid module dups + if awk '!/^server\.modules/{print}' $conf > $conf.$$ && mv $conf.$$ $conf; then + : + else + rm $conf.$$ + fi fi + chmod 644 $conf if is_command lighty-enable-mod ; then - lighty-enable-mod pihole-admin access redirect fastcgi setenv > /dev/null || true + lighty-enable-mod pihole-admin access accesslog redirect fastcgi setenv > /dev/null || true else # Otherwise, show info about installing them printf " %b Warning: 'lighty-enable-mod' utility not found\\n" "${INFO}" @@ -2717,12 +2729,12 @@ main() { restart_service pihole-FTL - # Update local and remote versions via updatechecker - /opt/pihole/updatecheck.sh - # Download and compile the aggregated block list runGravity + # Update local and remote versions via updatechecker + /opt/pihole/updatecheck.sh + if [[ "${useUpdateVars}" == false ]]; then displayFinalMessage "${pw}" fi diff --git a/gravity.sh b/gravity.sh index a5c944ce..c2795442 100755 --- a/gravity.sh +++ b/gravity.sh @@ -244,7 +244,7 @@ database_adlist_number() { return; fi - output=$( { printf ".timeout 30000\\nUPDATE adlist SET number = %i, invalid_domains = %i WHERE id = %i;\\n" "${num_source_lines}" "${num_invalid}" "${1}" | pihole-FTL sqlite3 "${gravityDBfile}"; } 2>&1 ) + output=$( { printf ".timeout 30000\\nUPDATE adlist SET number = %i, invalid_domains = %i WHERE id = %i;\\n" "${num_domains}" "${num_non_domains}" "${1}" | pihole-FTL sqlite3 "${gravityDBfile}"; } 2>&1 ) status="$?" if [[ "${status}" -ne 0 ]]; then @@ -519,12 +519,12 @@ gravity_DownloadBlocklists() { gravity_Blackbody=true } -# num_target_lines does increase for every correctly added domain in pareseList() -num_target_lines=0 -num_source_lines=0 -num_invalid=0 +# num_total_imported_domains increases for each list processed +num_total_imported_domains=0 +num_domains=0 +num_non_domains=0 parseList() { - local adlistID="${1}" src="${2}" target="${3}" incorrect_lines sample_incorrect_lines + local adlistID="${1}" src="${2}" target="${3}" non_domains sample_non_domains tmp_non_domains_str false_positive # This sed does the following things: # 1. Remove all lines containing no domains # 2. Remove all domains containing invalid characters. Valid are: a-z, A-Z, 0-9, dot (.), minus (-), underscore (_) @@ -534,36 +534,65 @@ parseList() { sed -r "/([^\.]+\.)+[^\.]{2,}/!d;/[^a-zA-Z0-9.\_-]/d;s/\.$//;s/$/,${adlistID}/;/.$/a\\" "${src}" >> "${target}" # Find lines containing no domains or with invalid characters (see above) - # Remove duplicates and limit to 5 domains - mapfile -t incorrect_lines <<< "$(sed -r "/([^\.]+\.)+[^\.]{2,}/d" < "${src}")" - mapfile -t -O "${#incorrect_lines[@]}" incorrect_lines <<< "$(sed -r "/[^a-zA-Z0-9.\_-]/!d" < "${src}")" - IFS=" " read -r -a sample_incorrect_lines <<< "$(tr ' ' '\n' <<< "${incorrect_lines[@]}" | sort -u | head -n 5| tr '\n' ' ')" - - local num_target_lines_new num_correct_lines - # Get number of lines in source file - num_source_lines="$(grep -c "^" "${src}")" - # Get the new number of lines in destination file - num_target_lines_new="$(grep -c "^" "${target}")" - # Number of new correctly added lines - num_correct_lines="$(( num_target_lines_new-num_target_lines ))" - # Update number of lines in target file - num_target_lines="$num_target_lines_new" - num_invalid="$(( num_source_lines-num_correct_lines ))" - if [[ "${num_invalid}" -eq 0 ]]; then - echo " ${INFO} Analyzed ${num_source_lines} domains" - else - echo " ${INFO} Analyzed ${num_source_lines} domains, ${num_invalid} domains invalid!" - fi - - # Display sample of invalid lines if we found some - if [ ${#sample_incorrect_lines[@]} -ne 0 ]; then - echo " Sample of invalid domains:" - for each in "${sample_incorrect_lines[@]}" + # Remove duplicates from the list + mapfile -t non_domains <<< "$(sed -r "/([^\.]+\.)+[^\.]{2,}/d" < "${src}")" + mapfile -t -O "${#non_domains[@]}" non_domains <<< "$(sed -r "/[^a-zA-Z0-9.\_-]/!d" < "${src}")" + IFS=" " read -r -a non_domains <<< "$(tr ' ' '\n' <<< "${non_domains[@]}" | sort -u | tr '\n' ' ')" + + # A list of items of common local hostnames not to report as unusable + # Some lists (i.e StevenBlack's) contain these as they are supposed to be used as HOST files + # but flagging them as unusable causes more confusion than it's worth - so we suppress them from the output + false_positives=( + "localhost" + "localhost.localdomain" + "local" + "broadcasthost" + "localhost" + "ip6-localhost" + "ip6-loopback" + "lo0 localhost" + "ip6-localnet" + "ip6-mcastprefix" + "ip6-allnodes" + "ip6-allrouters" + "ip6-allhosts" + ) + + # Read the unusable lines into a string + tmp_non_domains_str=" ${non_domains[*]} " + for false_positive in "${false_positives[@]}"; do + # Remove false positives from tmp_non_domains_str + tmp_non_domains_str="${tmp_non_domains_str/ ${false_positive} / }" + done + # Read the string back into an array + IFS=" " read -r -a non_domains <<< "${tmp_non_domains_str}" + + # Get a sample of non-domain entries, limited to 5 (the list should already have been de-duplicated) + IFS=" " read -r -a sample_non_domains <<< "$(tr ' ' '\n' <<< "${non_domains[@]}" | head -n 5 | tr '\n' ' ')" + + local tmp_new_imported_total + # Get the new number of domains in destination file + tmp_new_imported_total="$(grep -c "^" "${target}")" + # Number of imported lines for this file is the difference between the new total and the old total. (Or, the number of domains we just added.) + num_domains="$(( tmp_new_imported_total-num_total_imported_domains ))" + # Replace the running total with the new total. + num_total_imported_domains="$tmp_new_imported_total" + # Get the number of non_domains (this is the number of entries left after stripping the source of comments/duplicates/false positives/domains) + num_non_domains="${#non_domains[@]}" + + # If there are unusable lines, we display some information about them. This is not error or major cause for concern. + if [[ "${num_non_domains}" -ne 0 ]]; then + echo " ${INFO} Imported ${num_domains} domains, ignoring ${num_non_domains} non-domain entries" + echo " Sample of non-domain entries:" + for each in "${sample_non_domains[@]}" do - echo " - ${each}" + echo " - ${each}" done + else + echo " ${INFO} Imported ${num_domains} domains" fi } + compareLists() { local adlistID="${1}" target="${2}" @@ -716,8 +745,8 @@ gravity_DownloadBlocklistFromUrl() { else echo -e " ${CROSS} List download failed: ${COL_LIGHT_RED}no cached list available${COL_NC}" # Manually reset these two numbers because we do not call parseList here - num_source_lines=0 - num_invalid=0 + num_domains=0 + num_non_domains=0 database_adlist_number "${adlistID}" database_adlist_status "${adlistID}" "4" fi diff --git a/test/_fedora_35.Dockerfile b/test/_fedora_35.Dockerfile deleted file mode 100644 index 83c17650..00000000 --- a/test/_fedora_35.Dockerfile +++ /dev/null @@ -1,18 +0,0 @@ -FROM fedora:35 -RUN dnf install -y git initscripts - -ENV GITDIR /etc/.pihole -ENV SCRIPTDIR /opt/pihole - -RUN mkdir -p $GITDIR $SCRIPTDIR /etc/pihole -ADD . $GITDIR -RUN cp $GITDIR/advanced/Scripts/*.sh $GITDIR/gravity.sh $GITDIR/pihole $GITDIR/automated\ install/*.sh $SCRIPTDIR/ -ENV PATH /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:$SCRIPTDIR - -RUN true && \ - chmod +x $SCRIPTDIR/* - -ENV SKIP_INSTALL true -ENV OS_CHECK_DOMAIN_NAME dev-supportedos.pi-hole.net - -#sed '/# Start the installer/Q' /opt/pihole/basic-install.sh > /opt/pihole/stub_basic-install.sh && \ diff --git a/test/requirements.txt b/test/requirements.txt index e891242c..686f2a58 100644 --- a/test/requirements.txt +++ b/test/requirements.txt @@ -2,5 +2,5 @@ docker-compose == 1.29.2 pytest == 7.2.1 pytest-xdist == 3.1.0 pytest-testinfra == 7.0.0 -tox == 4.2.8 +tox == 4.3.5 diff --git a/test/tox.fedora_35.ini b/test/tox.fedora_35.ini deleted file mode 100644 index c571a564..00000000 --- a/test/tox.fedora_35.ini +++ /dev/null @@ -1,8 +0,0 @@ -[tox] -envlist = py3 - -[testenv:py3] -allowlist_externals = docker -deps = -rrequirements.txt -commands = docker build -f _fedora_35.Dockerfile -t pytest_pihole:test_container ../ - pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py ./test_centos_fedora_common_support.py ./test_fedora_support.py