Repository navigation
feat(fmt): import Hysteria 2 JSON configs (parseHysteria2Json) - #92
Conversation
HysteriaFmt's JSON import only handled HY1 - parseHysteria1Json hard-coded
protocolVersion=1 with a 'TODO parse HY2 JSON+YAML'. Standalone HY2 JSON configs (a common
distribution form) fell through to the HY1 parser and imported wrong.
- Add parseHysteria2Json mapping the official HY2 client config schema (server, auth, tls
{sni,insecure}, obfs {salamander|gecko}, bandwidth {up,down}) to HysteriaBean, mirroring
parseHysteria2(url).
- RawUpdater.parseJSON: add an HY2 detection branch (server && auth && (tls||obfs||bandwidth))
ahead of the HY1 branch; HY1 detection (up/up_mbps) is unchanged.
- Robolectric tests for core fields + salamander + gecko obfs (org.json needs Robolectric).
- Clash/Mihomo YAML HY2 already flows through the Clash branch, so no duplication.
Spike findings recorded in plans/022-hysteria2-json-import.md.
📝 WalkthroughWalkthroughAdds Hysteria 2 JSON parsing for client configs, routes matching subscription JSON objects to the new parser in ChangesHysteria 2 JSON import
Sequence Diagram(s)sequenceDiagram
participant RawUpdater
participant JSONObject
participant HysteriaBean
RawUpdater->>JSONObject: parseJSON matches HY2 server/auth object
JSONObject->>HysteriaBean: parseHysteria2Json() populates protocolVersion, server, auth, tls, obfs, bandwidth
HysteriaBean-->>RawUpdater: return parsed bean
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![greptile-apps[bot]](/s/com/githubusercontent/avatars/G.https/in/867647?s=60&v=4){kind=small}
| /** Read a HY2 bandwidth value as Mbps: a bare int, or a unit-suffixed string (bps/kbps/mbps/gbps/tbps). */ | ||
| private fun parseBandwidthMbps(obj: JSONObject, key: String): Int? { | ||
| obj.getIntNya(key)?.let { return it } | ||
| val raw = obj.getStr(key)?.trim()?.lowercase() ?: return null | ||
| val match = Regex("""^(\d+(?:\.\d+)?)\s*([a-z]*)$""").matchEntire(raw) ?: return null |
There was a problem hiding this comment.
Regex(...) is compiled into a new Pattern object on every invocation of parseBandwidthMbps. Because this function is called twice per bean (once for "up", once for "down"), every import re-compiles the same pattern. Lifting it to a top-level val compiles it once at class-load time.
| /** Read a HY2 bandwidth value as Mbps: a bare int, or a unit-suffixed string (bps/kbps/mbps/gbps/tbps). */ | |
| private fun parseBandwidthMbps(obj: JSONObject, key: String): Int? { | |
| obj.getIntNya(key)?.let { return it } | |
| val raw = obj.getStr(key)?.trim()?.lowercase() ?: return null | |
| val match = Regex("""^(\d+(?:\.\d+)?)\s*([a-z]*)$""").matchEntire(raw) ?: return null | |
| private val BANDWIDTH_REGEX = Regex("""^(\d+(?:\.\d+)?)\s*([a-z]*)$""") | |
| /** Read a HY2 bandwidth value as Mbps: a bare int, or a unit-suffixed string (bps/kbps/mbps/gbps/tbps). */ | |
| private fun parseBandwidthMbps(obj: JSONObject, key: String): Int? { | |
| obj.getIntNya(key)?.let { return it } | |
| val raw = obj.getStr(key)?.trim()?.lowercase() ?: return null | |
| val match = BANDWIDTH_REGEX.matchEntire(raw) ?: return null |
| json.has("server") && json.getStr("auth") != null && | ||
| !json.has("up") && !json.has("up_mbps") -> { |
There was a problem hiding this comment.
auth string silently falls through HY2 detection
getStr("auth") (from Formats.kt) returns null for both absent keys and blank string values. So a well-formed HY2 config that carries "auth": "" (or any whitespace-only value) will fail this check, skip the HY2 branch, and be silently dropped since no later branch matches it either. An empty auth would fail server-side anyway, but the failure mode here is a quiet no-op import rather than a visible error. Consider using json.has("auth") if the intent is purely structural detection, or adding a comment clarifying that blank auth is intentionally excluded.
| fun parseHysteria2Json_bareHostDefaultsPort443() { | ||
| val bean = JSONObject("""{ "server": "example.com", "auth": "a" }""").parseHysteria2Json() | ||
| assertEquals("example.com", bean.serverAddress) | ||
| assertEquals("443", bean.serverPorts) | ||
| } | ||
|
|
There was a problem hiding this comment.
The bareHostDefaultsPort443 test exercises the no-port branch of the IPv6-safe split, but there is no test for "server": "[::1]:8443" — the case that is most likely to regress, since it requires the code to find the colon after the closing bracket (]) rather than one of the colons inside the address literal. Adding a single assertion for this form would lock in the bracket-aware split behaviour.
![coderabbitai[bot]](/s/com/githubusercontent/avatars/G.https/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/io/nekohasekai/sagernet/fmt/hysteria/HysteriaFmt.kt`:
- Around line 301-309: The bandwidth parsing in HysteriaFmt.parse rate
conversion is truncating sub-Mbps values to 0 because mbps.toInt() floors
fractional results, which can unintentionally turn an explicit cap into
unlimited. Update the conversion logic in the mbps calculation path to round or
otherwise clamp any positive parsed value to at least 1 before returning it, and
keep the return value as a valid non-zero limit for small rates. Also review the
final coerceAtLeast(0) in this code path since the regex only accepts digits and
makes negative handling unnecessary.
- Around line 243-255: The server parsing logic in HysteriaFmt.kt still splits
bare IPv6 literals on the last colon, so Hysteria2 parsing treats an address
like 2001:db8::1 as host plus port. Update the server handling in the relevant
parsing method to detect unbracketed IPv6 literals before the last-colon port
split, and only extract a port when the input is a true host:port form. Keep the
existing bracketed IPv6 path via wrapIPV6Host() unchanged, and ensure
serverAddress/serverPorts are set correctly for bare IPv6, IPv4, and hostname
inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e940a0c8-fc8d-4526-9590-cb9157ded8ac
📒 Files selected for processing (3)
app/src/main/java/io/nekohasekai/sagernet/fmt/hysteria/HysteriaFmt.ktapp/src/main/java/io/nekohasekai/sagernet/group/RawUpdater.ktapp/src/test/java/io/nekohasekai/sagernet/fmt/HysteriaFmtTest.kt
| val server = optString("server") | ||
| // Only split off a port when there's an explicit one. A bare host or an IPv6 literal | ||
| // without a port must keep the whole string as the address (default port 443), | ||
| // matching parseHysteria2(url)'s HttpUrl behavior. | ||
| val lastColon = server.lastIndexOf(':') | ||
| val portPart = if (lastColon >= 0) server.substring(lastColon + 1) else "" | ||
| if (lastColon >= 0 && portPart.toIntOrNull() != null && !server.endsWith("]")) { | ||
| serverAddress = server.substring(0, lastColon) | ||
| serverPorts = portPart | ||
| } else { | ||
| serverAddress = server | ||
| serverPorts = "443" | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'fun\s+wrapIPV6Host' --type=ktRepository: hawkff/NekoBoxForAndroid
Length of output: 189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the IPv6 wrapper and the Hysteria formatter code paths.
rg -n -C 3 'wrapIPV6Host|parseHysteria2|displayAddress\(' app/src/main/java
# Read the relevant file around the cited lines.
wc -l app/src/main/java/io/nekohasekai/sagernet/fmt/hysteria/HysteriaFmt.kt
sed -n '1,340p' app/src/main/java/io/nekohasekai/sagernet/fmt/hysteria/HysteriaFmt.ktRepository: hawkff/NekoBoxForAndroid
Length of output: 31695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the helper definition if it lives elsewhere.
rg -n -C 4 'fun\s+wrapIPV6Host|wrapIPV6Host\(' app/src/main/java --glob '*.kt'Repository: hawkff/NekoBoxForAndroid
Length of output: 3431
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the IPv6 wrapper and the Hysteria formatter code paths.
rg -n -C 3 'wrapIPV6Host|parseHysteria2|displayAddress\(' app/src/main/java --glob '*.kt'
# Read the relevant file around the cited lines.
wc -l app/src/main/java/io/nekohasekai/sagernet/fmt/hysteria/HysteriaFmt.kt
sed -n '1,340p' app/src/main/java/io/nekohasekai/sagernet/fmt/hysteria/HysteriaFmt.ktRepository: hawkff/NekoBoxForAndroid
Length of output: 27786
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the helper definition if it lives elsewhere.
rg -n -C 4 'fun\s+wrapIPV6Host|wrapIPV6Host\(' app/src/main/java --glob '*.kt'Repository: hawkff/NekoBoxForAndroid
Length of output: 3431
Handle bare IPv6 literals before splitting on :. 2001:db8::1 currently becomes serverAddress = "2001:db8:" and serverPorts = "1". The bracketed case is already safe because wrapIPV6Host() unwraps then re-wraps.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/io/nekohasekai/sagernet/fmt/hysteria/HysteriaFmt.kt` around
lines 243 - 255, The server parsing logic in HysteriaFmt.kt still splits bare
IPv6 literals on the last colon, so Hysteria2 parsing treats an address like
2001:db8::1 as host plus port. Update the server handling in the relevant
parsing method to detect unbracketed IPv6 literals before the last-colon port
split, and only extract a port when the input is a true host:port form. Keep the
existing bracketed IPv6 path via wrapIPV6Host() unchanged, and ensure
serverAddress/serverPorts are set correctly for bare IPv6, IPv4, and hostname
inputs.
| val mbps = when (match.groupValues[2]) { | ||
| "", "mbps", "m" -> value | ||
| "bps", "b" -> value / 1_000_000.0 | ||
| "kbps", "k" -> value / 1_000.0 | ||
| "gbps", "g" -> value * 1_000.0 | ||
| "tbps", "t" -> value * 1_000_000.0 | ||
| else -> return null | ||
| } | ||
| return mbps.toInt().coerceAtLeast(0) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Sub-Mbps bandwidth truncates to 0.
mbps.toInt() floors fractional results, so values below 1 Mbps (e.g. "500 kbps" → 0.5 → 0, "800 bps" → 0) collapse to 0. Since HY2 treats 0 as unlimited, a small explicit cap silently becomes uncapped. Consider rounding and/or clamping positive results to at least 1.
Also note the regex only matches \d+, so a negative value is impossible and coerceAtLeast(0) is effectively dead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/io/nekohasekai/sagernet/fmt/hysteria/HysteriaFmt.kt` around
lines 301 - 309, The bandwidth parsing in HysteriaFmt.parse rate conversion is
truncating sub-Mbps values to 0 because mbps.toInt() floors fractional results,
which can unintentionally turn an explicit cap into unlimited. Update the
conversion logic in the mbps calculation path to round or otherwise clamp any
positive parsed value to at least 1 before returning it, and keep the return
value as a valid non-zero limit for small rates. Also review the final
coerceAtLeast(0) in this code path since the regex only accepts digits and makes
negative handling unnecessary.
1 participant
Summary
HysteriaFmt's JSON import only handled HY1 (
parseHysteria1Jsonhard-codedprotocolVersion=1with a "TODO parse HY2 JSON+YAML"). Standalone HY2 JSON configs (a common distribution form, and a marketed headline feature) fell through to the HY1 parser and imported wrong. (Plan 022 spike → implemented.)Changes
parseHysteria2Json: maps the official HY2 client config schema → HysteriaBean (server w/ default port 443 + IPv6-safe split, string auth, tls {sni,insecure}, obfs {salamander|gecko}, bandwidth {up,down} accepting bare ints or unit-suffixed strings like "100 mbps"/"1 gbps").RawUpdater.parseJSON: HY2 detection =server + string auth + !(up||up_mbps)ahead of the HY1 branch (HY1 detection unchanged).@Config(sdk=[33], application=Application::class)).Testing
testOssDebugUnitTestgreen on Depot CI. CodeRabbit clean (3 review findings fixed: IPv6/default-port, bandwidth string units, relaxed discriminator). Spike findings recorded in the plan file.Greptile Summary
This PR adds
parseHysteria2Jsonto import standalone HY2 JSON client configs that were previously falling through to the HY1 parser and being mis-imported. A new discriminator branch inRawUpdater.parseJSONdetects HY2 configs by the presence ofserver+ stringauthand the absence of HY1-onlyup/up_mbpskeys.parseHysteria2Jsonmaps the official HY2 schema (server with IPv6-safe port splitting, string auth,tls, salamander/geckoobfs,bandwidthaccepting bare ints or unit-suffixed strings) intoHysteriaBeanwithprotocolVersion = 2.parseBandwidthMbpsnormalizes bandwidth strings (\"100 mbps\",\"1 gbps\", bare ints) to integer Mbps, clamped to ≥ 0.Confidence Score: 4/5
Safe to merge; the new parser and discriminator correctly handle the documented HY2 schema, with only minor polish items remaining.
The implementation is well-scoped and the IPv6/port logic, obfs dispatch, and bandwidth normalisation are all correct for the inputs the tests cover. The three findings are non-blocking: a regex compiled on every call, a silent import failure for the unlikely case of a blank auth field, and a missing test for an IPv6-with-port server string that the code already handles correctly.
The discriminator in RawUpdater.kt and the getStr blank-string behaviour are worth a second look together, but neither requires a fix before merging.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[parseJSON receives JSONObject] --> B{has 'server' AND\nhas 'up' OR 'up_mbps'?} B -- Yes --> C[parseHysteria1Json\nprotocolVersion=1] B -- No --> D{has 'server' AND\ngetStr 'auth' != null AND\nno 'up' / 'up_mbps'?} D -- Yes --> E[parseHysteria2Json\nprotocolVersion=2] E --> F[Parse server:\nIPv6-safe lastColon split\ndefault port 443] F --> G[auth → TYPE_STRING\nauthPayload] G --> H[tls block:\nsni / allowInsecure] H --> I{obfs type?} I -- salamander --> J[OBFS_SALAMANDER\nset obfuscation] I -- gecko --> K[OBFS_GECKO\npassword + min/max pkt size] I -- else/none --> L[OBFS_NONE] J & K & L --> M[bandwidth block:\nparseBandwidthMbps\nbps/kbps/mbps/gbps/tbps → Mbps int] D -- No --> N[Continue to other\nprotocol branches\nSSR / SS / TrojanGo / ...]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[parseJSON receives JSONObject] --> B{has 'server' AND\nhas 'up' OR 'up_mbps'?} B -- Yes --> C[parseHysteria1Json\nprotocolVersion=1] B -- No --> D{has 'server' AND\ngetStr 'auth' != null AND\nno 'up' / 'up_mbps'?} D -- Yes --> E[parseHysteria2Json\nprotocolVersion=2] E --> F[Parse server:\nIPv6-safe lastColon split\ndefault port 443] F --> G[auth → TYPE_STRING\nauthPayload] G --> H[tls block:\nsni / allowInsecure] H --> I{obfs type?} I -- salamander --> J[OBFS_SALAMANDER\nset obfuscation] I -- gecko --> K[OBFS_GECKO\npassword + min/max pkt size] I -- else/none --> L[OBFS_NONE] J & K & L --> M[bandwidth block:\nparseBandwidthMbps\nbps/kbps/mbps/gbps/tbps → Mbps int] D -- No --> N[Continue to other\nprotocol branches\nSSR / SS / TrojanGo / ...]Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile