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

Sniff: Prevent crash on QUIC sniffer panic #3978

Merged
merged 2 commits into from
Nov 11, 2024
Merged

Sniff: Prevent crash on QUIC sniffer panic #3978

merged 2 commits into from
Nov 11, 2024

Conversation

Fangliding
Copy link
Member

这个copy来的QUIC sniffer有很小的概率会slice err 以后找时间研究下 现在没办法就跳过这次嗅探了 至少别崩溃

@RPRX
Copy link
Member

RPRX commented Nov 7, 2024

所以 #3972 是第几个包出的问题

@Fangliding
Copy link
Member Author

idk (( 那天晚上看了一下直接睡觉了 用python写个小程序往任意门发个一两mb大的文件就复现了

@RPRX
Copy link
Member

RPRX commented Nov 7, 2024

@Fangliding 加个 log 看看是不是固定位置

@Fangliding
Copy link
Member Author

Fangliding commented Nov 7, 2024

说了是固定位置呀

@RPRX
Copy link
Member

RPRX commented Nov 7, 2024

@Fangliding 看看是不是固定的几号包,比如调小 post size,出问题的序号不变

@Fangliding
Copy link
Member Author

我也不知道几号包里装的啥 看的转发来的数据

@RPRX
Copy link
Member

RPRX commented Nov 7, 2024

@Fangliding 加个 log 就知道是几号包了,看是客户端还是服务端的问题

@Fangliding
Copy link
Member Author

Fangliding commented Nov 7, 2024

客户端问题 客户端第二个包到第三个包中间丢了一些字节 以此类推

@RPRX
Copy link
Member

RPRX commented Nov 7, 2024

@Fangliding 这有点怪了,如果丢字节的话代理 TLS 时上行很容易 bad record,这次确定不是你那边用于测试的代码的问题吗

@Fangliding
Copy link
Member Author

我把client里要发的payload摘出来写入文件对比的 发现和收到的错误文件吻合 只有大于1MB的单次请求会有问题 所以普通的打开请求不会有问题 但是如果测上行就炸了 说通俗点就是小包不丢大包丢 speedtest什么的测出零点几一点几不是它的速度问题 而是测一会就断了导致的

@RPRX
Copy link
Member

RPRX commented Nov 7, 2024

我把client里要发的payload摘出来写入文件对比的

在哪

@Fangliding
Copy link
Member Author

SendUploadRequest() 里

@RPRX
Copy link
Member

RPRX commented Nov 7, 2024

我在看代码,没设 DiscardOverflow,可能 pipe 那里有 bug,还没找到,不过这 WithSizeLimit 到处都在用,应该没 bug 吧

@RPRX
Copy link
Member

RPRX commented Nov 7, 2024

但是这个 payload 又是直接从 pipe 里 read 出来的,或者是 buf.MultiBufferContainer 的问题,你看下前面的 chunk 有没有问题

@RPRX
Copy link
Member

RPRX commented Nov 9, 2024

你看下前面的 chunk 有没有问题

所以变量 chunk 有问题吗

@Fangliding
Copy link
Member Author

不会了(
而且这个和这个pr没关系

@RPRX
Copy link
Member

RPRX commented Nov 9, 2024

不会了(

SendUploadRequest() 的调用方传入的是 chunk,你把它摘出来看看有没有问题

而且这个和这个pr没关系

没事

@RPRX
Copy link
Member

RPRX commented Nov 11, 2024

这个 pr 的代码也需 fmt

@RPRX RPRX mentioned this pull request Nov 11, 2024
4 tasks
@RPRX RPRX merged commit 1ffb8a9 into main Nov 11, 2024
36 checks passed
@RPRX RPRX deleted the QUIC-Sniffer branch November 11, 2024 04:23
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