Skip to content

fix(config): avoid empty direct detours for DNS transports - #146

Open
LiuLin1220 wants to merge 2 commits into
hiddify:mainfrom
LiuLin1220:pr/dns-empty-direct-startup
Open

LiuLin1220 wants to merge 2 commits into
hiddify:mainfrom
LiuLin1220:pr/dns-empty-direct-startup

Conversation

@LiuLin1220

Copy link
Copy Markdown

Summary

  • stop WARP-disabled DNS transports from detouring through a semantically empty direct outbound
  • keep direct-fragment §hide§ available as a terminal route outbound
  • add a real BuildConfig -> box.New -> box.Start -> Close regression test

Root cause

The generated dns-remote-no-warp, HTTPS dns-direct, and dns-trick-direct transports set DialerOptions.Detour to the fragment direct outbound. That outbound is valid as a terminal route action, but sing-box rejects it when it is initialized as an empty direct dialer detour:

detour to an empty direct outbound makes no sense

The first commit reproduces the failure across bare, explicit UDP, and HTTPS direct DNS addresses, with DNS fragmentation both disabled and enabled.

Change

Clear those DNS detours so the transports use their own direct dialer. The change preserves:

  • the fragment direct outbound and tag used by terminal route actions
  • trick DNS TLS fragment and record fragment settings
  • the existing 300 ms trick DNS fallback delay
  • sing-box's empty-direct validation

This PR does not change regional routing, user-rule behavior, Google routing policy, WARP endpoint topology, or sing-box validation.

Verification

Passed:

go test -count=20 ./v2/config -run 'TestBuildConfigStartsWithDirectDNSTransports|TestRawDNSDetourToEmptyDirectOutboundIsRejected'
go test -count=1 ./v2/config

Red/green evidence:

  • test-only commit: all six startup cases fail with the expected empty-direct detour error
  • fix commit: the same cases pass; the negative raw-config test still proves an explicit empty direct detour is rejected

Broader baseline comparison:

  • go test -count=1 ./v2/... still reaches the pre-existing v2/profile/test.TestAddByContent failure because this build lacks with_wireguard; the same failure reproduces on db74dfc
  • go vet ./v2/config still reports the pre-existing v2/config/warp.go:212:4: unreachable code; the same diagnostic reproduces on db74dfc

No workflow, Makefile, submodule, or release-build files are changed.

@kokhlo kokhlo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified against current main (dbf3ffb, go 1.25.6 as CI pins it).

The production fix is already on main by another route. 8209feb (2026-10-02, "Refactor DNS and Hiddify options handling") added dialDetour() at v2/config/builder.go:99, which maps OutboundDirectTag/OutboundDirectFragmentTag to "" and is applied at every detour site (dns.go:420, builder.go:112 NTP, chain.go:76, warp.go:210). The per-call-site "" this PR passes in dns.go is superseded — and those call sites were rewritten again by the DNS-group work (06eaf61, 40986a1), so the dns.go hunks will not rebase cleanly.

The test file is the part that still matters. Nothing else in v2/config/*_test.go starts a real box, and main has no coverage of dialDetour at all. I dropped v2/config/dns_startup_test.go onto pristine main:

$ go test ./v2/config/ -run "TestBuildConfigStartsWithDirectDNSTransports|TestRawDNSDetourToEmptyDirectOutboundIsRejected" -count=1
ok  github.com/hiddify/hiddify-core/v2/config  0.444s

then mutated dialDetour to the identity (return tag) to confirm the test actually bites:

--- FAIL: TestBuildConfigStartsWithDirectDNSTransports (0.03s)
    dns_startup_test.go:44: start generated config: start dns/udp[dns-remote-no-warp]: detour to an empty direct outbound makes no sense
    dns_startup_test.go:44: start generated config: start dns/https[dns-direct]: detour to an empty direct outbound makes no sense

Exactly the startup failure this PR was opened for. So the suggestion: rebase keeping only v2/config/dns_startup_test.go and drop the dns.go hunks — the PR becomes the regression test for the central fix, with no behavior change to argue about.

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