Skip to content

Conversation

@j2rong4cn
Copy link
Contributor

@RPRX
Copy link
Member

RPRX commented Apr 29, 2025

#4642 (comment)

我觉得根本解决方案应该是修改 buffer.go 的 Extend() 函数,Extend 的部分置 0,你改一下然后改一下 QUIC sniffer 的代码吧

@j2rong4cn
Copy link
Contributor Author

@RPRX 这样改会造成额外开销,感觉还是原来的代码好,只需备注一下buffer.Extend返回的扩展部分不保证全部为0x0,其他使用buffer.Extend的基本都是从下标0开始覆写数据的,就QUIC sniffer中的那一行特殊

@RPRX
Copy link
Member

RPRX commented Apr 29, 2025

这不是仅 QUIC sniffer 的问题,而是基础 API 的问题,buffer.go 应当确保它提供的 buffer 初始数据为空,任何应用层代码都不应依赖“可能从 pool 中拿到有数据的 cap”来工作,修改 New() 过于影响性能,所以修改 Extend() 和 Resize() 的代码

我单独 commit 到 main 然后你 rebase 一下吧,如果有代码依赖于反复玩弄 capacity 中的数据,则很劣质,应当让它们 broken

@j2rong4cn
Copy link
Contributor Author

这不是仅 QUIC sniffer 的问题,而是基础 API 的问题,buffer.go 应当确保它提供的 buffer 初始数据为空,任何应用层代码都不应依赖“可能从 pool 中拿到有数据的 cap”来工作,修改 New() 过于影响性能,所以修改 Extend() 和 Resize() 的代码

我单独 commit 到 main 然后你 rebase 一下吧,如果有代码依赖于反复玩弄 capacity 中的数据,则很劣质,应当让它们 broken

行吧

@Fangliding
Copy link
Member

我有点好奇直接用[]byte不行吗 为什么还要把核心的buf拉进来用 也不是什么高流量的东西(

@RPRX
Copy link
Member

RPRX commented Apr 29, 2025

2eed70e 简单来说今天是 QUIC sniffer 遇到了问题,明天就可能是其它新代码遇到了问题,总不能到处 workaround

我有点好奇直接用[]byte不行吗 为什么还要把核心的buf拉进来用 也不是什么高流量的东西(

也不是不行,可能因为 v2fly 的代码是这样写的

Golang 又不是不用内存了就直接还给系统,而是先存着,没多大开销, 并且直接 []byte 出来的一定是 all-zero

说实话我觉得如果用不到 buffer.go 的一些特性,直接 []byte 就行了

@RPRX
Copy link
Member

RPRX commented Apr 29, 2025

不过既然 buffer.go 这基础 API 已经修好了,就继续用它吧,以后搬运 QUIC sniffer 的代码也简单一些,你 rebase 一下

@RPRX
Copy link
Member

RPRX commented Apr 29, 2025

@j2rong4cn 基于这个 2eed70e rebase,正常写代码就行,无需担心 Extend() 的问题了

按你所述 v2fly 的代码有 bug,我看到你那边的 PR v2fly/v2ray-core#3389 已经把 wipeBytes() 带上了,这下以后搬运代码又复杂了

@RPRX
Copy link
Member

RPRX commented Apr 29, 2025

所以要不还是把 QUIC sniffer 代码中的 cache 改为直接 []byte 吧

@RPRX
Copy link
Member

RPRX commented Apr 29, 2025

那就先这样吧,虽然 v2fly 不修 Extend() 的问题的话可能以后搬 QUIC sniffer 的代码会 wipe 两次,所以允许你把 ddf33a9 搬过去

@j2rong4cn j2rong4cn changed the title QUIC sniffer: Fix errors caused by legacy data in the Buffer QUIC sniffer: Optimiz the code Apr 29, 2025
@RPRX RPRX changed the title QUIC sniffer: Optimiz the code QUIC sniffer: Optimize the code Apr 29, 2025
@RPRX RPRX merged commit d9ebb9b into XTLS:main Apr 29, 2025
35 checks passed
@j2rong4cn
Copy link
Contributor Author

那就先这样吧,虽然 v2fly 不修 Extend() 的问题的话可能以后搬 QUIC sniffer 的代码会 wipe 两次,所以允许你把 ddf33a9 搬过去

@j2rong4cn j2rong4cn deleted the fix-quic-sniffer branch April 29, 2025 08:55
maoxikun pushed a commit to maoxikun/Xray-core that referenced this pull request Aug 23, 2025
maoxikun pushed a commit to maoxikun/Xray-core that referenced this pull request Aug 23, 2025
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.

3 participants