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

SplitHTTP server: Only "ok" to older clients #3671

Merged
merged 1 commit into from
Aug 10, 2024

Conversation

mmmray
Copy link
Collaborator

@mmmray mmmray commented Aug 10, 2024

#3643 (comment)

It makes sense. In earlier discussions of breaking changes, we assumed the server updates much faster than the client. But this is easy to do.

Note: responseFlusher.Flush() could also be conditional. It would mean that HTTP response headers are only written when the first VLESS response byte is written. For some CDN this is already the case. I am not sure what is preferred from a padding perspective. I guess technically response X-Padding can be removed if responseFlusher.Flush() is removed, and VLESS brings its own padding in the future. Anyway, I don't want to change it right now, because I am not sure if it triggers different timeout settings in the CDN. It can be changed later, it's not a breaking change.

@RPRX
Copy link
Member

RPRX commented Aug 10, 2024

VLESS padding 永远无法取代 GET response 的 header padding,因为不能保证经过 middlebox 后它们仍然黏在一起

@RPRX
Copy link
Member

RPRX commented Aug 10, 2024

话说这个 PR 要不要改成检测到没有任何 querystring 才发 ok

@RPRX
Copy link
Member

RPRX commented Aug 10, 2024

话说这个 PR 要不要改成检测到没有任何 querystring 才发 ok

想了下还是只检测 x_padding 吧

@RPRX RPRX merged commit 513182a into XTLS:main Aug 10, 2024
36 checks passed
@mmmray
Copy link
Collaborator Author

mmmray commented Aug 11, 2024

By the way, should this PR be changed to detect that there is no querystring before sending it? ok?

there are some people who randomly add ?ed=1234 to the path, so I don't want to break those configs 😬

(by the way, why is ed a querystring? i never understood)

@Fangliding
Copy link
Member

(by the way, why is ed a querystring? i never understood)

Because /ed1024/xxx or xxx/ed1024 doesn't seem like a good choice
As for why it's not made into a wsSetting, because this way the GUI client doesn't need any changes

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