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
Merged

support hosts #2157

merged 6 commits into from
Mar 18, 2019

Conversation

ayanamist
Copy link
Contributor

@ayanamist ayanamist commented Mar 14, 2019

fix #2150

@Mygod
Copy link
Contributor

Mygod 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
Copy link
Contributor Author

ayanamist commented Mar 14, 2019

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

@Mygod
Copy link
Contributor

Mygod 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
Copy link
Contributor Author

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

@Mygod Mygod left a comment

Choose a reason for hiding this comment

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

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

mobile/build.gradle Outdated Show resolved Hide resolved
@ayanamist
Copy link
Contributor Author

ayanamist 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 feature-hosts branch 2 times, most recently from f32e85b to 4f9790c Compare March 15, 2019 10:51
@madeye
Copy link
Contributor

madeye commented Mar 15, 2019

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

@Mygod
Copy link
Contributor

Mygod commented Mar 15, 2019

I will make it a global option.

@ayanamist
Copy link
Contributor Author

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 March 15, 2019 15:55
@Mygod
Copy link
Contributor

Mygod 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).

@ayanamist
Copy link
Contributor Author

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

@Mygod
Copy link
Contributor

Mygod commented Mar 16, 2019

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

@Mygod
Copy link
Contributor

Mygod commented Mar 16, 2019

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

@madeye
Copy link
Contributor

madeye commented Mar 16, 2019

Yeah, I will check this later.

@madeye
Copy link
Contributor

madeye commented Mar 18, 2019

Cool! Test it locally and works very well.

Copy link
Contributor

@madeye madeye left a comment

Choose a reason for hiding this comment

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

LGTM

@Mygod
Copy link
Contributor

Mygod commented Mar 18, 2019

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

@ayanamist
Copy link
Contributor Author

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

LGTM

@Mygod Mygod merged commit 76dee1c into shadowsocks:preference-1.1 Mar 18, 2019
@ayanamist ayanamist deleted the feature-hosts branch July 4, 2019 03:56
bannedbook pushed a commit to bannedbook/SpeedUp.VPN that referenced this pull request Dec 25, 2019
@Mygod Mygod mentioned this pull request Mar 15, 2020
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants