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

support hosts #2157

Merged
merged 6 commits into from Mar 18, 2019

Conversation

@ayanamist
Copy link
Contributor

commented Mar 14, 2019

fix #2150

@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2019

In our use case, parseNumericAddress is performance critical as it needs to be invoked once for every rule in ACL. That's why we are using inet_pton and parseNumericAddress as a fallback.

You can attach some benchmark for Inetaddresses or keep that in a separate pull request.

@ayanamist

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

@Mygod 我这个是给解析hosts文件用的,不可能单独剥离出来。另外实现是从guava里抠的(代码里注释写了port from),不是我自己写的。如果你们不用,那就用另外的实现好了。
目前hosts的实现已经基本ready了,我本来是想着周末测测再推上来的,只是刚好看到你们有类似的需求,所以提前push上来。

@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2019

Use this for now:

fun String?.parseNumericAddress(): InetAddress? = Os.inet_pton(OsConstants.AF_INET, this)
?: Os.inet_pton(OsConstants.AF_INET6, this)?.let { parseNumericAddress.invoke(null, this) as InetAddress }

@ayanamist ayanamist force-pushed the ayanamist:feature-hosts branch from 9a10338 to d9663f8 Mar 15, 2019
@Mygod Mygod added the enhancement label Mar 15, 2019
@ayanamist ayanamist force-pushed the ayanamist:feature-hosts branch from d9663f8 to 85131e2 Mar 15, 2019
@ayanamist

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

Use this for now:

shadowsocks-android/core/src/main/java/com/github/shadowsocks/utils/Utils.kt

Lines 57 to 58 in 6b60d91

fun String?.parseNumericAddress(): InetAddress? = Os.inet_pton(OsConstants.AF_INET, this)
?: Os.inet_pton(OsConstants.AF_INET6, this)?.let { parseNumericAddress.invoke(null, this) as InetAddress }

已经改了。

Copy link
Contributor

left a comment

This seems to be missing STORAGE permission but it doesn't matter. Also here's why I don't support using external storage.

@ayanamist

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

This seems to be missing STORAGE permission but it doesn't matter. Also here's why I don't support using external storage.

我是看到这里 https://developer.android.com/reference/android/content/Context.html#getExternalFilesDir(java.lang.String) 提到的

Starting in Build.VERSION_CODES.KITKAT, no permissions are required to read or write to the returned path; it's always accessible to the calling app.

所以特意没有声明权限。因为这个目录在sdcard里,Q之前,可以用Solid Explorer等编辑器来直接修改这个文件,类似root后的手机直接修改/etc/hosts一样。这也是因为我没有写UI代码能力的局限之处。如果有UI了,可以不用这样做。

@ayanamist ayanamist force-pushed the ayanamist:feature-hosts branch 2 times, most recently from f32e85b to 4f9790c Mar 15, 2019
@ayanamist ayanamist force-pushed the ayanamist:feature-hosts branch from 4f9790c to b89dccd Mar 15, 2019
@madeye

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2019

Can we follow the profile import logic to import hosts file?

@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2019

I will make it a global option.

@ayanamist

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

Can we follow the profile import logic to import hosts file?

这好像是ui部分的改动?这个我无能为力了

@Mygod Mygod self-assigned this Mar 15, 2019
@Mygod Mygod changed the base branch from master to preference-1.1 Mar 15, 2019
@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2019

@ayanamist Thank you for your contribution. Please review the code. And before you ask, yes I did expect you to write these UI codes. 😄

I had to rebase this onto preference-1.1 branch (see #2162), which sadly will take a while before it can be merged, but I see you are compiling this repo by yourself so it should not be an issue for you.

I have not tested the hosts functionality yet, you can look into that. Also you are welcome to contribute a similar UI for TV (if not I will do it later).

@Mygod
Mygod approved these changes Mar 15, 2019
@Mygod Mygod requested a review from madeye Mar 15, 2019
@ayanamist

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2019

我看现在是用merge的方式把代码从preference-1.1分支合并过来,需要我用rebase的方式重新弄一下吗?另外ui部分我确实搞不定,完全没有android ui知识……

@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2019

No everything is fine now. Just give this ui a spin and see if it works properly.

@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2019

@madeye @ayanamist Please review the changes and do some testing.

@madeye

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2019

Yeah, I will check this later.

@madeye

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

Cool! Test it locally and works very well.

@madeye
madeye approved these changes Mar 18, 2019
Copy link
Contributor

left a comment

LGTM

@Mygod

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

@ayanamist Thoughts? If not, we will merge this into #2162.

@ayanamist

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2019

@ayanamist Thoughts? If not, we will merge this into #2162.

LGTM

@Mygod Mygod merged commit 76dee1c into shadowsocks:preference-1.1 Mar 18, 2019
1 of 2 checks passed
1 of 2 checks passed
Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
ci/circleci Your tests passed on CircleCI!
Details
@ayanamist ayanamist deleted the ayanamist:feature-hosts branch Jul 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.