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

Config: Remove more legacy fields #3817

Merged
merged 13 commits into from
Sep 19, 2024
Merged

Conversation

KobeArthurScofield
Copy link
Contributor

移除:

  • dokodemo 旧式网络配置
  • freedom 超时设置(使用 userLevel 代替)
  • SOCKS 超时设置(使用 userLevel 代替)
  • SOCKS 客户端 4/4a 支持(服务端仍支持,无需修改)

变更

  • HTTP 服务端 HTTP 头修改避免应用的兼容性问题

功能弃用提示(需在 release note 注明)
由于 Dokodemo 与 HTTP 入站的超时设置也在功能弃用中(其功能均迁移至 userLevel),但一直未在文档中移除,现加入弃用提示并在 v25 前清理代码功能,防止下游客户端向即将移除的配置项填写内容导致完全无法运行。

ProtoBreak:此 pr 需要更新所有的 pb 配置。

@KobeArthurScofield
Copy link
Contributor Author

已将 Fallback-Fallbacks 错误提示替换并且允许其一直显示到明年之前(v25 移除)

@wyx2685
Copy link
Contributor

wyx2685 commented Sep 16, 2024

@APT-ZERO 没绷住

@mmmray
Copy link
Collaborator

mmmray commented Sep 16, 2024

after re-reading APT-ZERO's edited message, I think there's a better argument to be had here. I wish that was his initial message.

I don't feel strongly about any of these features. I am not a huge fan of userLevel as a concept in general (if you want to configure something per-inbound/outbound, it just seems to get in the way), but I guess it can be used.

@KobeArthurScofield
Copy link
Contributor Author

@APT-ZERO Thanks for reaching out about the feature removal problem.

What is benefits of removing Socks4/4a Client support? will it decrease Xray filesize?

Somehow it does decrease the file size a bit. But the main reason of removal is that this feature miserably haven't occurred into the documents and only appear in the release note. It is not a day one feature, but it was added at a messy time, so not many people know that, and I only found (re-discovered) this feature when I peeked at the code. The removal is only a client-side feature, while when use as a SOCKS server the core still support SOCKS 4/4a. If there are needs of using Xray-core as a Socks 4/4a client, I may revert this part of the modification before being merged.

If there is a feature that is not useful for the majority and adds to Xray filesize, use Golang Build Tags to make it optional

Better ask maintainers.

Or what about Timeout, userLevel is not a good replacement to just set a timeout (as it also replaces other levelpolicy settings to default or must create many copies of 0 levelpolicy for just a timeout)

  1. The migration was started around six or seven years ago, conducted by the then lead developer of V2Ray, Victoria Raymond. But unluckily she was disappeared suddenly, and all migration progress was halted half way, leaving a codebase that mixed with new and legacy codes. The timeout migration to policy was also halt, all timeout config were marked with [deprecated], with some fields has been removed for some in/outbounds, while others with leftovers.
  2. Even if you have only one inbound and two outbounds, the policy with userlevel can make all timeout settings consistent between all in/outbounds, just set everything to the same level number then you can have a more consistent experience when using the core, without inconsistent configs tearing each others. And the policy object also provides some useful controls, if you don't need all of them, just config what you need. And level 0 is always the default policy, use other number instead is better.
  3. The logic between the timeout and the policy level is: the timeout only works when you have not set the userLevel. In another saying, userLevel overrides the timeout. If you set both of them simultaneously you may got messed up, and when you read your config after one year you'll still get confused. So that's why removing the old config and trying to unify them.

And transports like SplitHTTP does not even uses connIdle policy, i thought you are going to add Timeout and keepAlive option for all Inbound and Outbounds, instead you are just removing them all

Policy does not apply on transports (WS, HTTP/2, gRPC, KCP, etc.), they just apply on proxy protocols, like SOCKS, Shadowsocks, VMESS, VLESS, and even Dokodemo-door and Freedom outbound. Transports may obey the the proxy protocol they're carrying, or having their own policy (KCP and SplitHTTP/h3). For timeout and keepAlive, you can have a look on the sockopts. HTTP/2 and gRPC also have their health cheeking mechanisms, which also uses keep alive packages in another way.

@KobeArthurScofield
Copy link
Contributor Author

I have no hard feeling, even feelings about whether adding features or remove features. Je suis juste un exécuteur. Viszlát

@KobeArthurScofield KobeArthurScofield changed the title Remove legacy fields and cleanup TypeMessage Remove legacy fields, legacy code in DNS, and cleanup TypeMessage Sep 17, 2024
@KobeArthurScofield
Copy link
Contributor Author

KobeArthurScofield commented Sep 17, 2024

根据 #3821#issuecomment-2354506296, 其更改已经合并至此。

后面还有一个 router 的旧遗留代码清理,但是它的代码调用和相关 tests 比较杂,清理起来比较费时,这个是单开合并还是让这个 pr 等几天然后一起合并? @RPRX @yuhan6665

P.S.:如果有需要标记弃用计划移除的内容也可以现在提出,可以顺手将弃用提示打上

@yuhan6665
Copy link
Member

I have some concern with the pace of our deletions. But this pr looks ok to me. I will merge later if no objections

@KobeArthurScofield
Copy link
Contributor Author

If someone really needs the SOCKS 4/4a as client I can revert that part, and of course I would like to patch the document.
(Yeah, maybe Windows is the most widely used SOCKS 4/4a client in the world, but we don't remove the ability of being a SOCKS 4/4a server of the core)

But for the unfortunate single timeout config, their coexistence with policy is still embarrassing and would make things messy, like which should be overridden, etc. And for VMess, VLESS, Shadowsocks and Trojan we don't even have single timeout config in the day-one code base and even in the original V2Ray, we just have the policy. A migration would be better, and then we can make the policy config better in the future. Out time and effort are not unlimited, focus on our goal.
For the undocumented timeout config, I'm afraid that their actual use is rare today, and mainly in the configs that were from the year 2017/2018/2019.

My focus in mainly on cleaning up the legacy codes, complete the migration process that was halted, and migrate the tests that was mixed with old codes (there are a lot of them). After these works I would have rare PRs again unless there are new legacy (that should have been replaced) codes are found, the leap between v24 and v25, or when I found an unbearable bug that I can fix. Oh and don't worry, the checklist shows that the remaining cleanups won't touch the config part anymore, except the two that was announced to cease between v24 and v25.

Salute to the maintainers, contributors and project supporters, everyone works with the field of anti-censorships.

@dyhkwong
Copy link
Contributor

Why are you guys debating about non-existing things... I have to tell you that socks4(a) client in xray has never worked. See v2fly/v2ray-core#1968 and v2fly/v2ray-core#1971

@KobeArthurScofield
Copy link
Contributor Author

Why are you guys debating about non-existing things... I have to tell you that socks4(a) client in xray has never worked. See v2fly/v2ray-core#1968 and v2fly/v2ray-core#1971

Good job. Thank you. SOCKS 4/4a client just don't have a shadow in documents of XTLS. Nothing worry. No burden to remove. Yay.

@wyx2685
Copy link
Contributor

wyx2685 commented Sep 17, 2024

@APT-ZERO please stop talking s**t everyday if you want something you can be the developer for it

@RPRX
Copy link
Member

RPRX commented Sep 18, 2024

由于 Dokodemo 与 HTTP 入站的超时设置也在功能弃用中(其功能均迁移至 userLevel),但一直未在文档中移除,现加入弃用提示并在 v25 前清理代码功能,防止下游客户端向即将移除的配置项填写内容导致完全无法运行。

没看到文档中有,直接清掉不用留着报错

我想了一下这个 pr 里的都可以直接清掉不用留着报错,包括单个 fallback 从未出现在 Xray 的文档中

@RPRX
Copy link
Member

RPRX commented Sep 18, 2024

后面还有一个 router 的旧遗留代码清理,但是它的代码调用和相关 tests 比较杂,清理起来比较费时,这个是单开合并还是让这个 pr 等几天然后一起合并? @RPRX @yuhan6665

单开,快的话这个 pr 今天就能合

@KobeArthurScofield
Copy link
Contributor Author

没看到文档中有,直接清掉不用留着报错

前几天在 XTLS/Xray-docs-next#577 才清理掉的,属于遗留长尾,就是不知道会不会有不少还在用这个设置的。
当然清理了也可以。

我想了一下这个 pr 里的都可以直接清掉不用留着报错,包括单个 fallback 从未出现在 Xray 的文档中

这个可以。

@RPRX
Copy link
Member

RPRX commented Sep 18, 2024

没看到文档中有,直接清掉不用留着报错

前几天在 XTLS/Xray-docs-next#577 才清理掉的,属于遗留长尾,就是不知道会不会有不少还在用这个设置的。 当然清理了也可以。

直接删掉就好

@RPRX
Copy link
Member

RPRX commented Sep 18, 2024

不过我想了下还是明天再合然后发个版吧,今天是小日子犯下反人类罪且不打算道歉的918

@KobeArthurScofield
Copy link
Contributor Author

就绪,随时可合

@KobeArthurScofield
Copy link
Contributor Author

KobeArthurScofield commented Sep 18, 2024

不过我想了下还是明天再合然后发个版吧

结果 router 的代码整理搭了便车
量不多,好处理
九成是和剩下那个记混了

@RPRX RPRX changed the title Remove legacy fields, legacy code in DNS, and cleanup TypeMessage Config: Remove more legacy fields Sep 19, 2024
@RPRX RPRX merged commit 57a41f3 into XTLS:main Sep 19, 2024
36 checks passed
@KobeArthurScofield KobeArthurScofield deleted the config-cleanup branch September 19, 2024 04:17
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.

6 participants