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

Remove remnants of global transport #3792

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

KobeArthurScofield
Copy link
Contributor

@KobeArthurScofield KobeArthurScofield commented Sep 10, 2024

Remove some remnants of #3751 . Better test this pr hard to prevent some unexpected consequences.

After this pr is merged, most of the remnants of global transport will not exist anymore. Does not affect the quacking code.

Why is this day-one remnant standing here for almost five years?

@yuhan6665 yuhan6665 merged commit 7496413 into XTLS:main Sep 11, 2024
36 checks passed
@KobeArthurScofield KobeArthurScofield deleted the rip-globaltransport branch September 12, 2024 06:01
@KobeArthurScofield
Copy link
Contributor Author

Bonus chatter:
#3751#3792 移除的代码做了考古,结论是全局传输配置应该是早期版本还没多入站多出站时的传输方式遗留演化来的。
对比一下时间之后,应该是:

  • 先有的原始全局传输相关的代码(3792 移除的内容,大概 V2Ray v1 就有了)
  • 后来到了 v3 将 transport 的 JSON 配置解析器移出来(3751 移除的 transport.go
  • 到了要升级至 v4 时 V 姐打算弃用全局传输,就标记了功能弃用提示,并且将对 transport 配置的解析换成了“检测出站和入站找不到具体 streamConfig transport 配置就将全局配置倒进去充当 streamConfig transport” 的 过渡性逻辑 (3751 移除的另外一部分代码)。因为这段过渡性修改,transport.goBuild() 函数失去引用,无法生成 TransportConfig,3792 中的代码判定永远为 false,弃用通知永远不会出现,引用的函数和变量也随之失效。
  • 然后 V 姐还没来得及移除 3792 里面的旧代码并且升级到 v5 就失踪了……至此 3792 移除的代码变成了悬案并且在没有做主动移除的分支里面都有这段几乎失效的代码,一直到最近。(没错,v2fly 那边 v5 的配置已经抛弃了全局传输配置,但是还有这段失效代码)

这些代码可能只有在阅读时或者搜索一部分关键词(historical,compatibility,deprecate 之类的)才可能被发现,而且埋藏比较深。估计有可能要地毯式扫描所有代码才能发现有哪些是已经失效或者弃用的变量、结构、函数之类,而且实际数量可能不少。

Bonus bonus chatter:
如果移除 globalTransportConfigCreatorCache 以及相关逻辑,会导致 #3244 的问题,导致所有本来可以省略用默认配置的地方都要手动写完整配置,比如被迫填写:

    "streamSettings": {
        "network": "tcp",
        "tcpSettings": {
            "header": {
                "type": "none"
            },
            "acceptProxyProtocol": false
        }
    }

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