net/dns/resolver: update (*forwarder).forwardWithDestChan to always return an error unless it sends a response to responseChan

We currently have two executions paths where (*forwarder).forwardWithDestChan
returns nil, rather than an error, without sending a DNS response to responseChan.

These paths are accompanied by a comment that reads:
// Returning an error will cause an internal retry, there is
// nothing we can do if parsing failed. Just drop the packet.
But it is not (or no longer longer) accurate: returning an error from forwardWithDestChan
does not currently cause a retry.

Moreover, although these paths are currently unreachable due to implementation details,
if (*forwarder).forwardWithDestChan were to return nil without sending a response to
responseChan, it would cause a deadlock at one call site and a panic at another.

Therefore, we update (*forwarder).forwardWithDestChan to return errors in those two paths
and remove comments that were no longer accurate and misleading.

Updates #cleanup
Updates #13571

Signed-off-by: Nick Hill <mykola.khyl@gmail.com>
This commit is contained in:
Nick Hill
2024-10-04 15:11:46 -05:00
committed by Nick Khyl
parent e7545f2eac
commit c2144c44a3
3 changed files with 15 additions and 16 deletions

View File

@@ -7,7 +7,6 @@ import (
"bytes"
"context"
"encoding/binary"
"errors"
"flag"
"fmt"
"io"
@@ -657,14 +656,20 @@ func TestForwarderTCPFallbackError(t *testing.T) {
}
})
_, err := runTestQuery(t, port, request, nil)
resp, err := runTestQuery(t, port, request, nil)
if !sawRequest.Load() {
t.Error("did not see DNS request")
}
if err == nil {
t.Error("wanted error, got nil")
} else if !errors.Is(err, errServerFailure) {
t.Errorf("wanted errServerFailure, got: %v", err)
if err != nil {
t.Fatalf("wanted nil, got %v", err)
}
var parser dns.Parser
respHeader, err := parser.Start(resp)
if err != nil {
t.Fatalf("parser.Start() failed: %v", err)
}
if got, want := respHeader.RCode, dns.RCodeServerFailure; got != want {
t.Errorf("wanted %v, got %v", want, got)
}
}