"Ghost" regex blacklist entry

#1

Problem with Beta 5.0:
I managed to get a “ghost” regex blacklist entry I can’t seem to remove through normal means. I tried adding ^(.+\.)?amp\..+\.com$ with comment “Google AMP” using the web interface, and it added this to my list:

Screen Shot 2020-02-05 at 3.51.15 PM
Screen Shot 2020-02-05 at 3.51.15 PM2864×160 15.7 KB

Clicking the delete button appeared to delete the entry and showed a green success message. However, upon reloading the page the entry is still there.

I also tried removing it through the command line:

$ pihole --regex -d '()'
  [i] () does not exist in regex blacklist, no need to remove!
$ pihole --regex -d "^(.+\.)?amp\..+\.com$"
  [i] ^(.+\.)?amp\..+\.com$ does not exist in regex blacklist, no need to remove!

List of regex blacklist entries (most have been redacted - 25 is the broken entry):

$ pihole --regex -l
Displaying regex blacklist:
  1: (^|\.)datasharing\.spotify\.com$ (enabled, last modified Fri, 31 Jan 2020 18:53:01 -0500)
  ...
  25:  (enabled, last modified Wed, 05 Feb 2020 06:46:10 -0500)

Debug Token:
https://tricorder.pi-hole.net/y5tkkssvau

EDIT: I found that I was able to successfully remove it through the remove button in the Group Management > Domains page in the web interface, though there’s certainly still a bug here.

EDIT 2: Seems like I can add ^(.+\.)?amp\..+\.com$ with the command line:

$ pihole --regex '^(.+\.)?amp\..+\.com$'
  [i] Adding ^(.+\.)?amp\..+\.com$ to the regex blacklist...
  [✓] Reloading DNS lists

So I suppose this is probably a problem with the web interface.

#2

Unfortunately, I’m not in the position to do any test myself until at least Tuesday.

@DanSchaper @PromoFaux @jfb Given that the new group management pages can do more than the legacy pages, shall we remove them in favor of the former? Or do you think this is too intrusive and we should rather fix this bug?

If we get rid of the legacy pages, we should clearly rearrange the navigation menu to avoid hiding these pages.

#3

I wouldn’t get rid of the simple single task page and send people over to a more complex page, even if the more complex page is more powerful.

1 Like
#4

We can maybe separate it as, for example, two submenus:

-List management (basic)
-List Management (advanced)

2 Likes
#5

That’s good thinking right there…

#6

I should note that while I was able to remove the bugged () entry through the group management page, attempting to add ^(.+\.)?amp\..+\.com$ as a regex blacklist entry incorrectly creates a () entry through BOTH the group management and legacy page. So, simply getting rid of the legacy pages wouldn’t completely fix this bug.

1 Like
#7

I’m unable to test right now, just trying ti understand this in theory:

  1. You open the group management domain page
  2. You add ^(.+\.)?amp\..+\.com$ as regex blacklist entry

What does happen?

  1. You see two entries being added (the requested one + ()), or
  2. Only en empty group regex () ?
#9

Can confirm.

Adding ^(.+.)?amp…+.com$ in regex blacklist creates a ()

Also if you delete something from blacklist you have to go to Group Management > Domains find it and delete it from there too.

Also the same thing happens if i add for example .* that means everything.

In stable version it is working but in beta you have to write .* like this to work ^.*$

Again what ever you delete from blacklist you have to delete it from domains too

#10

I tried this now and was not able to reproduce what you described. Here is exactly what I did:

  1. Went to http://pi.hole/admin/list.php?l=black
    Screenshot from 2020-02-12 19-59-08
    Screenshot from 2020-02-12 19-59-081009×220 11 KB
  2. Put in ^(.+.)?amp…+.com$ (no comment) and clicked Add (regex)
  3. See which I was expecting
    Screenshot from 2020-02-12 20-01-26
  4. Click the Trash icon
  5. See domain got removed, checked this on the group management domain page as well
#11

The domain you attempted to add was ^(.+.)?amp…+.com$. The domain I mentioned was ^(.+\.)?amp\..+\.com$. Like you, I am unable to reproduce the issue with the former, but am still able to reproduce the issue with mine using an up-to-date installation.

And yes, only the () entry appeared when I attempted to add the latter.

#12

I think i figured out where the problem is, i modified the /var/www/html/admin/scripts/pi-hole/php/group.php and removed the idn_to_ascii function and it can now insert it just fine in the group management -> domains.

// under add_domain
From - $domain = idn_to_ascii($_POST[‘domain’]);
To - $domain = $_POST[‘domain’];

something wrong with idn_to_ascii it returns false.
maybe idn_to_ascii is not suitable for regex?

#13

Confirmed now. Not sure where I copied the wrong one from or whether my clipboard did remove that itself…

Thanks for checking this and sorry for the delay, I have been away. Yes, we’ll simply skip the IDN conversion for regex where it makes no real sense.

#14

You can try it locally using:

pihole checkout web fix/idn_regex

I’d appreciate if you could do some testing on my bugfix.

Thanks for using the Pi-hole!

1 Like
#15

I don’t think you’re going crazy, you probably copied it from @Michail_Giannopoulos’s comment.

I can now successfully add ^(.+\.)?amp\..+\.com$ as a regex from the old Blacklist and Whitelist pages. However, on the Group Management > Domains page, typing that domain and choosing type Regex Blacklist/Whitelist gives an error, and nothing is added:

Error, something went wrong!
While executing: NOT NULL constraint failed: domainlist.domain

This seems to happen with any regex input on the Group Management page, not just ^(.+\.)?amp\..+\.com$.

#16

Oh yes, sorry. When moving around the domain encoding code, I forgot to add a general rule that applies also to the regex domains. This is now fixed, please update.

2 Likes
#17

Works correctly now, thanks!

1 Like
#18

Can confirm works very well!

1 Like
#19

Please, all who helped testing, go back to the main branch to continue receiving updates. This change has been merged into the beta code.

pihole checkout web release/v5.0
2 Likes