Update instructions

- Fix missing 'return' statements
- Close filehandles lexically sooner after opening (a lot of times I was
  relying on implicit closing from scope exit)
- Fix postfix if/unless/while
- Fix multi-line map{}
- Fix confusing unless()
- Use croak() instead of die()

CHANGELOG.md

@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+- Require Perl 5.12
+
+### Fixed
+- Fix warning when writing blocklist to stdout.
+- Various script clean-ups
+
 ## [0.2.0] - 2023-06-19
 
 ### Added
@@ -28,3 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [0.1.0] - 2022-11-12
 Initial published version
+
+[Unreleased]: https://codeberg.org/h3xx/you-dont-need-pihole/compare/v0.2.0...HEAD
+[0.2.0]: https://codeberg.org/h3xx/you-dont-need-pihole/compare/v0.1.0...v0.2.0
+[0.1.0]: https://codeberg.org/h3xx/you-dont-need-pihole/releases/tag/v0.1.0

README.md

@@ -3,7 +3,9 @@
 **Network-wide DNS blocking without extra hardware.**
 
 This project implements the ad-blocking functionality of a
-[Pi-hole](https://pi-hole.net) without needing any extra hardware.
+[Pi-hole](https://pi-hole.net) without needing any extra hardware. All you need
+is a Linux computer on your network. This might also work with a Mac OSX
+computer, but I haven't tested it.
 
 A Pi-hole is a [Raspberry Pi](https://www.raspberrypi.com/) based "black hole"
 for Internet advertisements. It works by intercepting and answering DNS queries
@@ -22,12 +24,20 @@ You'll need `dnsmasq` installed for this.
 git clone --recursive https://codeberg.org/h3xx/you-dont-need-pihole.git /etc/you-dont-need-pihole
 ```
 
-2. Run `update.sh` to generate the blocklist.
+2. Run `update.sh` to generate the initial blocklist:
 
-3. Update `dnsmasq.d/01-you-dont-need-pihole.conf`, replacing `/etc/you-dont-need-pihole` with wherever the root
-of this project is installed to.
+```sh
+/etc/you-dont-need-pihole/update.sh
+```
 
-4. Add the configuration directory to `dnsmasq.conf`:
+> [!IMPORTANT]
+> If you didn't use `/etc/you-dont-need-pihole` as the installation directory,
+> update `dnsmasq.d/01-you-dont-need-pihole.conf`, replacing
+> `/etc/you-dont-need-pihole` with wherever the root of this project is
+> installed to.
+
+4. Add You Don't Need Pi Hole's `dnsmasq.d` configuration directory path to
+   `dnsmasq.conf`:
 
 ```sh
 echo 'conf-dir=/etc/you-dont-need-pihole/dnsmasq.d' >> /etc/dnsmasq.conf
@@ -36,21 +46,32 @@ echo 'conf-dir=/etc/you-dont-need-pihole/dnsmasq.d' >> /etc/dnsmasq.conf
 You can also copy or symlink `dnsmasq.d/01-you-dont-need-pihole.conf` if you need
 the config to live somewhere else.
 
-5. Restart the `dnsmasq` service.
+5. Restart the `dnsmasq` service:
 
-6. Make sure `53/udp` is unfirewalled.
+```sh
+# Debian & Ubuntu, RedHat & CentOS
+service dnsmasq restart
+```
 
-7. Go into your router settings and change the IP addressed provied via DHCP to
-   be your server's local IP address. [See this thread for a
-   walkthrough.](https://discourse.pi-hole.net/t/how-do-i-configure-my-devices-to-use-pi-hole-as-their-dns-server/245)
+6. Make sure `53/udp` is unfirewalled:
 
-8. Recommended: Configure your local machine to use the local `dnsmasq` daemon
-   for client DNS queries, i.e. software running on the same server as
-   `dnsmasq`. This is a good idea because it may save some network traffic
+```sh
+# Debian & Ubuntu, RedHat & CentOS
+sudo firewall-cmd --add-port=53/udp --permanent
+```
+
+7. Go into your router settings and change the DNS IP address provided via your
+   router's DHCP responses to be your server's (i.e. the one with You Don't
+   Need Pi Hole) local IP address.
+   [See this thread for a walkthrough.](https://discourse.pi-hole.net/t/how-do-i-configure-my-devices-to-use-pi-hole-as-their-dns-server/245)
+
+8. Recommended: If you use the You Don't Need Pi Hole machine as a general
+   computer, configure it to use the local `dnsmasq` daemon for client DNS
+   queries. This is a good idea because it may save some network traffic
    depending on how your router works. Worst case scenario, it'll do nothing.
 
-If your OS uses `dhcpcd` for network configuration, you can add this line to your
-`/etc/dhcpcd.conf`
+If your OS uses `dhcpcd` for network configuration (Slackware), you can add
+this line to your `/etc/dhcpcd.conf`:
 
 ```
 static domain_name_servers=127.0.0.1

dev-t/doc-changelog-links.t

@@ -0,0 +1,181 @@
+#!perl
+use 5.012;
+use warnings FATAL => 'all';
+
+use Test::More 'no_plan';
+
+use FindBin qw//;
+my $changelog = "$FindBin::Bin/../CHANGELOG.md";
+my $braced_inner_r = qr/(?:[^\]\\]|\\.)*/;
+use constant PROJECT_GIT => 'https://codeberg.org/h3xx/you-dont-need-pihole';
+
+SKIP: {
+    my $clh;
+    unless (open $clh, '<', $changelog) {
+        skip("failed to open changelog file to test: $!");
+    }
+    my @cl_lines = <$clh>;
+    close $clh;
+
+    ok(scalar @cl_lines,
+        'changelog contains at least one line'
+    );
+
+    my %end_links = _link_references(@cl_lines);
+    my @reflinks = _ref_links(@cl_lines);
+
+    my @unresolved = _unresolved_links(\%end_links, \@reflinks);
+    ok(!scalar @unresolved, ChangelogError->new('unresolved links', @unresolved));
+
+    my %unused_links = _unused_links(\%end_links, \@reflinks);
+    ok(!scalar %unused_links, ChangelogError->new('unused end links', values %unused_links));
+
+    my @incorrect_links = _incorrect_end_links(\%end_links, \@reflinks);
+    ok(!scalar @incorrect_links, ChangelogError->new('incorrect end links', @incorrect_links));
+
+}
+
+sub _link_references {
+    my @lines = @_;
+    my $end_link_r = qr/^\s*\[(?<key>$braced_inner_r)\]:\s*(?<url>.*?)\s*$/;
+    my %end_links;
+    my $line_number = 0;
+    foreach my $line (@lines) {
+        ++$line_number;
+        if ($line =~ $end_link_r) {
+            $end_links{$+{key}} = {
+                line_number => $line_number,
+                url => $+{url},
+                key => $+{key},
+            };
+        }
+    }
+    return %end_links;
+}
+
+sub _ref_links {
+    my @lines = @_;
+    my $ref_link_r = qr/\[(?<text>$braced_inner_r)\]\[(?<key>$braced_inner_r)\]/;
+    # Make sure not to catch:
+    # - Normal links "[text](url)"
+    # - End links "[key]: url"
+    my $bare_ref_link_r = qr/\[(?<textkey>$braced_inner_r)\](?!\s*[(:])/;
+    my @reflinks;
+    my $line_number = 0;
+    foreach my $line (@lines) {
+        ++$line_number;
+        if ($line =~ $ref_link_r) {
+            push @reflinks, {
+                line_number => $line_number,
+                text => $+{text},
+                key => $+{key},
+            };
+        } elsif ($line =~ $bare_ref_link_r) {
+            push @reflinks, {
+                line_number => $line_number,
+                text => $+{textkey},
+                key => $+{textkey},
+            };
+        }
+    }
+    return @reflinks;
+}
+
+sub _unused_links {
+    my ($end_links, $reflinks) = @_;
+    # Make a copy, lest we destroy data
+    my %unused = %{$end_links};
+    foreach my $link (@{$reflinks}) {
+        my $key = $link->{key};
+        delete $unused{$key};
+    }
+    return %unused;
+}
+
+sub _unresolved_links {
+    my ($end_links, $reflinks) = @_;
+    my @unresolved;
+    foreach my $link (@{$reflinks}) {
+        my $key = $link->{key};
+        unless (exists $end_links->{$key}) {
+            push @unresolved, $link;
+        }
+    }
+    return @unresolved;
+}
+
+sub _incorrect_end_links {
+    my ($end_links, $reflinks) = @_;
+    my @versions_in_order;
+    my %versions_seen;
+    foreach my $link (@{$reflinks}) {
+        my $key = $link->{key};
+        if (_is_version($key) && !exists $versions_seen{$key}) {
+            $versions_seen{$key} = 1;
+            push @versions_in_order, $key;
+        }
+    }
+    my %expected_links = (
+        Unreleased => _make_tag_link("v$versions_in_order[0]", 'HEAD'),
+        $versions_in_order[-1] => _make_tag_link("v$versions_in_order[-1]"),
+    );
+    foreach my $idx (0 .. ($#versions_in_order - 1)) {
+        my $this_version = $versions_in_order[$idx];
+        my $last_version = $versions_in_order[$idx + 1];
+        $expected_links{$this_version} = _make_tag_link("v$last_version", "v$this_version");
+    }
+
+    my @incorrect_links;
+    while (my ($key, $link) = each %{$end_links}) {
+        if (exists $expected_links{$key}) {
+            my $got = $link->{url};
+            my $expected = $expected_links{$key};
+            if ($got ne $expected) {
+                $link->{url_expected} = $expected;
+                push @incorrect_links, $link;
+            }
+        }
+    }
+    return @incorrect_links;
+}
+
+sub _make_tag_link {
+    my ($before_tag, $after_tag) = @_;
+    unless (defined $after_tag) {
+        return sprintf '%s/releases/tag/%s', PROJECT_GIT, $before_tag;
+    }
+    return sprintf '%s/compare/%s...%s', PROJECT_GIT, $before_tag, $after_tag;
+}
+
+sub _is_version {
+    my $ver = shift;
+    return $ver =~ /^[0-9.]+$/;
+}
+
+package ChangelogError;
+use 5.012;
+use warnings FATAL => 'all';
+use overload '""' => '_as_string';
+
+sub new {
+    my ($class, $name, @data) = @_;
+    return bless {
+        _name => $name,
+        _data => \@data,
+    }, $class;
+}
+
+sub _as_string {
+    my $self = shift;
+    my @all = ($self->{_name});
+    foreach my $datum (@{$self->{_data}}) {
+        my @out;
+        foreach my $key (qw/ line_number key text url url_expected /) {
+            if (defined $datum->{$key}) {
+                push @out, "$key: $datum->{$key}";
+            }
+        }
+        push @all, '- ' . (join ', ', @out);
+    }
+    return join "\n", @all;
+}

make-block.pl

@@ -16,6 +16,7 @@
 use 5.012;
 use warnings;
 
+use Carp qw/ croak /;
 use Getopt::Long qw/ GetOptions :config bundling no_getopt_compat no_ignore_case /;
 use FindBin qw//;
 
@@ -23,23 +24,26 @@ my %domains;
 my $dupes = 0;
 my $skip = 0;
 my $removed_allowed = 0;
-sub add_domain_list {
+
+sub _add_domain_list {
     my $file = shift;
-    foreach my $line (read_stripped($file)) {
+    foreach my $line (_read_stripped($file)) {
         my $domain = lc $line;
         if (defined $domains{$domain}) {
             ++$dupes;
         }
         $domains{$domain} = 1;
     }
+    return;
 }
 
-sub add_host_file {
+sub _add_host_file {
     my $file = shift;
-    foreach my $line (read_stripped($file)) {
+    foreach my $line (_read_stripped($file)) {
         my @parts = split /\s+/, $line;
-        die "Malformed line in $file: $line; @parts"
-            unless @parts > 1;
+        if (@parts < 2) {
+            croak("Malformed line in $file: $line; @parts");
+        }
         if (lc $parts[0] eq lc $parts[1]) {
             ++$skip;
             next;
@@ -54,20 +58,30 @@ sub add_host_file {
         }
         $domains{$domain} = 1;
     }
+    return;
 }
 
-sub read_stripped {
+sub _read_stripped {
     my $file = shift;
 
-    open my $fni, '<', $file
-        or die "Failed to open file $file for reading: $!";
-
-    map {
-        chomp;
+    my @stripped_lines;
+    my $add_stripped = sub {
+        my $line = shift;
+        chomp $line;
         # Strip whitespace and comments
-        s/^\s+|\s+$|\s*#.*$//;
-        $_ || ()
-    } <$fni>;
+        $line =~ s/^\s+|\s+$|\s*#.*$//;
+        if ($line) {
+            push @stripped_lines, $line;
+        }
+    };
+    open my $fhi, '<', $file
+        or croak("Failed to open file $file for reading: $!");
+
+    while (my $line = <$fhi>) {
+        $add_stripped->($line);
+    }
+    close $fhi;
+    return @stripped_lines;
 }
 
 MAIN: {
@@ -87,38 +101,36 @@ MAIN: {
     my @allow_lists = glob "$workdir/allowlists/*.domains";
 
     foreach my $listfile (@domain_lists) {
-        add_domain_list($listfile);
+        _add_domain_list($listfile);
     }
     foreach my $hostfile (@hosts_lists) {
-        add_host_file($hostfile);
+        _add_host_file($hostfile);
     }
 
     # Apply allowlists
     my @allow_domains;
     foreach my $allowlist (@allow_lists) {
-        push @allow_domains, read_stripped($allowlist);
+        push @allow_domains, _read_stripped($allowlist);
     }
     my $before = %domains;
     delete %domains{@allow_domains};
     # Count number removed
     $removed_allowed = $before - %domains;
 
-    my $written = 0;
+    my @block_ip = sort split /\s+/, $block_ip;
     my $fho = \*STDOUT;
     if (defined $out && length $out) {
         open $fho, '>', $out
-            or die "Failed to open file $out for writing: $!";
+            or croak("Failed to open file $out for writing: $!");
     }
-    my @block_ip = sort split /\s+/, $block_ip;
-    print $fho map {
-        ++$written;
-        my $domain = $_;
-        map {
+    foreach my $domain (sort keys %domains) {
+        print $fho map {
             "$_ $domain\n"
         } @block_ip;
-    } sort keys %domains;
+    }
+    close $fho;
 
-    printf STDERR "%d domains written to %s from\n", $written, $out // 'STDOUT';
+    printf STDERR "%d domains written to %s from\n", (scalar %domains), $out // 'STDOUT';
     printf STDERR " - %d .domains files\n", (scalar @domain_lists);
     printf STDERR " - %d .hosts files\n", (scalar @hosts_lists);
     if ($dupes) {

util/run-dev-tests.sh

@@ -0,0 +1,10 @@
+#!/bin/bash
+
+WORKDIR=${0%/*}
+BASEDIR="$WORKDIR/.."
+cd "$BASEDIR" || exit
+
+if ! prove 'dev-t'; then
+    printf 'Developer tests failed!\n' >&2
+    exit 1
+fi