Use byte regexes to fix compiltation error

[Y0ba]

Fixes error error: pattern can match invalid UTF-8 in regular expressions. It still rejects unicode characters with another error error: Unicode not allowed here but that's fine since there is no unicode characters in dns records anyway.

[zonyitoo]

I didn't notice that Regex has a byte only version. Great work!

[Y0ba]

Should I add ascii check for |asd.xyz and ||asd.xyz patterns? Currently they accept international domains.

[zonyitoo]

Good point. I think you should add it for consistency, the whole ACL file should have exactly one syntax constraint.

[zonyitoo]

Just check every bytes of each lines if they are in 0..128.

u8 has a method is_ascii.

[Y0ba]

String types have is_ascii too: https://doc.rust-lang.org/std/primitive.str.html#method.is_ascii

[Y0ba]

Done. I also factored out some code into new struct ParsingRules to reduce code duplication and size of load_from_file function.

[Y0ba]

@zonyitoo I also wonder about case sensitivity. Does ss' internal dns server lowercase dns requests? Maybe add lowercasing for |asd.xyz and ||asd.xyz rules too? It's also possible to lowercase regex-patterns and disable case insensitivity in regex engine to improve performance event further.

[zonyitoo]

Well, for socks5 and http proxy protocol, they didn't require domain names to be lowercase.

On the server side, the DNS client is using trust-dns-resolver, which I think will translate names to lowercase when sending queries (not sure).

Maybe add lowercasing for |asd.xyz and ||asd.xyz rules too? It's also possible to lowercase regex-patterns and disable case insensitivity in regex engine to improve performance event further.

Sounds good.

[zonyitoo]

Oh BTW, since ACLs are now limited to ASCII, should we convert IDNs to Punycode before checking if it matches any rules?

[dev4u]

多谢小伙伴们快速发布了代码，修复了缺陷。

[Y0ba]

I can convert |... and ||.... But with regexes it's complicated (if not impossible), for example rule 一 is xn--4gq while 一一 is xn--4gqa, thus 一+ cannot be encoded in punycode in any meaningful way.

[zonyitoo]

Should we convert domains to lowercase ASCII when handling check_target_bypassed?

[Y0ba]

DNS server always operates with ASCII. There is no need to convert anything. And if trust-dns-resolver lowercases incoming dns requests there is no need to lowercase them too. But I wonder is there a way to add auto test for that?

[Y0ba]

So, ss' DNS server already explicitly converts names to ascii. I can add here conversion to lowercase just in case.

shadowsocks-rust/crates/shadowsocks-service/src/local/dns/server.rs

Lines 313 to 323 in 731e613

	fn check_name_in_proxy_list(acl: &AccessControl, name: &Name) -> Option<bool> {
	if name.is_fqdn() {
	// remove the last dot from FQDN
	let mut name = name.to_ascii();
	name.pop();
	acl.check_host_in_proxy_list(&name)
	} else {
	// unconditionally use default for PQDNs
	Some(acl.is_default_in_proxy_list())
	}
	}

[Y0ba]

Should we convert domains to lowercase ASCII when handling check_target_bypassed?

Ooh. Now I see what did you mean. There are two places with acl checks. First is check_name_in_proxy_list and second is check_target_bypassed. check_target_bypassed isn't related to dns and gets hosts directly from connection queries. Yea, there should be both punycode and lowercase conversions.