-
Notifications
You must be signed in to change notification settings - Fork 0
adding function for adding custom sing-box config from user #156
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
Conversation
… URL or it already exists inside the sing-box config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds configuration parsing capabilities to support multiple server configuration formats, including JSON sing-box configs and URL-based configurations for popular proxy protocols like shadowsocks, trojan, vless, and vmess.
- Implements URL parsing for various proxy protocols (shadowsocks, trojan, vless, vmess)
- Adds
AddServerByURLmethod to the Manager for handling both JSON and URL-based configurations - Includes comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| servers/parser.go | New file implementing URL validation and parsing logic for different proxy protocols |
| servers/manager.go | Adds AddServerByURL method with telemetry support for parsing configurations |
| servers/manager_test.go | Adds test cases for the new AddServerByURL functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
servers/manager.go
Outdated
| // AddServerByURL parse a value that can be a JSON sing-box config or a base64 | ||
| // encoded config from another provider. It parses the config into a sing-box config | ||
| // and add it to the user managed group | ||
| func (m *Manager) AddServerByURL(ctx context.Context, value []byte) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have separate functions for URL formatted configs and JSON configs, the caller will know which format it is. It will simplify this function significantly.
servers/parser.go
Outdated
| return o, fmt.Errorf("couldn't parse server port: %w", err) | ||
| } | ||
|
|
||
| switch providedURL.Scheme { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are shadowsocks, trojan, vmess, and vless the only protocols we're supporting for URL config format? If so, we should return an error if the scheme is none of these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, for now from the repositories that I've been looking (mahsa configs, or v2ray-configs), those are the most common protocols that use the URL format. Anyway I'll remove from this PR the URL part for now because I'm moving the parser into a different repository
|
@garmr-ulfr I've changed the purpose of this PR so it's only related to sing-box custom config, I'll create other pull requests for the other configs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
servers/manager.go
Outdated
| // AddServerWithSingboxJSON parse a value that can be a JSON sing-box config. | ||
| // It parses the config into a sing-box config and add it to the user managed group. | ||
| func (m *Manager) AddServerWithSingboxJSON(ctx context.Context, value []byte) error { | ||
| ctx, span := otel.Tracer(tracerName).Start(ctx, "Manager.AddServerByURL") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably change Manager.AddServerByURL to Manager.AddServerWithSingboxJSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, thanks for remembering!
This PR is part of getlantern/engineering#2573 and the work will be broken into two other pull requests, one of them on another repository for parsing a plural set of configurations and another PR for integrating the URL parser on radiance