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

XHTTP client: Move x_padding into Referer header #4298

Merged
merged 4 commits into from
Jan 18, 2025

Conversation

rPDmYQ
Copy link
Contributor

@rPDmYQ rPDmYQ commented Jan 16, 2025

Move the padding into Referer header, to not bloat the reverse proxy access log.

From browser, the path part of Referer header can be controlled with fetch:

fetch("https://server.example.com/some_path", {referrer: "/?x_padding=000", referrerPolicy: "unsafe-url"})

The actual sent Referer header value would be http://<xray.browser.dialer>/?x_padding=000, and it won't trigger CORS preflight, even if the URL is long (no 128 length limit of "safelisted headers").

Therefore it's fully compatible with browser as dialer without overhead. The only downside would be some browsers may strip the path under privacy-protection mode (I see Firefox doing that, but it can be disabled by user for specific sites).

@Fangliding
Copy link
Member

Fangliding commented Jan 16, 2025

I think this change is acceptable (if others don't think the refer header is too odd)
But we should merge this after making browser dialer capable of processing this padding

@RPRX
Copy link
Member

RPRX commented Jan 17, 2025

我觉得不用加选项,这项改变应当逐步变成默认的,甚至可以要求服务端及时升级,v25 直接 breaking 也很合理

由于旧服务端会要求 path padding,正好新客户端连不上旧服务端,#4253#4113 (comment) 都方便了

@RPRX
Copy link
Member

RPRX commented Jan 17, 2025

就是说最近在给 stream-up 加这个 014f5e6 ,从而引入了 x_version,这里 breaking 的话就不需要它了,有点想把它删掉

还有我想要求 xPaddingBytes 不得设为负数,不确定是这个 PR 一起改了还是

@RPRX
Copy link
Member

RPRX commented Jan 17, 2025

你先这样改一下:

  1. 无需加配置,直接改成 Referer,URL 即为以前的完整 URL,新服务端兼容新旧客户端
  2. 变量名用 referer
  3. 给 browser dialer 加一下,这个其实挺简单的

@rPDmYQ rPDmYQ force-pushed the xhttp-padding-in-header branch from 1f5b32a to d4d87e7 Compare January 18, 2025 08:00
@rPDmYQ
Copy link
Contributor Author

rPDmYQ commented Jan 18, 2025

变量名用 referer

"Referer" is a misspelling of "referrer". This thing is usually referred as "referrer" in code (e.g. in fetch's parameter), and "referer" is only used when in HTTP header.

@rPDmYQ rPDmYQ changed the title XHTTP config: alternative paddingInHeader XHTTP config: move x_padding into Referer header Jan 18, 2025
@onediram
Copy link

onediram commented Jan 18, 2025 via email

// 'X' is assigned an 8 bit code, so HPACK compression won't change actual padding length on the wire.
// https://www.rfc-editor.org/rfc/rfc9204.html#section-4.1.2-2
// h3's similar QPACK feature uses the same huffman table.
header.Set("Referer", referrerHeaderPaddingPrefix+strings.Repeat("X", int(paddingLen)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great

@RPRX
Copy link
Member

RPRX commented Jan 18, 2025

感谢 @rPDmYQ 的贡献,我觉得把 x_padding 放 Referer 是一个非常好的 idea,且这个 PR 还有一些惊喜:

  1. 将 header padding 的 0 改为了 X,说明他仔细研究过 H2 等协议细节
  2. 同步修改了 packet-up 相关代码,说明他通读过 XHTTP 的代码
  3. 同步修改了 browser dialer 相关代码,我都不太想碰这部分

我打算先合并这个 PR,并发起另一个 PR 完成剩下的修改

@RPRX RPRX changed the title XHTTP config: move x_padding into Referer header XHTTP client: Move x_padding into Referer header Jan 18, 2025
@RPRX RPRX merged commit 14a6636 into XTLS:main Jan 18, 2025
35 checks passed
@RPRX
Copy link
Member

RPRX commented Jan 18, 2025

对了,你有空的话可以研究下给 browser dialer 加 stream-up,此前我的研究:#4148 (comment)

到时服务端可能得通过 Referer host 来判断要不要及时响应 stream-up 并发保活包,因为 Chrome 还不支持 bidirectional streaming

@RPRX
Copy link
Member

RPRX commented Jan 18, 2025

记录一下针对这个 PR 我想改的地方:x_padding 无需设为 const;c.Host 不一定有值;既然 path 没对上可能 query 也别要了

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.

4 participants