c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(nextlite): wrap DNSDecoder errors (#962)

Browse Source 
The simplest fix is to wrap such errors in dnsdecoder.go.

Fixes https://github.com/ooni/probe/issues/2317.
This commit is contained in:
Simone Basso 2022-09-14 13:47:20 +02:00 committed by GitHub
parent 550b602a00
commit 7ee9f096d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 65 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
internal/netxlite/dnsdecoder.go
View File

@ -23,17 +23,22 @@ var (
ErrDNSIsQuery = errors.New("ooresolver: expected response but received query")
)
// dnsDecoderWrapError ensures we wrap the returned errors
func dnsDecoderWrapError(err error) error {
return MaybeNewErrWrapper(ClassifyResolverError, ResolveOperation, err)
}
// DecodeResponse implements model.DNSDecoder.DecodeResponse.
func (d *DNSDecoderMiekg) DecodeResponse(data []byte, query model.DNSQuery) (model.DNSResponse, error) {
reply := &dns.Msg{}
if err := reply.Unpack(data); err != nil {
return nil, err
return nil, dnsDecoderWrapError(err)
}
if !reply.Response {
return nil, ErrDNSIsQuery
return nil, dnsDecoderWrapError(ErrDNSIsQuery)
}
if reply.Id != query.ID() {
return nil, ErrDNSReplyWithWrongQueryID
return nil, dnsDecoderWrapError(ErrDNSReplyWithWrongQueryID)
}
resp := &dnsResponse{
bytes: data,
@ -77,20 +82,20 @@ func (r *dnsResponse) rcodeToError() error {
case dns.RcodeSuccess:
return nil
case dns.RcodeNameError:
return ErrOODNSNoSuchHost
return dnsDecoderWrapError(ErrOODNSNoSuchHost)
case dns.RcodeRefused:
return ErrOODNSRefused
return dnsDecoderWrapError(ErrOODNSRefused)
case dns.RcodeServerFailure:
return ErrOODNSServfail
return dnsDecoderWrapError(ErrOODNSServfail)
default:
return ErrOODNSMisbehaving
return dnsDecoderWrapError(ErrOODNSMisbehaving)
}
}
// DecodeHTTPS implements model.DNSResponse.DecodeHTTPS.
func (r *dnsResponse) DecodeHTTPS() (*model.HTTPSSvc, error) {
if err := r.rcodeToError(); err != nil {
return nil, err
return nil, err // error already wrapped
}
out := &model.HTTPSSvc{
ALPN: []string{}, // ensure it's not nil
@ -117,7 +122,7 @@ func (r *dnsResponse) DecodeHTTPS() (*model.HTTPSSvc, error) {
}
}
if len(out.IPv4) <= 0 && len(out.IPv6) <= 0 {
return nil, ErrOODNSNoAnswer
return nil, dnsDecoderWrapError(ErrOODNSNoAnswer)
}
return out, nil
}
@ -125,7 +130,7 @@ func (r *dnsResponse) DecodeHTTPS() (*model.HTTPSSvc, error) {
// DecodeLookupHost implements model.DNSResponse.DecodeLookupHost.
func (r *dnsResponse) DecodeLookupHost() ([]string, error) {
if err := r.rcodeToError(); err != nil {
return nil, err
return nil, err // error already wrapped
}
var addrs []string
for _, answer := range r.msg.Answer {
@ -143,7 +148,7 @@ func (r *dnsResponse) DecodeLookupHost() ([]string, error) {
}
}
if len(addrs) <= 0 {
return nil, ErrOODNSNoAnswer
return nil, dnsDecoderWrapError(ErrOODNSNoAnswer)
}
return addrs, nil
}
@ -151,7 +156,7 @@ func (r *dnsResponse) DecodeLookupHost() ([]string, error) {
// DecodeNS implements model.DNSResponse.DecodeNS.
func (r *dnsResponse) DecodeNS() ([]*net.NS, error) {
if err := r.rcodeToError(); err != nil {
return nil, err
return nil, err // error already wrapped
}
out := []*net.NS{}
for _, answer := range r.msg.Answer {
@ -161,7 +166,7 @@ func (r *dnsResponse) DecodeNS() ([]*net.NS, error) {
}
}
if len(out) < 1 {
return nil, ErrOODNSNoAnswer
return nil, dnsDecoderWrapError(ErrOODNSNoAnswer)
}
return out, nil
}
@ -169,7 +174,7 @@ func (r *dnsResponse) DecodeNS() ([]*net.NS, error) {
// DecodeCNAME implements model.DNSResponse.DecodeCNAME.
func (r *dnsResponse) DecodeCNAME() (string, error) {
if err := r.rcodeToError(); err != nil {
return "", err
return "", err // error already wrapped
}
for _, answer := range r.msg.Answer {
switch avalue := answer.(type) {
@ -177,7 +182,7 @@ func (r *dnsResponse) DecodeCNAME() (string, error) {
return avalue.Target, nil
}
}
return "", ErrOODNSNoAnswer
return "", dnsDecoderWrapError(ErrOODNSNoAnswer)
}
var _ model.DNSDecoder = &DNSDecoderMiekg{}

46
internal/netxlite/dnsdecoder_test.go
View File

@ -12,14 +12,22 @@ import (
"github.com/ooni/probe-cli/v3/internal/runtimex"
)
func dnsDecoderErrorIsWrapped(err error) bool {
var errwrapper *ErrWrapper
return errors.As(err, &errwrapper)
}
func TestDNSDecoderMiekg(t *testing.T) {
t.Run("DecodeResponse", func(t *testing.T) {
t.Run("UnpackError", func(t *testing.T) {
d := &DNSDecoderMiekg{}
resp, err := d.DecodeResponse(nil, &mocks.DNSQuery{})
if err == nil || err.Error() != "dns: overflow unpacking uint16" {
if err == nil || err.Error() != "unknown_failure: dns: overflow unpacking uint16" {
t.Fatal("unexpected error", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if resp != nil {
t.Fatal("expected nil resp here")
}
@ -33,6 +41,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrDNSIsQuery) {
t.Fatal("unexpected err", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if resp != nil {
t.Fatal("expected nil resp here")
}
@ -53,6 +64,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrDNSReplyWithWrongQueryID) {
t.Fatal("unexpected error", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if resp != nil {
t.Fatal("expected nil resp here")
}
@ -163,6 +177,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, io.err) {
t.Fatal("unexpected err", err)
}
if err != nil && !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
})
}
})
@ -187,6 +204,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrOODNSRefused) {
t.Fatal("unexpected err", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if https != nil {
t.Fatal("expected nil https result")
}
@ -210,6 +230,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrOODNSNoAnswer) {
t.Fatal("unexpected err", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if https != nil {
t.Fatal("expected nil https results")
}
@ -268,6 +291,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrOODNSRefused) {
t.Fatal("unexpected err", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if len(ns) > 0 {
t.Fatal("expected empty ns result")
}
@ -291,6 +317,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrOODNSNoAnswer) {
t.Fatal("unexpected err", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if len(ns) > 0 {
t.Fatal("expected empty ns results")
}
@ -343,6 +372,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrOODNSRefused) {
t.Fatal("unexpected err", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if len(addrs) > 0 {
t.Fatal("expected empty addrs result")
}
@ -366,6 +398,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrOODNSNoAnswer) {
t.Fatal("unexpected err", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if len(addrs) > 0 {
t.Fatal("expected empty ns results")
}
@ -456,6 +491,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrOODNSNoAnswer) {
t.Fatal("not the error we expected", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if len(addrs) > 0 {
t.Fatal("expected no addrs here")
}
@ -482,6 +520,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrOODNSNoAnswer) {
t.Fatal("not the error we expected", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if len(addrs) > 0 {
t.Fatal("expected no addrs here")
}
@ -532,6 +573,9 @@ func TestDNSDecoderMiekg(t *testing.T) {
if !errors.Is(err, ErrOODNSNoAnswer) {
t.Fatal("unexpected err", err)
}
if !dnsDecoderErrorIsWrapped(err) {
t.Fatal("unwrapped error", err)
}
if cname != "" {
t.Fatal("expected empty cname result")
}