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

feat: update bootstrap_dns_android #966

Merged
merged 29 commits into from
May 4, 2021
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
bbb1834
update with non_specified network
rurirei May 4, 2021
602e34d
remove wrongs
rurirei May 4, 2021
de31ea3
add alternative bootstrapDialer
rurirei May 4, 2021
fbdf783
do test
rurirei May 4, 2021
c7158f4
Update dns_bootstrap_android_test.go
rurirei May 4, 2021
35ffcd2
example of alternative dialer
rurirei May 4, 2021
2bf158f
public method
rurirei May 4, 2021
6dc353f
move const
rurirei May 4, 2021
60a043e
Update dns_bootstrap_android_test.go
rurirei May 4, 2021
90d5a29
no duplicated
rurirei May 4, 2021
1c12258
Rename infra/conf/dns_bootstrap_android.go to transport/internet/syst…
rurirei May 4, 2021
995f793
Update system_dns_android.go
rurirei May 4, 2021
aba9fbd
Update and rename infra/conf/dns_bootstrap_android_test.go to transpo…
rurirei May 4, 2021
88f0302
no imports
rurirei May 4, 2021
33b6d12
Update system_dns_android.go
rurirei May 4, 2021
2555128
Update system_dns_android_test.go
rurirei May 4, 2021
b98f06d
create systemDNS
rurirei May 4, 2021
066a1eb
Create system_dns_android.go
rurirei May 4, 2021
02c1267
Update system_dialer.go
rurirei May 4, 2021
528c268
call Resolver on systemDialer
rurirei May 4, 2021
4376c72
create system_dns_test.go
rurirei May 4, 2021
63a800f
resolver.LookupIP params
rurirei May 4, 2021
06129f8
param fix
rurirei May 4, 2021
56614cc
noneed TestSystemDNSResolver
rurirei May 4, 2021
f35b82c
revert: no specified resolver
rurirei May 4, 2021
00c452b
Delete system_dns.go
rurirei May 4, 2021
6033eaf
android only
rurirei May 4, 2021
b424575
android only test
rurirei May 4, 2021
f7903f2
typo
rurirei May 4, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions infra/conf/dns_bootstrap_android.go

This file was deleted.

28 changes: 0 additions & 28 deletions infra/conf/dns_bootstrap_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion transport/internet/system_dialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ func (d *DefaultSystemDialer) Dial(ctx context.Context, src net.Address, dest ne

dialer := &net.Dialer{
Timeout: time.Second * 16,
DualStack: true,
LocalAddr: resolveSrcAddr(dest.Network, src),
Resolver: NewDNSResolver(),
Copy link
Contributor

Choose a reason for hiding this comment

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

这个是一个针对 Adnroid 系统的修改,但是修改了在其他平台下的逻辑。

Copy link
Contributor

Choose a reason for hiding this comment

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

这个设计没有考虑到是使用V2Ray自己的DNS系统进行解析的可能。

}

if sockopt != nil || len(d.controllers) > 0 {
Expand Down
18 changes: 18 additions & 0 deletions transport/internet/system_dns.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package internet

import (
"context"
"net"
)

type DNSResolverFunc func() *net.Resolver

var NewDNSResolver DNSResolverFunc = func() *net.Resolver {
Copy link
Contributor

Choose a reason for hiding this comment

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

这个公开API的设计无法满足根据状态自动改变解析器,感觉未来要改。因此不应该成为一个公开的接口
这个默认的解析器修改了之前的默认的行为,不太建议。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the public number may be a temporary way to provide a configurable dns address..

Copy link
Contributor

@xiaokangwang xiaokangwang May 4, 2021

Choose a reason for hiding this comment

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

There is a best effort attempt to reduce the amount of API change. This is a API change for using V2Ray as library, the user of library can always patch V2Ray so there is no need to introduce a API that will change later.

return &net.Resolver{
PreferGo: true,
Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
var dialer net.Dialer
return dialer.DialContext(ctx, network, address)
},
}
}
21 changes: 21 additions & 0 deletions transport/internet/system_dns_android.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// +build android

package internet

import (
"context"
"net"
)

func init() {
NewDNSResolver = func() *net.Resolver {
return &net.Resolver{
PreferGo: true,
Dial: func(ctx context.Context, network, _ string) (net.Conn, error) {
const systemDNS = "8.8.8.8:53"

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you say a configurable dns address, not default "8.8.8.8:53"? this is a TODO item.

Copy link
Contributor

@CalmLong CalmLong May 4, 2021

Choose a reason for hiding this comment

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

这里有了一个不可配置的DNS服务器。这个已经被 @nekohasekai 投诉了。 还没来及改。 如果修改的话想一次改掉。

这话我就不爱听了,做周边就可以投诉,那之前安卓跑二进制遇到 DNS 解析的问题时也没见得有人就出来改一下 v2ray/v2ray-core/issues/1909

再说这个更改虽然不优雅,但是

  • 仅在安卓系统上生效
  • 不会影响 V2Ray 内置的 DNS

况且之前我的想法是从内置的 DNS 配置中取一个放到这里,要是没有配置再设置一个,但是那样很麻烦,而且硬编码也是和 @kslr 商量过的

如果说真的影响了安卓 App 的开发,获取到系统的 DNS 塞到内置的 DNS 里不就行了么,虽然还是用了 V2Ray 内置的 DNS,但是 DNS 地址还是用的系统的又有什么关系

Copy link
Contributor

@xiaokangwang xiaokangwang May 4, 2021

Choose a reason for hiding this comment

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

之前修改已经解决了不能用的问题了。现在进一步的修改应该彻底的解决掉这个问题。如果仅仅影响Android的话倒是没有问题,但是实际上这个PR还产生了对其他平台的修改。我要求修改是基于这个PR产生了对于其他的平台的修改以及新建了一个不必要的公开API,而不是因为改了Android的部分。

Copy link
Contributor

Choose a reason for hiding this comment

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

如果仅仅是更新了Android的部分的话当然是没有问题的。

Copy link
Contributor

@xiaokangwang xiaokangwang May 4, 2021

Choose a reason for hiding this comment

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

这个PR在我当时处理的时候还产生对其他平台的行为的修改。

Copy link
Contributor

Choose a reason for hiding this comment

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

这里有了一个不可配置的DNS服务器。这个已经被 @nekohasekai 投诉了。 还没来及改。 如果修改的话想一次改掉。

这话我就不爱听了,做周边就可以投诉,那之前安卓跑二进制遇到 DNS 解析的问题时也没见得有人就出来改一下 v2ray/v2ray-core/issues/1909

再说这个更改虽然不优雅,但是

* 仅在安卓系统上生效

* 不会影响 V2Ray 内置的 DNS

况且之前我的想法是从内置的 DNS 配置中取一个放到这里,要是没有配置再设置一个,但是那样很麻烦,而且硬编码也是和 @kslr 商量过的

如果说真的影响了安卓 App 的开发,获取到系统的 DNS 塞到内置的 DNS 里不就行了么,虽然还是用了 V2Ray 内置的 DNS,但是 DNS 地址还是用的系统的又有什么关系

当时那个修改没有问题,毕竟解决了不能用的问题。现在想做的是看看能不能彻底的解决这个问题。

Copy link
Contributor

Choose a reason for hiding this comment

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

如果不能长久的解决这个问题,就不应该去产生会让别人的配置炸掉的情况。我这里的这个高标准是因为这个PR在当时产生了会炸掉一些人设置的情况,所以要一次把问题解决。

Copy link
Contributor

Choose a reason for hiding this comment

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

@CalmLong 给我的投诉是说现在的思路不够好,不是说你当时不应该改。当时的修改确实解决了一个问题,而且没有产生不好的影响,因此是很好的。我是希望能找到更好的改变的方法。 如果让你产生了误会, 我对其深表歉意。

Copy link
Contributor

Choose a reason for hiding this comment

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

这里有了一个不可配置的DNS服务器。这个已经被 @nekohasekai 投诉了。 还没来及改。 如果修改的话想一次改掉。

这话我就不爱听了,做周边就可以投诉,那之前安卓跑二进制遇到 DNS 解析的问题时也没见得有人就出来改一下 v2ray/v2ray-core/issues/1909

再说这个更改虽然不优雅,但是

* 仅在安卓系统上生效

* 不会影响 V2Ray 内置的 DNS

况且之前我的想法是从内置的 DNS 配置中取一个放到这里,要是没有配置再设置一个,但是那样很麻烦,而且硬编码也是和 @kslr 商量过的

如果说真的影响了安卓 App 的开发,获取到系统的 DNS 塞到内置的 DNS 里不就行了么,虽然还是用了 V2Ray 内置的 DNS,但是 DNS 地址还是用的系统的又有什么关系

这个问题我会想办法解决的,非常抱歉。。。。

var dialer net.Dialer
return dialer.DialContext(ctx, network, systemDNS)
},
}
}
}
13 changes: 13 additions & 0 deletions transport/internet/system_dns_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package internet

import (
"context"
"testing"
)

func TestDNSResolver(t *testing.T) {
resolver := NewDNSResolver()
if ips, err := resolver.LookupIP(context.Background(), "ip", "www.google.com"); err != nil {
t.Errorf("failed to lookupIP, %v, %v", ips, err)
}
}