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

Unify local DNS behavior of Cgo and pure Go resolver #15

Closed
wants to merge 2 commits into from

Conversation

dyhkwong
Copy link

@dyhkwong dyhkwong commented Apr 14, 2024

SagerNet/sing-box#1681

For Go resolver, local DNS must set a detour, its traffic will go through sing-box and destination will be incorrectly overridden. This PR make Go resolver's traffic ignore detour and not go through sing-box, just like Cgo resolver's behavior.

P.S. On Windows, Cgo resolver doesn't rely on the Cgo tool, and resolver prefers Cgo to Go, so even if compiling with CGO_ENABLED=0, sing-box will still use the Cgo resolver.

@fanyiguang
Copy link

fanyiguang commented Apr 15, 2024

直接使用context.Background()失去了上层对dns超时的控制能力,感觉与其他transport的思路冲突了。使用transport_base.go中exchange(ctx context.Context, message *dns.Msg)的方法是不是会好一点

@dyhkwong
Copy link
Author

Go resolver 不使用自定义 Dial 应已不会引起 destination 被错误覆盖的问题,已恢复原代码。

@fanyiguang
Copy link

fanyiguang commented Apr 15, 2024

Go resolver 不使用自定义 Dial 应已不会引起 destination 被错误覆盖的问题,已恢复原代码。

我这边提一嘴哈,从sing-dns是一个可独立使用的第三方出发来看,如果取消了local的自定义 Dial感觉有点扼杀了它的特性。对于sing-dns而言transport_local其实是有保持Cgo和纯 Go解析器行为一致的,是它引用的底层包没有做到这点。所以我觉得保留自定义的Dial,并参考transport_tcp.go在NewLocalTransport时保存ctx用来resolver.LookupNetIP(),至于上层传下来的ctx只用来做超时控制,这样会比较好。(没有吹毛求疵的意思,是说下自己的看法,勿怪)

@dyhkwong
Copy link
Author

dyhkwong commented Apr 15, 2024 via email

@nekohasekai nekohasekai force-pushed the dev branch 4 times, most recently from bda82fb to e88a7cb Compare June 24, 2024 04:13
@nekohasekai nekohasekai force-pushed the dev branch 4 times, most recently from cb35bb0 to f20dc0f Compare July 27, 2024 01:30
@nekohasekai nekohasekai force-pushed the dev branch 2 times, most recently from c6f02f9 to 0a21cc9 Compare November 14, 2024 09:52
@dyhkwong dyhkwong closed this Nov 18, 2024
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.

2 participants