Skip to content

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

Merged
RPRX merged 1 commit intoXTLS:mainfrom
mmmray:sh-conditional-ok
Aug 10, 2024
Merged

SplitHTTP server: Only "ok" to older clients#3671
RPRX merged 1 commit intoXTLS:mainfrom
mmmray:sh-conditional-ok

Conversation

@mmmray
Copy link
Contributor

@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
@mmmray
Copy link
Contributor 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

leninalive pushed a commit to amnezia-vpn/amnezia-xray-core that referenced this pull request Oct 29, 2024
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