Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add blacklisting wildcard support #312

Merged
merged 16 commits into from
Jan 11, 2017
Merged

Add blacklisting wildcard support #312

merged 16 commits into from
Jan 11, 2017

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Dec 31, 2016

By submitting this pull request, I confirm the following (please check boxes, eg [X] - no spaces) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?: 10


Pi-hole core: pi-hole/pi-hole#1065

Add blacklisting wildcard support to the web UI.
screenshot at 2016-12-31 17-39-28

Blocked:

  • exact.de
  • wildcard.de
  • abc.wildcard.de

Not blocked:

  • abc.exact.de

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

@DL6ER DL6ER added this to the v2.3 milestone Dec 31, 2016
@DL6ER
Copy link
Member Author

DL6ER commented Dec 31, 2016

Query Log has still to be adjusted to account for the wildcard entries (they are not contained within the gravity lists).

@DL6ER
Copy link
Member Author

DL6ER commented Dec 31, 2016

screenshot at 2016-12-31 18-21-37

@DL6ER
Copy link
Member Author

DL6ER commented Dec 31, 2016

Ready for Check and Merge as of now.

Note that wildcard blocked entries do not appear in the Top Advertisers list. I'll add this later since it will depend on #297 which has to be merged first (bug fix + improvement on the algorithms).

@DL6ER DL6ER modified the milestones: v2.4, v2.3 Jan 2, 2017
Conflicts:
	scripts/pi-hole/js/queries.js
@DL6ER DL6ER changed the title Add blacklisting wildcard support [WIP] Add blacklisting wildcard support Jan 9, 2017
@DL6ER
Copy link
Member Author

DL6ER commented Jan 9, 2017

On hold since I don't know and Adam can't remember how we can effectively whitelist subdomains of wildcard blocked domains.

@DL6ER DL6ER changed the title [WIP] Add blacklisting wildcard support Add blacklisting wildcard support Jan 9, 2017
@DL6ER
Copy link
Member Author

DL6ER commented Jan 9, 2017

Disregard my comment above - let's get wildcard blocking running! We will tell the users that everything they wildcard block will be gone. Maybe there is an user who can come up with a clever idea on how to do the selective whitelisting. If not - they should not use it.

Copy link
Contributor

@AzureMarker AzureMarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks like it's working! Just a few things.

<?php if($list === "black") { ?>
<h3>Wildcard blocking</h3>
<ul class="list-group" id="list-wildcard"></ul>
<?php } ?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The headings don't show up unless the page is reloaded. Maybe dynamically change it via JS.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change between black- and whitelist without reloading the page?

@@ -136,7 +138,8 @@ $(document).ready(function() {
});
$("#all-queries tbody").on( "click", "button", function () {
var data = tableApi.row( $(this).parents("tr") ).data();
if (data[4] === "Pi-holed")
status = data[4];
if (status.substr(0,2) === "Pi")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you can't whitelist a wildcard-blocked domain, we should not show the whitelist button. Same idea on the Core repo with the block page.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, will do that, although on the main page it will be much more difficult (it does not know on loading of the page as the blocking reason comes in only later via AJAX).

Copy link
Member Author

@DL6ER DL6ER Jan 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

screenshot at 2017-01-11 11-26-54

@PromoFaux
Copy link
Member

Actually, thinking about it wildcard whitelisting should be fairly trivial, too.

Let's say you want to whitelist *.google.com, just make sure that when gravity.list is created, all entries that match that pattern are removed.

Adam can't remember how we can effectively whitelist subdomains of wildcard blocked domains.

I made a post about this, I forgot! :

https://discourse.pi-hole.net/t/how-do-i-add-wildcard-sites-to-the-blacklist/337

@AzureMarker
Copy link
Contributor

AzureMarker commented Jan 11, 2017

Approved

Approved with PullApprove

@DL6ER DL6ER merged commit c109352 into devel Jan 11, 2017
@DL6ER DL6ER deleted the wildcard_blacklisting branch January 11, 2017 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants