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

📃 Exclude @cn from PAC proxied list #2982

Merged
merged 1 commit into from
Oct 11, 2020

Conversation

database64128
Copy link
Contributor

@database64128 database64128 commented Oct 11, 2020

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • Searched for similar pull requests

  • Compiled the code with Visual Studio

  • Require translation update

  • Require document update (readme.md, wikipage, etc)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New feature

Description of your pull request and other information

- PAC proxied list only contains domain names from geolocation-!cn without a `cn` attribute
Copy link
Collaborator

@chenshaoju chenshaoju left a comment

Choose a reason for hiding this comment

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

The quick test is passed, base Shadowsocks-4.2.0.40-Release.exe, the domain like blizzard.cn etc, has not through the proxy.

But some domains like qualcomm.cn, dynacw.com.cn, verisign.com.cn, microsoft.com etc, still in the list.

I'm curious about these domains.

Anyway, 👍 Looks good to me, but someone else must approve.

@database64128
Copy link
Contributor Author

But some domains like qualcomm.cn, dynacw.com.cn, verisign.com.cn, microsoft.com etc, still in the list.

The domains you mentioned don't have the cn attribute. So it's the expected behavior that they are included in the proxied list. And they actually resolves to non-CN IP addresses. So that's not a problem at all.

@chenshaoju
Copy link
Collaborator

image

Odd... But anyway this is Geosite issue. 😅

Copy link
Collaborator

@Stzx Stzx left a comment

Choose a reason for hiding this comment

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

In fact, for the problems in the third party, I personally think it should be solved only upstream. But when the upstream processing has not yet resulted, I think we can apply some temporary fixes.

@database64128
Copy link
Contributor Author

In fact, for the problems in the third party, I personally think it should be solved only upstream. But when the upstream processing has not yet resulted, I think we can apply some temporary fixes.

See v2fly/domain-list-community#235 (comment).

In a nutshell, geolocation-!cn is just a list of non-CN entities' domain names. It is supposed to contain non-CN entities' Chinese domain names. The cn attribute is the upstream's answer to our use case, where we only want non-CN domain names in the list, and don't want Chinese domain names.

@database64128
Copy link
Contributor Author

However, I question the use of domain-based methods to achieve this goal.

Domain-based methods are all our infrastructure can support right now.

This causes trouble for some major services with CDN support

This PR addresses that issue by excluding the cn attribute, which marks domain names that have Chinese CDNs.

it is better providing an access-list based method as default and give users a choice to opt for the experimental Geo-based method.

gfwlist has been poorly maintained for a long time. And in my personal experience, I ended up maintaining a long list of user rules. With geosite, the problem is mostly mitigated, because most commonly known sites are included. And anyone can contribute and they process PRs fast.

A more realistic method would be the use of Geo-Location IP profiles.

Once again, anyone is more than welcome to work with us to add routing.

@database64128
Copy link
Contributor Author

The 'cn' attribute is incomplete, even for the domains of some well-known major service providers like Microsoft.

It's actually quite complete for most major service providers.

gfwlist is not coming back, period.

@database64128
Copy link
Contributor Author

@ziruizhuang Thank you for your insightful comments! I made some improvements in #2990. Please check it out. Feel free to give more feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants