Fix Mihomo xHTTP subscription options#509
Open
Rerowros wants to merge 2 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughNormalizes xHTTP parsing and host defaults, adds Mihomo/Clash Meta xhttp-opts serialization (including download/reuse settings and advanced fields), hardens host/port selection, guards download-settings processing, updates models to accept numeric inputs, and adds comprehensive tests. ChangesxHTTP Subscription Enhancement
Sequence DiagramsequenceDiagram
participant Client as Xray JSON (subscription)
participant Parser as XRayConfig._handle_xhttp_settings
participant Inbound as Parsed SubscriptionInboundData
participant Hosts as app.core.hosts (host builder)
participant Clash as ClashConfiguration / Mihomo serializer
Client->>Parser: xhttpSettings (top-level + extra)
Parser->>Inbound: map fields, http_headers, download_settings
Inbound->>Hosts: apply inbound defaults (no_grpc_header, xmux, download_settings)
Hosts->>Clash: provide normalized transport_config and download_settings
Clash->>Clash: build xhttp-opts (padding/session/uplink/reuse/download-settings)
Clash->>Output: serialized Mihomo / Clash Meta host entry
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_subscription_clash_xhttp.py (1)
133-235: ⚡ Quick winAdd a regression test for inbound-default
download_settingsfallback.Current tests don’t cover the path where xHTTP
downloadSettingscomes from parsed core inbound defaults (without host-levelxhttp_settings.download_settings). A small case here would guard the fallback contract end-to-end.🤖 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 `@tests/test_subscription_clash_xhttp.py` around lines 133 - 235, Add a regression test that verifies xHTTP downloadSettings provided via an inbound-level/default location are picked up when host-level xhttp_settings.download_settings is absent: create a new test (similar to test_xray_parser_reads_xhttp_extra_advanced_fields) that feeds XRayConfig input where "downloadSettings" appears only in the inbound's streamSettings/xhttpSettings (or core inbound defaults) and assert parsed.inbounds_by_tag[...] produces the expected "download_settings" dict; also add an integration-like assertion that ClashMetaConfiguration (meta.add and the resulting meta.data["proxies"][0]["xhttp-opts"]["download-settings"]) uses those fallback values when serializing. Ensure the test references XRayConfig, parsed.inbounds_by_tag, ClashMetaConfiguration and meta.add so reviewers can locate the behavior under test.
🤖 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/core/hosts.py`:
- Around line 264-267: The download_settings field is being set to None unless
both xs and down_settings exist, which discards core-parsed xHTTP defaults;
change the assignment so it uses down_settings when available (xs and
down_settings) otherwise falls back to inbound_config.get("download_settings")
(and only None if neither exists). Update the download_settings expression near
where xmux and http_headers are set (referencing xs, down_settings,
inbound_config, host) to: download_settings = down_settings if xs and
down_settings else inbound_config.get("download_settings").
---
Nitpick comments:
In `@tests/test_subscription_clash_xhttp.py`:
- Around line 133-235: Add a regression test that verifies xHTTP
downloadSettings provided via an inbound-level/default location are picked up
when host-level xhttp_settings.download_settings is absent: create a new test
(similar to test_xray_parser_reads_xhttp_extra_advanced_fields) that feeds
XRayConfig input where "downloadSettings" appears only in the inbound's
streamSettings/xhttpSettings (or core inbound defaults) and assert
parsed.inbounds_by_tag[...] produces the expected "download_settings" dict; also
add an integration-like assertion that ClashMetaConfiguration (meta.add and the
resulting meta.data["proxies"][0]["xhttp-opts"]["download-settings"]) uses those
fallback values when serializing. Ensure the test references XRayConfig,
parsed.inbounds_by_tag, ClashMetaConfiguration and meta.add so reviewers can
locate the behavior under test.
🪄 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: a4d78da4-ac40-49c4-b046-d24de3cbdbb5
📒 Files selected for processing (5)
app/core/hosts.pyapp/core/xray.pyapp/subscription/clash.pyapp/subscription/xray.pytests/test_subscription_clash_xhttp.py
There was a problem hiding this comment.
Pull request overview
This PR fixes Mihomo/Clash Meta subscription generation for xHTTP by ensuring xHTTP inbounds are no longer skipped, and by serializing Mihomo-specific xhttp-opts (including advanced fields, XMUX→reuse-settings, and a normalized download-settings shape). It also extends Xray core config parsing to capture additional xhttpSettings.extra fields and adds coverage for these behaviors.
Changes:
- Enable xHTTP proxy generation for
ClashMetaConfigurationwhile keeping classic Clash skipping behavior. - Expand Mihomo xHTTP serialization (
xhttp-opts) to include advanced options, XMUX mapping (reuse-settings), and a safer Mihomo-shapeddownload-settings. - Extend Xray core parsing + subscription serialization to carry more xHTTP
extrafields and accept dict-shaped download settings; add focused tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_subscription_clash_xhttp.py |
Adds tests ensuring xHTTP is generated for Clash Meta and advanced xhttp-opts/download-settings are serialized correctly. |
app/subscription/xray.py |
Allows xHTTP downloadSettings to pass through when already provided as a dict. |
app/subscription/clash.py |
Implements Mihomo-specific xHTTP option serialization, XMUX→reuse-settings mapping, and Clash Meta xHTTP support. |
app/core/xray.py |
Parses additional xHTTP extra fields (headers, noGRPCHeader, sc*, xmux, downloadSettings) into inbound settings. |
app/core/hosts.py |
Adjusts merge behavior for xHTTP advanced defaults (no_grpc_header/sc*) and xmux/http_headers sourcing. |
Comments suppressed due to low confidence (1)
app/subscription/clash.py:547
- On invalid string ports,
_select_portreturns0, which will emit an invalid port in generated configs (and is hard to debug). Prefer returningNone(so it can be omitted) or raising/propagating an error instead of silently producing port 0.
def _select_port(port: int | str | list[int] | list[str] | None) -> int | None:
"""Normalize port values from subscription data."""
if port is None:
return None
if isinstance(port, list):
if not port:
return None
port = port[0]
if isinstance(port, str):
try:
return int(port)
except ValueError:
return 0
return port
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rerowros
force-pushed
the
fix/mihomo-xhttp-options
branch
from
eecac35 to
2c829b1
Compare
Rerowros
force-pushed
the
fix/mihomo-xhttp-options
branch
from
2c829b1 to
018c3bf
Compare
Contributor
Author
|
Concrete upstream references for the xHTTP field mapping in this PR: Mihomo client config shapeMihomo now documents
Relevant Mihomo source at
Relevant Mihomo PR history:
Xray / XHTTP source field namesPasarGuard stores/parses Xray-style camelCase fields. Xray's
Mapping used by this PRThis PR converts Xray/PasarGuard camelCase settings to Mihomo kebab-case settings:
This is why the PR keeps the existing basic xHTTP support from #347 and only fills in the missing advanced Mihomo fields plus safer normalization for nested |
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
xhttp-optsxmuxvalues to Mihomoreuse-settingsdownload-settingswith a Mihomo-specific shape instead of raw internal/Xray dataxhttpSettings.extrafieldsFixes #508
Tests
uv run ruff check app/core/hosts.py app/core/xray.py app/subscription/clash.py app/subscription/xray.py tests/test_subscription_clash_xhttp.pyuv run pytest tests/test_subscription_clash_xhttp.pyuv run pytest tests/api/test_core.py::test_xray_auto_detects_fallback_tls_without_manual_fallback_tagsuv run python -m py_compile app/core/hosts.py app/core/xray.py app/subscription/clash.py app/subscription/xray.py tests/test_subscription_clash_xhttp.pygit diff --checkSummary by CodeRabbit
Improvements
Tests