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

Fix buffer.UDP destination override #2356

Merged
merged 7 commits into from Aug 29, 2023
Merged

Fix buffer.UDP destination override #2356

merged 7 commits into from Aug 29, 2023

Conversation

dyhkwong
Copy link
Contributor

@dyhkwong dyhkwong commented Jul 21, 2023

Previous "fix" only works for FakeDNS (QUIC sniffer not working) and breaks NAT type.

To always send IP to proxy server for UDP, you may want to implement domainStrategy for outbound.

@RPRX RPRX requested a review from yuhan6665 July 21, 2023 14:59
@RPRX
Copy link
Member

RPRX commented Aug 26, 2023

感谢 PR,或许改一下 org 名

@dyhkwong
Copy link
Contributor Author

关于 #2367,routeOnly 是为了在不覆盖目标地址的情况下将嗅探出的域名用于路由判断,但是启用 fake DNS 时,由于目标地址是 fake IP,必须覆盖目标地址才能发往正确的目标地址,不能 routeOnly。
#2367 是在修复 fakeDNS 的判断,跟(不开启 routeOnly 时的)destOverride 没什么关系。
我不知道 fakedns+others (或者 fakedns 错失后使用其他 sniffer 补救的行为)实际上是否工作以及是否有真实世界的使用者。如果是工作的并有使用者,还需要另外的修复。

@RPRX
Copy link
Member

RPRX commented Aug 27, 2023

我看了一下代码,包括 fakedns 在内的多个 sniffer 同时存在时,resComp.ProtocolForDomainResult() 会不会不是 "fakedns"?

@dyhkwong
Copy link
Contributor Author

我看了一下代码,包括 fakedns 在内的多个 sniffer 同时存在时,resComp.ProtocolForDomainResult() 会不会不是 "fakedns"?

如果 fakedns+others (或者 fakedns 错失后使用其他 sniffer 补救的行为)的功能正常运作,这是有可能的。

assuming that both "fakedns+others" and "using other sniffer when fake DNS missed" are not broken
@RPRX
Copy link
Member

RPRX commented Aug 29, 2023

我对这部分代码不熟悉,不过我觉得可以相信你的代码,让我们先把它合并,发一个 v1.8.4 让大家测试看看有没有引入新的问题

@RPRX RPRX merged commit b8bd243 into XTLS:main Aug 29, 2023
35 checks passed
@RPRX
Copy link
Member

RPRX commented Aug 29, 2023

感谢 PR

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