feat(hy2): add Hysteria2 protocol support - #97
Merged
Conversation
added 2 commits
May 26, 2026 04:53
- Add hy2/ package with hysteria2:// URL parsing (hy2_url.go) - Implement DialHY2 using github.com/apernet/hysteria/core/v2 client - Support Salamander and Gecko obfuscation via ConnFactory wrapper - Support all standard HY2 URL parameters (sni, insecure, alpn, bandwidth, etc.) Implements Option B - direct use of hysteria/core/v2 with ~150 lines across 2 new files. No xray-core or sing-box dependency added.
- 14 test cases covering all URL parameters - Both hysteria2:// and hy2:// schemes tested - Interface method validation - Default SNI behavior confirmed
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The ALPN query parameter is parsed into HY2Config but never used when building the hysteria client.Config; either wire it into the client configuration (if supported) or remove it to avoid a misleading, no-op option.
- In obfsConnFactory.New, the addr parameter and the ServerAddr field are ignored and a UDP socket is always bound via net.ListenUDP("udp", nil); consider either using these values (e.g., for binding or dialing) or removing them to avoid unused/ignored configuration.
- HY2 URL handling is slightly inconsistent: only the "hysteria2" scheme is registered via RegisterParser (not "hy2"), Protocol() always returns "hysteria2", and Opaque() strips only the "hysteria2://" prefix; if both schemes are intended to be first-class, consider registering a parser for "hy2" and making Protocol/Opaque scheme-aware.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The ALPN query parameter is parsed into HY2Config but never used when building the hysteria client.Config; either wire it into the client configuration (if supported) or remove it to avoid a misleading, no-op option.
- In obfsConnFactory.New, the addr parameter and the ServerAddr field are ignored and a UDP socket is always bound via net.ListenUDP("udp", nil); consider either using these values (e.g., for binding or dialing) or removing them to avoid unused/ignored configuration.
- HY2 URL handling is slightly inconsistent: only the "hysteria2" scheme is registered via RegisterParser (not "hy2"), Protocol() always returns "hysteria2", and Opaque() strips only the "hysteria2://" prefix; if both schemes are intended to be first-class, consider registering a parser for "hy2" and making Protocol/Opaque scheme-aware.
## Individual Comments
### Comment 1
<location path="hy2/proxy_hy2.go" line_range="34-43" />
<code_context>
+// New creates a new obfuscated UDP connection
+func (f *obfsConnFactory) New(addr net.Addr) (net.PacketConn, error) {
+ // Create raw UDP conn - each call gets a fresh connection
+ conn, err := net.ListenUDP("udp", nil)
+ if err != nil {
+ return nil, err
+ }
+
+ var obfuscated net.PacketConn
+ switch f.ObfsType {
+ case "salamander":
+ obfuscated, err = obfs.WrapPacketConnSalamander(conn, []byte(f.ObfsPassword))
+ if err != nil {
+ return nil, err
+ }
+ case "gecko":
+ obfuscated, err = obfs.WrapPacketConnGecko(conn, obfs.GeckoOptions{
+ Password: []byte(f.ObfsPassword),
+ MinPacketSize: f.ObfsMinPacketSize,
</code_context>
<issue_to_address>
**issue (bug_risk):** UDP conn is leaked when obfuscation wrapping fails
If `WrapPacketConnSalamander` or `WrapPacketConnGecko` returns an error, the function returns without closing `conn`, leaking the UDP socket. Ensure `conn` is closed on these error paths (e.g., via a deferred close or a helper that wraps and handles cleanup).
</issue_to_address>
### Comment 2
<location path="hy2/proxy_hy2.go" line_range="165-175" />
<code_context>
+ return 0, fmt.Errorf("no number found in bandwidth value")
+ }
+
+ var multiplier uint64 = 1
+ switch {
+ case strings.HasPrefix(unit, "gbps"):
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Unknown bandwidth units fall back to raw bps silently
In `parseBandwidthValue`, if `unit` doesn’t match any known prefix, `multiplier` stays at `1`, so unknown units are interpreted as raw `bps` instead of failing. This can mask config errors (e.g., `100m` or typos). Please validate `unit` explicitly and return an error for unsupported values rather than defaulting silently.
```suggestion
var multiplier uint64
switch {
case unit == "" || strings.HasPrefix(unit, "bps"):
multiplier = 1
case strings.HasPrefix(unit, "gbps"):
multiplier = 1_000_000_000
case strings.HasPrefix(unit, "mbps"):
multiplier = 1_000_000
case strings.HasPrefix(unit, "kbps"):
multiplier = 1_000
default:
return 0, fmt.Errorf("unsupported bandwidth unit %q", unit)
}
```
</issue_to_address>
### Comment 3
<location path="hy2/hy2_url.go" line_range="26" />
<code_context>
+ Port int
+ SNI string
+ Insecure bool
+ ALPN string
+ ObfsType string // "salamander" or "gecko"
+ ObfsPassword string
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Parsed ALPN parameter is not wired into the client configuration
`alpn` is parsed into `HY2Config.ALPN` but never used when building the Hysteria2 client (e.g., not passed to `TLSConfig` / `NextProtos`), so it’s currently a no-op. Please either propagate it into the TLS/client config or remove the field and query parameter to avoid misleading behavior.
Suggested implementation:
```golang
// HY2Config stores Hysteria2 URL parameters and client TLS options (e.g., ALPN)
```
To actually wire `ALPN` into the client configuration (and make it non‑no‑op), you’ll also need to:
1. Locate the code that builds the Hysteria2 client from `HY2Config` (likely something like `NewHY2Client(config HY2Config)` or similar, possibly in another file such as `hy2/client.go` or `hy2/hy2_client.go`).
2. When constructing the `*tls.Config` for the client, set `NextProtos` from `HY2Config.ALPN`. For example (Go-style pseudocode):
```go
tlsConf := &tls.Config{
ServerName: config.SNI,
InsecureSkipVerify: config.Insecure,
// ...
}
if config.ALPN != "" {
// Support comma-separated ALPN list, e.g. "h3,h2"
alpnTokens := strings.Split(config.ALPN, ",")
for i := range alpnTokens {
alpnTokens[i] = strings.TrimSpace(alpnTokens[i])
}
tlsConf.NextProtos = alpnTokens
}
```
3. Ensure any Hysteria2 client options that accept a TLS config (or separate ALPN setting) use this `tlsConf` instance so that the ALPN negotiation is actually applied.
4. If there is a URL parser that fills `HY2Config.ALPN` from a query parameter (e.g., `?alpn=h3,h2`), keep it; otherwise, add parsing and validation there and update the documentation/README accordingly.
These changes will make the `alpn` URL parameter effective by propagating it into the TLS configuration instead of leaving it as an unused field.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
added 2 commits
May 26, 2026 07:15
- Remove unused ALPN field and query parsing (no-op option) - Fix UDP conn leak on obfuscation setup failure - Add explicit error for unknown bandwidth units - Make Protocol() scheme-aware (hysteria2 vs hy2) - Add TestHY2URLInterfaceHy2, TestHY2URLOpaqueHy2 for hy2:// scheme - Add TestParseBandwidthValue with unknown unit error cases - Remove unused ServerAddr field from obfsConnFactory - Add TestBothSchemesRegistered for both registered schemes
- Return error when obfs-min-packet-size/obfs-max-packet-size is invalid - Improve SplitHostPort: only default to port 443 when error is 'missing port' - Change unused addr param to _ in obfsConnFactory.New()
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #97 +/- ##
==========================================
+ Coverage 13.36% 16.92% +3.56%
==========================================
Files 27 29 +2
Lines 1594 1784 +190
==========================================
+ Hits 213 302 +89
- Misses 1365 1452 +87
- Partials 16 30 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add native Hysteria2 protocol support to proxyclient via direct use of
github.com/apernet/hysteria/core/v2.Changes
hy2/hy2_url.go: URL parser forhysteria2://andhy2://schemeshy2/proxy_hy2.go: DialHY2 factory using hysteria core client with obfuscation supporthy2/hy2_url_test.go: Unit tests with 14 test cases covering all parametersSupported URL Parameters
sniinsecurealpnh3)obfssalamanderorgecko)obfs-passwordobfs-min-packet-sizeobfs-max-packet-sizeup100 mbps)downfastopenExample
Dependencies
github.com/apernet/hysteria/core/v2 v2.9.2github.com/apernet/hysteria/extras/v2/obfs v2.9.2Related