search

pi-hole / web

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

UI Whitelist Add - Split on space breaks when more than one space is used #554

Closed dschaper closed 6 years ago

dschaper commented 7 years ago

@rockmasterflex69 commented on Tue Jul 25 2017

In raising this issue, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your issue:

How familiar are you with the codebase?:

1

[BUG REPORT]:

The split on space code specifically checks for exactly one space. This is silly. It might also be worth investigating if it is using an extremely general definition of whitespace for splitting, and if not, make it so.

[BUG | ISSUE] Expected Behaviour: UI shouldn't give a flying fruit loop if you use one space or two as a separator, so "xxx.yz xxx.zz" and "xxx.yz  xxx.zz" should be the same. Therefore, entering "notreal.org  notreal.com" and clicking ADD should put both of these domains in your whitelist.

[BUG | ISSUE] Actual Behaviour:

The whitelist page will throw an error: "Failure! Something went wrong. is not a valid domain.

[BUG | ISSUE] Steps to reproduce: Paste this into the window, click Add. notreal.org  notreal.com

-

-

(Optional) Debug token generated by pihole -d:

<token>

This template was created based on the work of udemy-dl.

@technicalpyro commented on Wed Jul 26 2017

this seems to be more of a feature request than a issue or bug. most users will enter the black or whitelist domains one at a time or directly modify the files on the device itself as opposed to copy paste from large lists which is where this seems to be coming from

Please submit this using the below link on the discourse forum

Feature requests are here : https://discourse.pi-hole.net/c/feature-requests

@dschaper commented on Wed Jul 26 2017

It borders on both a bug and a FR, breaking on whitespace should be in the function, I just have to find which function does that and hope it's not buried in the awk stuff.

@WaLLy3K commented on Wed Jul 26 2017

If separating domains with multiple spaces using pihole, it separates the two out as expected.

pihole -w foo.bar.com     foo.bar
::: Adding foo.bar.com to /etc/pihole/whitelist.txt...
::: foo.bar.com does not exist in /etc/pihole/blacklist.txt, no need to remove!
::: foo.bar.com does not exist in /etc/dnsmasq.d/03-pihole-wildcard.conf, no need to remove!
::: Adding foo.bar to /etc/pihole/whitelist.txt...
::: foo.bar does not exist in /etc/pihole/blacklist.txt, no need to remove!
::: foo.bar does not exist in /etc/dnsmasq.d/03-pihole-wildcard.conf, no need to remove!

If doing the same via Web Admin, the issue that O.P discovered comes into play. Thus, this is definitely a bug for AdminLTE.

@dschaper commented on Wed Jul 26 2017

Moving to Admin repo...

steffanson commented 6 years ago

Wouldn't it make sense to remove whitespaces at the beginning and end of the string? Now when I copy and paste " inspectlet.com sessionreplay.com" into the field, the domain is marked invalid due to the whitespaces, which have to be removed manually.
This would also makes sense for "Query adlists".

This FR is not only about the whitelist but also about the blacklist I guess?

AzureMarker commented 6 years ago

@steffanson I'll add a trim into the code to handle that case

jacobsalmela commented 6 years ago

Solved in v3.2