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

Make SplitHTTP work under CF Argo #3479

Merged
merged 2 commits into from
Jun 28, 2024
Merged

Conversation

mmmray
Copy link
Collaborator

@mmmray mmmray commented Jun 27, 2024

No description provided.

@Fangliding
Copy link
Member

It cloud work after test

@Fangliding Fangliding marked this pull request as ready for review June 28, 2024 07:30
@Fangliding Fangliding merged commit 8c44e2c into XTLS:main Jun 28, 2024
34 checks passed
@RPRX
Copy link
Member

RPRX commented Jun 28, 2024

虽然但是,能不能不要把 cloudflare 这样的字眼写在 commit message 或代码或注释中,风扇手太快了,不过 argo 还好

@RPRX
Copy link
Member

RPRX commented Jun 28, 2024

对了还有,我感觉这里过于固定的 headers 和 ok 会造成长度特征,你们用 WireShark 看一下

@RPRX
Copy link
Member

RPRX commented Jun 28, 2024

话说风扇你加个注释咋把 writer.Header().Set("Content-Type", "text/event-stream") 搞到 writer.WriteHeader(http.StatusOK) 下面了

@Fangliding
Copy link
Member

看瞎了我以为这俩是一块的
我是没什么道德包袱加注释只有一个理由就是不想让后来者感到疑惑
这个后面可以改 我觉得可以让服务器resp header允许自定义(目前三个基于http的协议header都是仅在客户端生效)

@RPRX
Copy link
Member

RPRX commented Jun 28, 2024

不是说道德包袱,毕竟 cloudflare 的梦想是接管互联网上的所有流量,warp 都让人们免费无限用了,只是搞针对有被删库的风险

既然有要改的你直接 force-push 吧,记得把 commit message 和注释改了

@RPRX
Copy link
Member

RPRX commented Jun 28, 2024

Headers 可以随便塞还好,那个 ok 的问题比较大,就算 Xray 是 flush 到 CDN 的,CDN 有可能是单独发一个包,这下强特征了

改成 ooooooooook 吧,o 数量不定,下个版本的客户端要先能解析,以后版本的服务端可能会发 @mmmray

Fangliding added a commit that referenced this pull request Jun 28, 2024
* Make SplitHTTP work under CF Argo

* Update hub.go

---------

Co-Authored-By: 风扇滑翔翼 <[email protected]>
@RPRX
Copy link
Member

RPRX commented Jun 28, 2024

@Fangliding commit message

@Fangliding
Copy link
Member

刚刚本地git出了点小问题 改完才看到

RPRX added a commit that referenced this pull request Jun 28, 2024
@RPRX
Copy link
Member

RPRX commented Jun 28, 2024

感觉还是太魔性了,@Fangliding 帮我把 commit title 改成 SplitHTTP: ok -> ooooooooook (parser in client)

@mmmray
Copy link
Collaborator Author

mmmray commented Jun 28, 2024

the ok payload comes together with response headers, in my tests it's not its own package. I think if this is now changed, it's another breaking change. I see what you did, that works.

@mmmray mmmray deleted the splithttp-sse branch June 28, 2024 09:23
@mmmray
Copy link
Collaborator Author

mmmray commented Jun 28, 2024

Also why did this PR get merged. I didn't test it at all. @Fangliding are you saying you tested it or are sure it works?

@Fangliding
Copy link
Member

Yes, I reproduced this issue yesterday and this PR can solve the problem

@mmmray
Copy link
Collaborator Author

mmmray commented Jun 28, 2024

cool, thanks!

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