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.

Sign up for GitHub

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

Jump to bottom

Block requests to localhost and similar, when not on localhost and similar #463

Merged
merged 1 commit into from
Sep 16, 2020
Merged

Block requests to localhost and similar, when not on localhost and similar #463

merged 1 commit into from
Sep 16, 2020

Conversation

pes10k
Copy link
Collaborator

@pes10k pes10k commented Sep 15, 2020

fixes #462

@liamengland1
Copy link

add an anchor || ?

@ryanbr
Copy link
Collaborator

ryanbr commented Sep 15, 2020

@antonok-edm uBO doesn't like ipv6, can adblock-rust work with ::1?

@pes10k pes10k force-pushed the block-requests-to-localhost branch 2 times, most recently from 13239e5 to c90cc01 Compare September 15, 2020 23:12
@liamengland1
Copy link

change ~*.local to ~local. IDK if brave's adblock engine works differently but it works in uBo/abp

@pes10k pes10k force-pushed the block-requests-to-localhost branch from c90cc01 to 5c2d4eb Compare September 15, 2020 23:30
@pes10k
Copy link
Collaborator Author

pes10k commented Sep 15, 2020

thanks @llacb47 , like i mentioned to @ryanbr, glad to have you both helping me brute force this (you can probably tell i dont write non-trivial filter list rules often ;)

ryanbr
ryanbr approved these changes Sep 15, 2020
View reviewed changes
Copy link
Collaborator

@ryanbr ryanbr left a comment

Choose a reason for hiding this comment

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

If ::1 is fine then this is okay.

antonok-edm
antonok-edm requested changes Sep 16, 2020
View reviewed changes
brave-lists/brave-specific.txt Outdated
||localhost^$third-party,domain=~127.0.0.1|~::1|~local
||127.0.0.1^$third-party,domain=~localhost|~::1|~local
||::1^$third-party,domain=~localhost|~127.0.0.1|~local
.local^$third-party,domain=~localhost|~127.0.0.1|~::1
Copy link
Collaborator

Choose a reason for hiding this comment

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

These won't work as-is, but if you change them to [::1], they do work correctly.

I'll add these as tests to adblock-rust.

@ryanbr
Copy link
Collaborator

ryanbr commented Sep 16, 2020

I would also consider changing from .local^$.. to ||local^$...

@antonok-edm
Copy link
Collaborator

antonok-edm commented Sep 16, 2020 •
edited
Loading

oh also, I don't think ~local is working correctly - see brave/adblock-rust#138

@pes10k
Copy link
Collaborator Author

pes10k commented Sep 16, 2020

okie dokie, will hold off on more changes until you report back :)

@pes10k pes10k force-pushed the block-requests-to-localhost branch 2 times, most recently from b837618 to 14a5824 Compare September 16, 2020 03:01
@pes10k
Copy link
Collaborator Author

pes10k commented Sep 16, 2020

okie as discussed, going to solve this by splitting into two PRs, this one w/o .local, and #465 which adds back in the .local addresses

@pes10k pes10k force-pushed the block-requests-to-localhost branch from 14a5824 to 37a5238 Compare September 16, 2020 03:05
@pes10k pes10k merged commit ed9420c into master Sep 16, 2020
@antonok-edm antonok-edm deleted the block-requests-to-localhost branch September 16, 2020 03:11
@pes10k pes10k mentioned this pull request Sep 16, 2020
Merged
@pes10k pes10k restored the block-requests-to-localhost branch September 16, 2020 03:21
This was referenced Sep 16, 2020
Closed
Merged
@redlumxn redlumxn mentioned this pull request Aug 18, 2022
Open
@ShivanKaul ShivanKaul mentioned this pull request Dec 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antonok-edm antonok-edm antonok-edm approved these changes

@ryanbr ryanbr ryanbr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Block connections to localhost resources from non localhost pages
4 participants
@pes10k @liamengland1 @ryanbr @antonok-edm