diff --git a/internal/engine/internal/sessionresolver/clientmaker.go b/internal/engine/internal/sessionresolver/clientmaker.go index b2ce862..abc07d0 100644 --- a/internal/engine/internal/sessionresolver/clientmaker.go +++ b/internal/engine/internal/sessionresolver/clientmaker.go @@ -1,5 +1,9 @@ package sessionresolver +// +// Code for mocking the creation of a client. +// + import ( "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/model" diff --git a/internal/engine/internal/sessionresolver/codec.go b/internal/engine/internal/sessionresolver/codec.go deleted file mode 100644 index b73f080..0000000 --- a/internal/engine/internal/sessionresolver/codec.go +++ /dev/null @@ -1,35 +0,0 @@ -package sessionresolver - -import ( - "encoding/json" -) - -// codec is the codec we use. -type codec interface { - // Encode encodes v as a stream of bytes. - Encode(v interface{}) ([]byte, error) - - // Decode decodes b into a stream of bytes. - Decode(b []byte, v interface{}) error -} - -// getCodec always returns a valid codec. -func (r *Resolver) getCodec() codec { - if r.codec != nil { - return r.codec - } - return &defaultCodec{} -} - -// defaultCodec is the default codec. -type defaultCodec struct{} - -// Decode decodes b into v using the default codec. -func (*defaultCodec) Decode(b []byte, v interface{}) error { - return json.Unmarshal(b, v) -} - -// Encode encodes v using the default codec. -func (*defaultCodec) Encode(v interface{}) ([]byte, error) { - return json.Marshal(v) -} diff --git a/internal/engine/internal/sessionresolver/doc.go b/internal/engine/internal/sessionresolver/doc.go new file mode 100644 index 0000000..266312b --- /dev/null +++ b/internal/engine/internal/sessionresolver/doc.go @@ -0,0 +1,24 @@ +// Package sessionresolver contains the resolver used by the session. This +// resolver will try to figure out which is the best service for running +// domain name resolutions and will consistently use it. +// +// Occasionally this code will also swap the best resolver with other +// ~good resolvers to give them a chance to perform. +// +// The penalty/reward mechanism is strongly derivative, so the code should +// adapt ~quickly to changing network conditions. Occasionally, we will +// have longer resolutions when trying out other resolvers. +// +// At the beginning we randomize the known resolvers so that we do not +// have any preferential ordering. The initial resolutions may be slower +// if there are many issues with resolvers. +// +// The system resolver is given the lowest priority at the beginning +// but it will of course be the most popular resolver if anything else +// is failing us. (We will still occasionally probe for other working +// resolvers and increase their score on success.) +// +// We also support a socks5 proxy. When such a proxy is configured, +// the code WILL skip http3 resolvers AS WELL AS the system +// resolver, in an attempt to avoid leaking your queries. +package sessionresolver diff --git a/internal/engine/internal/sessionresolver/errwrapper.go b/internal/engine/internal/sessionresolver/errwrapper.go index d343854..8f43a66 100644 --- a/internal/engine/internal/sessionresolver/errwrapper.go +++ b/internal/engine/internal/sessionresolver/errwrapper.go @@ -1,23 +1,40 @@ package sessionresolver +// +// Error wrapping +// + import ( "errors" "fmt" ) -// errwrapper wraps an error to include the URL of the +// errWrapper wraps an error to include the URL of the // resolver that we're currently using. -type errwrapper struct { - error - URL string +type errWrapper struct { + err error + url string +} + +// newErrWrapper creates a new err wrapper. +func newErrWrapper(err error, URL string) *errWrapper { + return &errWrapper{ + err: err, + url: URL, + } } // Error implements error.Error. -func (ew *errwrapper) Error() string { - return fmt.Sprintf("<%s> %s", ew.URL, ew.error.Error()) +func (ew *errWrapper) Error() string { + return fmt.Sprintf("<%s> %s", ew.url, ew.err.Error()) } // Is allows consumers to query for the type of the underlying error. -func (ew *errwrapper) Is(target error) bool { - return errors.Is(ew.error, target) +func (ew *errWrapper) Is(target error) bool { + return errors.Is(ew.err, target) +} + +// Unwrap returns the underlying error. +func (ew *errWrapper) Unwrap() error { + return ew.err } diff --git a/internal/engine/internal/sessionresolver/errwrapper_test.go b/internal/engine/internal/sessionresolver/errwrapper_test.go index 44c2e3d..1db791e 100644 --- a/internal/engine/internal/sessionresolver/errwrapper_test.go +++ b/internal/engine/internal/sessionresolver/errwrapper_test.go @@ -9,10 +9,7 @@ import ( ) func TestErrWrapper(t *testing.T) { - ew := &errwrapper{ - error: io.EOF, - URL: "https://dns.quad9.net/dns-query", - } + ew := newErrWrapper(io.EOF, "https://dns.quad9.net/dns-query") o := ew.Error() expect := " EOF" if diff := cmp.Diff(expect, o); diff != "" { @@ -21,4 +18,7 @@ func TestErrWrapper(t *testing.T) { if !errors.Is(ew, io.EOF) { t.Fatal("not the sub-error we expected") } + if errors.Unwrap(ew) != io.EOF { + t.Fatal("unwrap failed") + } } diff --git a/internal/engine/internal/sessionresolver/jsoncodec.go b/internal/engine/internal/sessionresolver/jsoncodec.go new file mode 100644 index 0000000..d884204 --- /dev/null +++ b/internal/engine/internal/sessionresolver/jsoncodec.go @@ -0,0 +1,37 @@ +package sessionresolver + +// +// JSON codec +// + +import "encoding/json" + +// jsonCodec encodes to/decodes from JSON. +type jsonCodec interface { + // Encode encodes v as a JSON stream of bytes. + Encode(v interface{}) ([]byte, error) + + // Decode decodes b from a JSON stream of bytes. + Decode(b []byte, v interface{}) error +} + +// codec always returns a valid jsonCodec. +func (r *Resolver) codec() jsonCodec { + if r.jsonCodec != nil { + return r.jsonCodec + } + return &jsonCodecStdlib{} +} + +// jsonCodecStdlib is the default codec. +type jsonCodecStdlib struct{} + +// Decode implements jsonCodec.Decode. +func (*jsonCodecStdlib) Decode(b []byte, v interface{}) error { + return json.Unmarshal(b, v) +} + +// Encode implements jsonCodec.Encode. +func (*jsonCodecStdlib) Encode(v interface{}) ([]byte, error) { + return json.Marshal(v) +} diff --git a/internal/engine/internal/sessionresolver/codec_test.go b/internal/engine/internal/sessionresolver/jsoncodec_test.go similarity index 52% rename from internal/engine/internal/sessionresolver/codec_test.go rename to internal/engine/internal/sessionresolver/jsoncodec_test.go index 7687770..77d90c0 100644 --- a/internal/engine/internal/sessionresolver/codec_test.go +++ b/internal/engine/internal/sessionresolver/jsoncodec_test.go @@ -6,40 +6,40 @@ import ( "github.com/google/go-cmp/cmp" ) -type FakeCodec struct { +type jsonCodecMockable struct { EncodeData []byte EncodeErr error DecodeErr error } -func (c *FakeCodec) Encode(v interface{}) ([]byte, error) { +func (c *jsonCodecMockable) Encode(v interface{}) ([]byte, error) { return c.EncodeData, c.EncodeErr } -func (c *FakeCodec) Decode(b []byte, v interface{}) error { +func (c *jsonCodecMockable) Decode(b []byte, v interface{}) error { return c.DecodeErr } -func TestCodecCustom(t *testing.T) { - c := &FakeCodec{} - reso := &Resolver{codec: c} - if r := reso.getCodec(); r != c { +func TestJSONCodecCustom(t *testing.T) { + c := &jsonCodecMockable{} + reso := &Resolver{jsonCodec: c} + if r := reso.codec(); r != c { t.Fatal("not the codec we expected") } } -func TestCodecDefault(t *testing.T) { +func TestJSONCodecDefault(t *testing.T) { reso := &Resolver{} in := resolverinfo{ URL: "https://dns.google/dns.query", Score: 0.99, } - data, err := reso.getCodec().Encode(in) + data, err := reso.codec().Encode(in) if err != nil { t.Fatal(err) } var out resolverinfo - if err := reso.getCodec().Decode(data, &out); err != nil { + if err := reso.codec().Decode(data, &out); err != nil { t.Fatal(err) } if diff := cmp.Diff(in, out); diff != "" { diff --git a/internal/engine/internal/sessionresolver/childresolver.go b/internal/engine/internal/sessionresolver/lookup.go similarity index 98% rename from internal/engine/internal/sessionresolver/childresolver.go rename to internal/engine/internal/sessionresolver/lookup.go index be9aa69..4fad123 100644 --- a/internal/engine/internal/sessionresolver/childresolver.go +++ b/internal/engine/internal/sessionresolver/lookup.go @@ -1,5 +1,9 @@ package sessionresolver +// +// Actual lookup code +// + import ( "context" "time" diff --git a/internal/engine/internal/sessionresolver/childresolver_test.go b/internal/engine/internal/sessionresolver/lookup_test.go similarity index 100% rename from internal/engine/internal/sessionresolver/childresolver_test.go rename to internal/engine/internal/sessionresolver/lookup_test.go diff --git a/internal/engine/internal/sessionresolver/sessionresolver.go b/internal/engine/internal/sessionresolver/resolver.go similarity index 75% rename from internal/engine/internal/sessionresolver/sessionresolver.go rename to internal/engine/internal/sessionresolver/resolver.go index 782298d..e257f4e 100644 --- a/internal/engine/internal/sessionresolver/sessionresolver.go +++ b/internal/engine/internal/sessionresolver/resolver.go @@ -1,28 +1,9 @@ -// Package sessionresolver contains the resolver used by the session. This -// resolver will try to figure out which is the best service for running -// domain name resolutions and will consistently use it. -// -// Occasionally this code will also swap the best resolver with other -// ~good resolvers to give them a chance to perform. -// -// The penalty/reward mechanism is strongly derivative, so the code should -// adapt ~quickly to changing network conditions. Occasionally, we will -// have longer resolutions when trying out other resolvers. -// -// At the beginning we randomize the known resolvers so that we do not -// have any preferential ordering. The initial resolutions may be slower -// if there are many issues with resolvers. -// -// The system resolver is given the lowest priority at the beginning -// but it will of course be the most popular resolver if anything else -// is failing us. (We will still occasionally probe for other working -// resolvers and increase their score on success.) -// -// We also support a socks5 proxy. When such a proxy is configured, -// the code WILL skip http3 resolvers AS WELL AS the system -// resolver, in an attempt to avoid leaking your queries. package sessionresolver +// +// Implementation of Resolver +// + import ( "context" "encoding/json" @@ -52,11 +33,8 @@ import ( // // You MUST NOT modify public fields of this structure once it // has been created, because that MAY lead to data races. -// -// You should create an instance of this structure and use -// it in internal/engine/session.go. type Resolver struct { - // ByteCounter is the optional byte counter. It will count + // ByteCounter is the OPTIONAL byte counter. It will count // the bytes used by any child resolver except for the // system resolver, whose bytes ARE NOT counted. If this // field is not set, then we won't count the bytes. @@ -67,21 +45,21 @@ type Resolver struct { // working better in your network. KVStore model.KeyValueStore - // Logger is the optional logger you want us to use + // Logger is the OPTIONAL logger you want us to use // to emit log messages. Logger model.Logger - // ProxyURL is the optional URL of the socks5 proxy + // ProxyURL is the OPTIONAL URL of the socks5 proxy // we should be using. If not set, then we WON'T use // any proxy. If set, then we WON'T use any http3 // based resolvers and we WON'T use the system resolver. ProxyURL *url.URL - // codec is the optional codec to use. If not set, we - // will construct a default codec. - codec codec + // jsonCodec is the OPTIONAL JSON Codec to use. If not set, + // we will construct a default codec. + jsonCodec jsonCodec - // dnsClientMaker is the optional dnsclientmaker to + // dnsClientMaker is the OPTIONAL dnsclientmaker to // use. If not set, we will use the default. dnsClientMaker dnsclientmaker @@ -111,16 +89,17 @@ func (r *Resolver) Stats() string { return fmt.Sprintf("sessionresolver: %s", string(data)) } -var errNotImplemented = errors.New("not implemented") +// errLookupNotImplemented indicates a given lookup type is not implemented. +var errLookupNotImplemented = errors.New("sessionresolver: lookup not implemented") // LookupHTTPS implements Resolver.LookupHTTPS. func (r *Resolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPSSvc, error) { - return nil, errNotImplemented + return nil, errLookupNotImplemented } // LookupNS implements Resolver.LookupNS. func (r *Resolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { - return nil, errNotImplemented + return nil, errLookupNotImplemented } // ErrLookupHost indicates that LookupHost failed. @@ -143,7 +122,7 @@ func (r *Resolver) LookupHost(ctx context.Context, hostname string) ([]string, e if err == nil { return addrs, nil } - me.Add(&errwrapper{error: err, URL: e.URL}) + me.Add(newErrWrapper(err, e.URL)) } return nil, me } diff --git a/internal/engine/internal/sessionresolver/sessionresolver_test.go b/internal/engine/internal/sessionresolver/resolver_test.go similarity index 96% rename from internal/engine/internal/sessionresolver/sessionresolver_test.go rename to internal/engine/internal/sessionresolver/resolver_test.go index d1cf3f9..2729c06 100644 --- a/internal/engine/internal/sessionresolver/sessionresolver_test.go +++ b/internal/engine/internal/sessionresolver/resolver_test.go @@ -47,14 +47,14 @@ func TestTypicalUsageWithFailure(t *testing.T) { // means that we need to go down hunting what's the // real error that occurred and use more verbose code. { - var errWrapper *errwrapper - if !errors.As(child, &errWrapper) { + var ew *errWrapper + if !errors.As(child, &ew) { t.Fatal("not an instance of errwrapper") } - var dnsError *net.DNSError - if errors.As(errWrapper.error, &dnsError) { - if !strings.HasSuffix(dnsError.Err, "operation was canceled") { - t.Fatal("not the error we expected", dnsError.Err) + var de *net.DNSError + if errors.As(ew, &de) { + if !strings.HasSuffix(de.Err, "operation was canceled") { + t.Fatal("not the error we expected", de.Err) } continue } @@ -361,7 +361,7 @@ func TestUnimplementedFunctions(t *testing.T) { t.Run("LookupHTTPS", func(t *testing.T) { r := &Resolver{} https, err := r.LookupHTTPS(context.Background(), "dns.google") - if !errors.Is(err, errNotImplemented) { + if !errors.Is(err, errLookupNotImplemented) { t.Fatal("unexpected error", err) } if https != nil { @@ -372,7 +372,7 @@ func TestUnimplementedFunctions(t *testing.T) { t.Run("LookupNS", func(t *testing.T) { r := &Resolver{} ns, err := r.LookupNS(context.Background(), "dns.google") - if !errors.Is(err, errNotImplemented) { + if !errors.Is(err, errLookupNotImplemented) { t.Fatal("unexpected error", err) } if len(ns) > 0 { diff --git a/internal/engine/internal/sessionresolver/resolvermaker.go b/internal/engine/internal/sessionresolver/resolvermaker.go index 2da4411..5283501 100644 --- a/internal/engine/internal/sessionresolver/resolvermaker.go +++ b/internal/engine/internal/sessionresolver/resolvermaker.go @@ -1,5 +1,9 @@ package sessionresolver +// +// Code for creating a new child resolver +// + import ( "math/rand" "strings" diff --git a/internal/engine/internal/sessionresolver/state.go b/internal/engine/internal/sessionresolver/state.go index 19970c3..513c320 100644 --- a/internal/engine/internal/sessionresolver/state.go +++ b/internal/engine/internal/sessionresolver/state.go @@ -1,5 +1,9 @@ package sessionresolver +// +// Persistent on-disk state +// + import ( "errors" "sort" @@ -31,7 +35,7 @@ func (r *Resolver) readstate() ([]*resolverinfo, error) { return nil, err } var ri []*resolverinfo - if err := r.getCodec().Decode(data, &ri); err != nil { + if err := r.codec().Decode(data, &ri); err != nil { return nil, err } return ri, nil @@ -89,12 +93,12 @@ func (r *Resolver) readstatedefault() []*resolverinfo { return ri } -// writestate writes the state on the kvstore. +// writestate writes the state to the kvstore. func (r *Resolver) writestate(ri []*resolverinfo) error { if r.KVStore == nil { return ErrNilKVStore } - data, err := r.getCodec().Encode(ri) + data, err := r.codec().Encode(ri) if err != nil { return err } diff --git a/internal/engine/internal/sessionresolver/state_test.go b/internal/engine/internal/sessionresolver/state_test.go index 0046325..0ac1a78 100644 --- a/internal/engine/internal/sessionresolver/state_test.go +++ b/internal/engine/internal/sessionresolver/state_test.go @@ -32,8 +32,8 @@ func TestReadStateNothingInKVStore(t *testing.T) { func TestReadStateDecodeError(t *testing.T) { errMocked := errors.New("mocked error") reso := &Resolver{ - KVStore: &kvstore.Memory{}, - codec: &FakeCodec{DecodeErr: errMocked}, + KVStore: &kvstore.Memory{}, + jsonCodec: &jsonCodecMockable{DecodeErr: errMocked}, } if err := reso.KVStore.Set(storekey, []byte(`[]`)); err != nil { t.Fatal(err) @@ -130,7 +130,7 @@ func TestWriteStateNoKVStore(t *testing.T) { func TestWriteStateCannotSerialize(t *testing.T) { errMocked := errors.New("mocked error") reso := &Resolver{ - codec: &FakeCodec{ + jsonCodec: &jsonCodecMockable{ EncodeErr: errMocked, }, KVStore: &kvstore.Memory{},