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

bugfix: acl rule pattern match error for tree rule #1254

Merged
merged 1 commit into from
Jul 30, 2023

Conversation

chuxi
Copy link
Contributor

@chuxi chuxi commented Jul 30, 2023

the generated acl file by running genacl_proxy_gfw_bypass_china_ip.py does not correctly match the pattern in src/acl/mod.rs.

the old version will use heavy regex set rules and it will compile all patterns when the first time used. Instead of using tree subdomain methods.

Actually, most rules are generated for subdomain tree. By removing the last duplicated \$, we can make it right.

If it is not right or has some problem, please comment here.

@zonyitoo
Copy link
Collaborator

You shouldn't just remove the last \$ instead of making it optional: \$?

@zonyitoo
Copy link
Collaborator

zonyitoo commented Jul 30, 2023

Hmm, I think it is just a special style only in https://raw.githubusercontent.com/NateScarlet/gfwlist.acl/master/gfwlist.acl.json . He omitted the tailing $ for simple domain rules, but kept the $ for complex rules:

\\.google\\.ws
^([^/]+\\.)*google\\.(ac|ad|ae|af|ai|al|am|as|at|az)$

It should be a special case, because if he wrote the first rule as ^([^/]+\\.)*\\.google\\.ws$, then it could also be handled as a tree rule.

@chuxi
Copy link
Contributor Author

chuxi commented Jul 30, 2023

I had tried the two solution: removing \$ or updating \$?.

And the source file https://github.com/shadowsocks/shadowsocks-libev/blob/master/acl/gfwlist.acl it has the ending $. shadowsocks-rust does not have the ending $ for acl rules.

So to keep compatibility, I update it again to use \$?.

@chuxi
Copy link
Contributor Author

chuxi commented Jul 30, 2023

Hmm, I think it is just a special style only in https://raw.githubusercontent.com/NateScarlet/gfwlist.acl/master/gfwlist.acl.json . He omitted the tailing $ for simple domain rules, but kept the $ for complex rules:

\\.google\\.ws
^([^/]+\\.)*google\\.(ac|ad|ae|af|ai|al|am|as|at|az)$

It should be a special case, because if he written the first rule as ^([^/+\\.)*\\.google\\.ws$, then it could also be handled as a tree rule.

Yes. I see. So I updated again.

@zonyitoo
Copy link
Collaborator

Let me think twice..

@chuxi
Copy link
Contributor Author

chuxi commented Jul 30, 2023

Let me think twice..

If you decided, just tell me here...

I can accept both solutions...

@zonyitoo zonyitoo merged commit acc7768 into shadowsocks:master Jul 30, 2023
6 checks passed
@chuxi chuxi deleted the bugfix-acl-1 branch July 30, 2023 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants