Skip to content

Commit dfa4603

Browse files
nekohasekainekohasekai
committed
dns: ignore split lookup errors on partial success
1 parent 250bddf commit dfa4603

File tree

2 files changed

+10
-173
lines changed

2 files changed

+10
-173
lines changed

‎dns/router.go‎

Lines changed: 10 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -380,31 +380,7 @@ type exchangeWithRulesResult struct {
380380

381381
const dnsRespondMissingResponseMessage = "respond action requires an evaluated response from a preceding evaluate action"
382382

383-
type lookupSplitHardError struct {
384-
cause error
385-
}
386-
387-
func (e *lookupSplitHardError) Error() string {
388-
return e.cause.Error()
389-
}
390-
391-
func (e *lookupSplitHardError) Unwrap() error {
392-
return e.cause
393-
}
394-
395-
func newLookupSplitHardError(err error) error {
396-
if err == nil {
397-
return nil
398-
}
399-
return &lookupSplitHardError{cause: err}
400-
}
401-
402-
func isLookupSplitHardError(err error) bool {
403-
var target *lookupSplitHardError
404-
return errors.As(err, &target)
405-
}
406-
407-
func (r *Router) exchangeWithRules(ctx context.Context, rules []adapter.DNSRule, message *mDNS.Msg, options adapter.DNSQueryOptions, allowFakeIP bool, hardFailMissingTransport bool) exchangeWithRulesResult {
383+
func (r *Router) exchangeWithRules(ctx context.Context, rules []adapter.DNSRule, message *mDNS.Msg, options adapter.DNSQueryOptions, allowFakeIP bool) exchangeWithRulesResult {
408384
metadata := adapter.ContextFrom(ctx)
409385
if metadata == nil {
410386
panic("no context")
@@ -428,11 +404,7 @@ func (r *Router) exchangeWithRules(ctx context.Context, rules []adapter.DNSRule,
428404
transport, status := r.resolveDNSRoute(action.Server, action.RuleActionDNSRouteOptions, allowFakeIP, &queryOptions)
429405
switch status {
430406
case dnsRouteStatusMissing:
431-
err := E.New("transport not found: ", action.Server)
432-
if hardFailMissingTransport {
433-
return exchangeWithRulesResult{err: newLookupSplitHardError(err)}
434-
}
435-
r.logger.ErrorContext(ctx, err)
407+
r.logger.ErrorContext(ctx, "transport not found: ", action.Server)
436408
evaluatedResponse = nil
437409
evaluatedTransport = nil
438410
continue
@@ -458,7 +430,7 @@ func (r *Router) exchangeWithRules(ctx context.Context, rules []adapter.DNSRule,
458430
case *R.RuleActionRespond:
459431
if evaluatedResponse == nil {
460432
return exchangeWithRulesResult{
461-
err: newLookupSplitHardError(E.New(dnsRespondMissingResponseMessage)),
433+
err: E.New(dnsRespondMissingResponseMessage),
462434
}
463435
}
464436
return exchangeWithRulesResult{
@@ -470,11 +442,7 @@ func (r *Router) exchangeWithRules(ctx context.Context, rules []adapter.DNSRule,
470442
transport, status := r.resolveDNSRoute(action.Server, action.RuleActionDNSRouteOptions, allowFakeIP, &queryOptions)
471443
switch status {
472444
case dnsRouteStatusMissing:
473-
err := E.New("transport not found: ", action.Server)
474-
if hardFailMissingTransport {
475-
return exchangeWithRulesResult{err: newLookupSplitHardError(err)}
476-
}
477-
r.logger.ErrorContext(ctx, err)
445+
r.logger.ErrorContext(ctx, "transport not found: ", action.Server)
478446
continue
479447
case dnsRouteStatusSkipped:
480448
continue
@@ -579,58 +547,22 @@ func (r *Router) lookupWithRules(ctx context.Context, rules []adapter.DNSRule, d
579547
return r.lookupWithRulesType(ctx, rules, domain, mDNS.TypeAAAA, lookupOptions)
580548
}
581549
var (
582-
response4 []netip.Addr
583-
response6 []netip.Addr
584-
ordinaryErr4 error
585-
ordinaryErr6 error
586-
hardErr4 error
587-
hardErr6 error
550+
response4 []netip.Addr
551+
response6 []netip.Addr
588552
)
589553
var group task.Group
590554
group.Append("exchange4", func(ctx context.Context) error {
591555
result, err := r.lookupWithRulesType(ctx, rules, domain, mDNS.TypeA, lookupOptions)
592556
response4 = result
593-
if err == nil {
594-
return nil
595-
}
596-
if E.IsClosedOrCanceled(err) {
597-
return err
598-
}
599-
if isLookupSplitHardError(err) {
600-
hardErr4 = err
601-
return nil
602-
}
603-
ordinaryErr4 = err
604-
return nil
557+
return err
605558
})
606559
group.Append("exchange6", func(ctx context.Context) error {
607560
result, err := r.lookupWithRulesType(ctx, rules, domain, mDNS.TypeAAAA, lookupOptions)
608561
response6 = result
609-
if err == nil {
610-
return nil
611-
}
612-
if E.IsClosedOrCanceled(err) {
613-
return err
614-
}
615-
if isLookupSplitHardError(err) {
616-
hardErr6 = err
617-
return nil
618-
}
619-
ordinaryErr6 = err
620-
return nil
562+
return err
621563
})
622564
err := group.Run(ctx)
623-
if err != nil {
624-
return nil, err
625-
}
626-
err = E.Errors(hardErr4, hardErr6)
627565
if len(response4) == 0 && len(response6) == 0 {
628-
if err != nil {
629-
return nil, err
630-
}
631-
return nil, E.Errors(ordinaryErr4, ordinaryErr6)
632-
}
633-
if err != nil {
634566
return nil, err
635567
}
636568
return sortAddresses(response4, response6, strategy), nil
@@ -647,7 +579,7 @@ func (r *Router) lookupWithRulesType(ctx context.Context, rules []adapter.DNSRul
647579
Qclass: mDNS.ClassINET,
648580
}},
649581
}
650-
exchangeResult := r.exchangeWithRules(withLookupQueryMetadata(ctx, qType), rules, request, options, false, true)
582+
exchangeResult := r.exchangeWithRules(withLookupQueryMetadata(ctx, qType), rules, request, options, false)
651583
if exchangeResult.rejectAction != nil {
652584
return nil, exchangeResult.rejectAction.Error(ctx)
653585
}
@@ -706,7 +638,7 @@ func (r *Router) Exchange(ctx context.Context, message *mDNS.Msg, options adapte
706638
}
707639
response, err = r.client.Exchange(ctx, transport, message, options, nil)
708640
} else if !legacyDNSMode {
709-
exchangeResult := r.exchangeWithRules(ctx, rules, message, options, true, false)
641+
exchangeResult := r.exchangeWithRules(ctx, rules, message, options, true)
710642
response, transport, err = exchangeResult.response, exchangeResult.transport, exchangeResult.err
711643
} else {
712644
var (

‎dns/router_test.go‎

Lines changed: 0 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -1855,101 +1855,6 @@ func TestLookupLegacyDNSModeDisabledAllowsPartialSuccessForExchangeFailure(t *te
18551855
require.Equal(t, []netip.Addr{netip.MustParseAddr("1.1.1.1")}, addresses)
18561856
}
18571857

1858-
func TestLookupLegacyDNSModeDisabledRespondWithoutEvaluatedResponseIsHardError(t *testing.T) {
1859-
t.Parallel()
1860-
1861-
defaultTransport := &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP}
1862-
router := newTestRouter(t, []option.DNSRule{
1863-
{
1864-
Type: C.RuleTypeDefault,
1865-
DefaultOptions: option.DefaultDNSRule{
1866-
RawDefaultDNSRule: option.RawDefaultDNSRule{
1867-
Domain: badoption.Listable[string]{"example.com"},
1868-
},
1869-
DNSRuleAction: option.DNSRuleAction{
1870-
Action: C.RuleActionTypeEvaluate,
1871-
RouteOptions: option.DNSRouteActionOptions{Server: "upstream"},
1872-
},
1873-
},
1874-
},
1875-
{
1876-
Type: C.RuleTypeDefault,
1877-
DefaultOptions: option.DefaultDNSRule{
1878-
RawDefaultDNSRule: option.RawDefaultDNSRule{
1879-
Domain: badoption.Listable[string]{"example.com"},
1880-
},
1881-
DNSRuleAction: option.DNSRuleAction{
1882-
Action: C.RuleActionTypeRespond,
1883-
},
1884-
},
1885-
},
1886-
}, &fakeDNSTransportManager{
1887-
defaultTransport: defaultTransport,
1888-
transports: map[string]adapter.DNSTransport{
1889-
"default": defaultTransport,
1890-
"upstream": &fakeDNSTransport{tag: "upstream", transportType: C.DNSTypeUDP},
1891-
},
1892-
}, &fakeDNSClient{
1893-
exchange: func(transport adapter.DNSTransport, message *mDNS.Msg) (*mDNS.Msg, error) {
1894-
require.Equal(t, "upstream", transport.Tag())
1895-
switch message.Question[0].Qtype {
1896-
case mDNS.TypeA:
1897-
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("1.1.1.1")}, 60), nil
1898-
case mDNS.TypeAAAA:
1899-
return nil, E.New("upstream exchange failed")
1900-
default:
1901-
return nil, E.New("unexpected qtype")
1902-
}
1903-
},
1904-
})
1905-
router.legacyDNSMode = false
1906-
1907-
addresses, err := router.Lookup(context.Background(), "example.com", adapter.DNSQueryOptions{})
1908-
require.Nil(t, addresses)
1909-
require.ErrorContains(t, err, dnsRespondMissingResponseMessage)
1910-
}
1911-
1912-
func TestLookupLegacyDNSModeDisabledTransportNotFoundIsHardError(t *testing.T) {
1913-
t.Parallel()
1914-
1915-
defaultTransport := &fakeDNSTransport{tag: "default", transportType: C.DNSTypeUDP}
1916-
router := newTestRouter(t, []option.DNSRule{{
1917-
Type: C.RuleTypeDefault,
1918-
DefaultOptions: option.DefaultDNSRule{
1919-
RawDefaultDNSRule: option.RawDefaultDNSRule{
1920-
Domain: badoption.Listable[string]{"example.com"},
1921-
QueryType: badoption.Listable[option.DNSQueryType]{option.DNSQueryType(mDNS.TypeAAAA)},
1922-
},
1923-
DNSRuleAction: option.DNSRuleAction{
1924-
Action: C.RuleActionTypeRoute,
1925-
RouteOptions: option.DNSRouteActionOptions{Server: "missing"},
1926-
},
1927-
},
1928-
}}, &fakeDNSTransportManager{
1929-
defaultTransport: defaultTransport,
1930-
transports: map[string]adapter.DNSTransport{
1931-
"default": defaultTransport,
1932-
},
1933-
}, &fakeDNSClient{
1934-
exchange: func(transport adapter.DNSTransport, message *mDNS.Msg) (*mDNS.Msg, error) {
1935-
require.Equal(t, "default", transport.Tag())
1936-
switch message.Question[0].Qtype {
1937-
case mDNS.TypeA:
1938-
return FixedResponse(0, message.Question[0], []netip.Addr{netip.MustParseAddr("1.1.1.1")}, 60), nil
1939-
case mDNS.TypeAAAA:
1940-
return FixedResponse(0, message.Question[0], nil, 60), nil
1941-
default:
1942-
return nil, E.New("unexpected qtype")
1943-
}
1944-
},
1945-
})
1946-
router.legacyDNSMode = false
1947-
1948-
addresses, err := router.Lookup(context.Background(), "example.com", adapter.DNSQueryOptions{})
1949-
require.Nil(t, addresses)
1950-
require.ErrorContains(t, err, "transport not found: missing")
1951-
}
1952-
19531858
func TestLookupLegacyDNSModeDisabledAllowsPartialSuccessForRcodeError(t *testing.T) {
19541859
t.Parallel()
19551860

0 commit comments

Comments
 (0)
⚡