search

pi-hole / web

Pi-hole Dashboard for stats and more
https://pi-hole.net
Other
1.96k stars 547 forks source link

Custom DNS does not handle multiple whitespace in custom.list #3036

Open Javex opened 1 month ago

Javex commented 1 month ago

Versions

Pi-hole version is v5.18.2 (Latest: v5.18.2) web version is v5.21 (Latest: v5.21) FTL version is v5.25.2 (Latest: v5.25.2)

Platform

Expected behavior

When adding entries to /etc/pihole/custom.list via editing the file (bypassing the Web UI) and specifying a row separated by more than one space, the Web UI should show them, parsing the file as you would /etc/hosts as that's what dnsmasq does.

Actual behavior / bug

Rows that don't exactly fit <IP><space><DNS> won't show.

Steps to reproduce

Steps to reproduce the behavior:

Put this in /etc/pihole/custom.list:

192.168.1.1 one.space.test
192.168.1.2  two.space.test

Go to /admin/dns_records.php and observe that the second row is missing from the table. If reloading pihole-FTL, note that it will still resolve, but it won't show up here.

The corresponding line is here: https://github.com/pi-hole/web/blob/master/scripts/pi-hole/php/func.php#L205

Because it splits on a single space, it does not handle this edge case.

Additional context

It could be argued that this is not a problem as users aren't supposed to write this file directly. However, it would be nice to support this use case. In my case, I manage this file via Ansible and to make it easier to read I have aligned it to look like a table:

192.168.1.1  second
192.168.1.10 first

You might not consider this a bug so feel free to label it as a feature request (if you consider it nice to have) or close it (if you explicitly don't want to support this).

If you did want to fix this, full-fledged parsing of /etc/hosts would probably be ideal, so that e.g. comments won't trip it up. A simpler implementation could just be handling multiple white space between the two "columns"

DL6ER commented 1 month ago

This is fixed in the current beta v6.0 as handing custom DNS records moved into /etc/pihole/pihole.toml with specified syntax. It'd be great if you could test if migrating with your particular file works as expected.

See https://pi-hole.net/blog/2023/10/09/pi-hole-v6-beta-testing/

Javex commented 1 month ago

Thanks for the quick reply @DL6ER! I've just set up one of my nodes to use v6 and you're right: This now works, if I add multiple spaces it handles it correctly!

github-actions[bot] commented 1 week ago

This issue is stale because it has been open 30 days with no activity. Please comment or update this issue or it will be closed in 5 days.

Javex commented 1 week ago

Do you want to close this issue since it’s fixed in v6?