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

feat: introduce sing-box support #261

Merged
merged 1 commit into from
Apr 21, 2024
Merged

Conversation

ladit
Copy link
Contributor

@ladit ladit commented Apr 12, 2024

Please consider complementing CONTRIBUTING.md.

Copy link

netlify bot commented Apr 12, 2024

Deploy Preview for surgio-documentation ready!

Name Link
🔨 Latest commit d957f7d
🔍 Latest deploy log https://app.netlify.com/sites/surgio-documentation/deploys/662516b45cd67f0008e13efa
😎 Deploy Preview https://deploy-preview-261--surgio-documentation.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ladit ladit mentioned this pull request Apr 14, 2024
@geekdada
Copy link
Member

非常感谢贡献代码!我稍后新建一个分支线合并过去。

@ladit
Copy link
Contributor Author

ladit commented Apr 19, 2024

已经rebase到master并解决CI错误


export const Socks5NodeConfigValidator = SimpleNodeConfigValidator.extend({
export const Socks5NodeConfigValidator = TlsNodeConfigValidator.extend({
Copy link
Member

Choose a reason for hiding this comment

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

嗯不错

@@ -1064,3 +1118,21 @@ module.exports = defineClashProvider({
}
})
```

## multiplex多路复用
Copy link
Member

Choose a reason for hiding this comment

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

这个除了 Singbox 还有什么支持?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mihomo中也有使用,底层也是sing-box

Copy link
Member

Choose a reason for hiding this comment

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

OK,那我看看是不是 Clash 也可以支持一下

CONTRIBUTING.md Outdated
## test locally

- Run `pnpm run build` after modifing this repo
- Update dependencies to `"surgio": "file:path/to/surgio"` in your test repo (eg. `my-rule-store`) and run `pnpm install`
Copy link
Member

Choose a reason for hiding this comment

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

这个和 pnpm link surgio 有什么区别?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"surgio": "file:path/to/surgio"pnpm link surgio应该是等价的。
我在debug时发现需要先执行pnpm run build生成surgio/build目录,否则无法执行surgio generate,建议考虑完善CONTRIBUTING.md

Copy link
Member

Choose a reason for hiding this comment

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

我建议用 link,这样不用对代码进行修改,不会提交到仓库里

Copy link
Contributor Author

@ladit ladit Apr 21, 2024

Choose a reason for hiding this comment

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

OK,我修改了

})

export const HttpsNodeConfigValidator = TlsNodeConfigValidator.extend({
type: z.literal(NodeTypeEnum.HTTPS),
username: z.string(),
password: z.string(),
path: z.string().optional(),
headers: z.record(z.string(), z.array(z.string())).optional(),
Copy link
Member

Choose a reason for hiding this comment

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

headers 是否能先保持和之前一致,用 z.record(z.string()),不然我觉得很混乱

Copy link
Contributor Author

Choose a reason for hiding this comment

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

可以,这里我惯用go中的header结构了

Copy link
Member

Choose a reason for hiding this comment

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

对,我猜也是 go 的库造成的。咱先不考虑这个情况,实际情况下很少遇到。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经统一为z.record(z.string())

@@ -97,3 +97,17 @@ export const TlsNodeConfigValidator = SimpleNodeConfigValidator.extend({
serverCertFingerprintSha256: z.ostring(),
clientFingerprint: z.ostring(),
})

export const MultiplexValidator = z.object({
Copy link
Member

Choose a reason for hiding this comment

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

这个验证是否能直接作为 SimpleNodeConfigValidator 的一部分,这样就不用逐个写了。隐藏条件是只有 TCP 协议才生效,不过我觉得这个小坑应该不会有太大问题。你觉得呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

个人倾向于目前这样做法的原因是:

  • 喜欢组合大于继承
  • 协议变化并不多,逐个写也挺方便的😀,可以在类型系统上避免udp协议使用此字段

当然目前整体是用的继承,我觉得作为SimpleNodeConfigValidator的一部分也可以。

return result
}

const typeMap = {
Copy link
Member

Choose a reason for hiding this comment

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

这个 Map 不能用内置的么?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个map只列出了sing-box支持的协议,在这里做了不支持的协议拦截。此外socks5在sing-box中是socks。

list: ReadonlyArray<PossibleNodeConfigType>,
filter?: NodeFilterType | SortedNodeFilterType,
) {
return JSON.stringify(getSingboxNodes(list, filter)).slice(1, -1)
Copy link
Member

Choose a reason for hiding this comment

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

这个处理有点奇怪,把双引号去掉?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,具体见这里的说明。

@geekdada
Copy link
Member

我对在 Nunjucks 模板里写 JSON 有点抗拒,你还有没有更好的主意……

代码里问了几个问题,麻烦解答一下,谢谢。

@geekdada geekdada self-assigned this Apr 21, 2024
@ladit
Copy link
Contributor Author

ladit commented Apr 21, 2024

关于在Nunjucks 模板里写 JSON ,我个人是觉得OK,因为我们只是把yaml、json当作纯文本,并拼凑其中的内容。如果我们不认为这个json配置是template,那就势必要在某种程度上破坏这个项目的整体设计(provider + template -> artifact),在某些地方做特殊判断和处理。
此外,json是合格的yaml子集,{{ getClashNodes(nodeList) | json }}已经是在yaml中用Nunjucks写json了😂。

@geekdada geekdada changed the base branch from master to dev April 21, 2024 13:53
@geekdada
Copy link
Member

麻烦修复一下单测,我就合并

@ladit
Copy link
Contributor Author

ladit commented Apr 21, 2024

已经修正

@geekdada geekdada merged commit cdf49c0 into surgioproject:dev Apr 21, 2024
1 check passed
@geekdada
Copy link
Member

非常感谢,我会尽快发布新版本!

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