diff --git a/internal/engine/experiment/webconnectivity/control.go b/internal/engine/experiment/webconnectivity/control.go index 8d4b3ab..4d189b0 100644 --- a/internal/engine/experiment/webconnectivity/control.go +++ b/internal/engine/experiment/webconnectivity/control.go @@ -57,13 +57,13 @@ func Control( HTTPClient: sess.DefaultHTTPClient(), Logger: sess.Logger(), } - sess.Logger().Infof("control %s...", creq.HTTPRequest) + sess.Logger().Infof("control for %s...", creq.HTTPRequest) // make sure error is wrapped err = errorx.SafeErrWrapperBuilder{ Error: clnt.PostJSON(ctx, "/", creq, &out), Operation: errorx.TopLevelOperation, }.MaybeBuild() - sess.Logger().Infof("control %s... %+v", creq.HTTPRequest, err) + sess.Logger().Infof("control for %s... %+v", creq.HTTPRequest, err) (&out.DNS).FillASNs(sess) return } diff --git a/internal/engine/experiment/webconnectivity/dnslookup.go b/internal/engine/experiment/webconnectivity/dnslookup.go index 92133b7..6d0a324 100644 --- a/internal/engine/experiment/webconnectivity/dnslookup.go +++ b/internal/engine/experiment/webconnectivity/dnslookup.go @@ -37,6 +37,7 @@ func DNSLookup(ctx context.Context, config DNSLookupConfig) (out DNSLookupResult } if answer.IPv6 != "" { out.Addrs[answer.IPv6] = answer.ASN + continue } } } diff --git a/internal/engine/experiment/webconnectivity/httpget.go b/internal/engine/experiment/webconnectivity/httpget.go index ad177f5..35200ca 100644 --- a/internal/engine/experiment/webconnectivity/httpget.go +++ b/internal/engine/experiment/webconnectivity/httpget.go @@ -3,6 +3,7 @@ package webconnectivity import ( "context" "fmt" + "net" "net/url" "strings" @@ -25,6 +26,22 @@ type HTTPGetResult struct { Failure *string } +// TODO(bassosimone): Web Connectivity uses too much external testing +// and we should actually expose much less to the outside by using +// internal testing and by making _many_ functions private. + +// HTTPGetMakeDNSCache constructs the DNSCache option for HTTPGet +// by combining domain and addresses into a single string. As a +// corner case, if the domain is an IP address, we return an empty +// string. This corner case corresponds to Web Connectivity +// inputs like https://1.1.1.1. +func HTTPGetMakeDNSCache(domain, addresses string) string { + if net.ParseIP(domain) != nil { + return "" + } + return fmt.Sprintf("%s %s", domain, addresses) +} + // HTTPGet performs the HTTP/HTTPS part of Web Connectivity. func HTTPGet(ctx context.Context, config HTTPGetConfig) (out HTTPGetResult) { addresses := strings.Join(config.Addresses, " ") @@ -38,7 +55,7 @@ func HTTPGet(ctx context.Context, config HTTPGetConfig) (out HTTPGetResult) { domain := config.TargetURL.Hostname() result, err := urlgetter.Getter{ Config: urlgetter.Config{ - DNSCache: fmt.Sprintf("%s %s", domain, addresses), + DNSCache: HTTPGetMakeDNSCache(domain, addresses), }, Session: config.Session, Target: target, diff --git a/internal/engine/experiment/webconnectivity/httpget_test.go b/internal/engine/experiment/webconnectivity/httpget_test.go index d19cdbf..6f1ddc9 100644 --- a/internal/engine/experiment/webconnectivity/httpget_test.go +++ b/internal/engine/experiment/webconnectivity/httpget_test.go @@ -25,3 +25,20 @@ func TestHTTPGet(t *testing.T) { t.Fatal(*r.Failure) } } + +func TestHTTPGetMakeDNSCache(t *testing.T) { + // test for input being an IP + out := webconnectivity.HTTPGetMakeDNSCache( + "1.1.1.1", "1.1.1.1", + ) + if out != "" { + t.Fatal("expected empty output here") + } + // test for input being a domain + out = webconnectivity.HTTPGetMakeDNSCache( + "dns.google", "8.8.8.8 8.8.4.4", + ) + if out != "dns.google 8.8.8.8 8.8.4.4" { + t.Fatal("expected ordinary output here") + } +} diff --git a/internal/engine/experiment/webconnectivity/webconnectivity.go b/internal/engine/experiment/webconnectivity/webconnectivity.go index 9788e49..7f76ed4 100644 --- a/internal/engine/experiment/webconnectivity/webconnectivity.go +++ b/internal/engine/experiment/webconnectivity/webconnectivity.go @@ -19,7 +19,7 @@ import ( const ( testName = "web_connectivity" - testVersion = "0.2.0" + testVersion = "0.3.0" ) // Config contains the experiment config. diff --git a/internal/engine/experiment/webconnectivity/webconnectivity_test.go b/internal/engine/experiment/webconnectivity/webconnectivity_test.go index e9e3335..8a04618 100644 --- a/internal/engine/experiment/webconnectivity/webconnectivity_test.go +++ b/internal/engine/experiment/webconnectivity/webconnectivity_test.go @@ -21,7 +21,7 @@ func TestNewExperimentMeasurer(t *testing.T) { if measurer.ExperimentName() != "web_connectivity" { t.Fatal("unexpected name") } - if measurer.ExperimentVersion() != "0.2.0" { + if measurer.ExperimentVersion() != "0.3.0" { t.Fatal("unexpected version") } } diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index 554fcdf..25f2365 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -126,6 +126,7 @@ func NewResolver(config Config) Resolver { config.BaseResolver = resolver.SystemResolver{} } var r Resolver = config.BaseResolver + r = resolver.AddressResolver{Resolver: r} if config.CacheResolutions { r = &resolver.CacheResolver{Resolver: r} } @@ -146,7 +147,6 @@ func NewResolver(config Config) Resolver { if config.ResolveSaver != nil { r = resolver.SaverResolver{Resolver: r, Saver: config.ResolveSaver} } - r = resolver.AddressResolver{Resolver: r} return resolver.IDNAResolver{Resolver: r} } diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index 00bf58e..9526ee8 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -23,15 +23,15 @@ func TestNewResolverVanilla(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ir.Resolver.(resolver.AddressResolver) + ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver) + ar, ok := ewr.Resolver.(resolver.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } - _, ok = ewr.Resolver.(resolver.SystemResolver) + _, ok = ar.Resolver.(resolver.SystemResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -47,15 +47,15 @@ func TestNewResolverSpecificResolver(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ir.Resolver.(resolver.AddressResolver) + ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver) + ar, ok := ewr.Resolver.(resolver.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } - _, ok = ewr.Resolver.(resolver.BogonResolver) + _, ok = ar.Resolver.(resolver.BogonResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -69,11 +69,7 @@ func TestNewResolverWithBogonFilter(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ir.Resolver.(resolver.AddressResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -81,7 +77,11 @@ func TestNewResolverWithBogonFilter(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - _, ok = br.Resolver.(resolver.SystemResolver) + ar, ok := br.Resolver.(resolver.AddressResolver) + if !ok { + t.Fatal("not the resolver we expected") + } + _, ok = ar.Resolver.(resolver.SystemResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -95,11 +95,7 @@ func TestNewResolverWithLogging(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ir.Resolver.(resolver.AddressResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - lr, ok := ar.Resolver.(resolver.LoggingResolver) + lr, ok := ir.Resolver.(resolver.LoggingResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -110,7 +106,11 @@ func TestNewResolverWithLogging(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - _, ok = ewr.Resolver.(resolver.SystemResolver) + ar, ok := ewr.Resolver.(resolver.AddressResolver) + if !ok { + t.Fatal("not the resolver we expected") + } + _, ok = ar.Resolver.(resolver.SystemResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -125,11 +125,7 @@ func TestNewResolverWithSaver(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ir.Resolver.(resolver.AddressResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - sr, ok := ar.Resolver.(resolver.SaverResolver) + sr, ok := ir.Resolver.(resolver.SaverResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -140,7 +136,11 @@ func TestNewResolverWithSaver(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - _, ok = ewr.Resolver.(resolver.SystemResolver) + ar, ok := ewr.Resolver.(resolver.AddressResolver) + if !ok { + t.Fatal("not the resolver we expected") + } + _, ok = ar.Resolver.(resolver.SystemResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -154,11 +154,7 @@ func TestNewResolverWithReadWriteCache(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ir.Resolver.(resolver.AddressResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -169,7 +165,11 @@ func TestNewResolverWithReadWriteCache(t *testing.T) { if cr.ReadOnly != false { t.Fatal("expected readwrite cache here") } - _, ok = cr.Resolver.(resolver.SystemResolver) + ar, ok := cr.Resolver.(resolver.AddressResolver) + if !ok { + t.Fatal("not the resolver we expected") + } + _, ok = ar.Resolver.(resolver.SystemResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -185,11 +185,7 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ir.Resolver.(resolver.AddressResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - ewr, ok := ar.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -203,7 +199,11 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) { if cr.Get("dns.google.com")[0] != "8.8.8.8" { t.Fatal("cache not correctly prefilled") } - _, ok = cr.Resolver.(resolver.SystemResolver) + ar, ok := cr.Resolver.(resolver.AddressResolver) + if !ok { + t.Fatal("not the resolver we expected") + } + _, ok = ar.Resolver.(resolver.SystemResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -233,7 +233,7 @@ func TestNewDialerVanilla(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - if _, ok := ir.Resolver.(resolver.AddressResolver); !ok { + if _, ok := ir.Resolver.(resolver.ErrorWrapperResolver); !ok { t.Fatal("not the resolver we expected") } ewd, ok := dnsd.Dialer.(dialer.ErrorWrapperDialer) @@ -315,7 +315,7 @@ func TestNewDialerWithLogger(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - if _, ok := ir.Resolver.(resolver.AddressResolver); !ok { + if _, ok := ir.Resolver.(resolver.LoggingResolver); !ok { t.Fatal("not the resolver we expected") } ld, ok := dnsd.Dialer.(dialer.LoggingDialer) @@ -365,7 +365,7 @@ func TestNewDialerWithDialSaver(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - if _, ok := ir.Resolver.(resolver.AddressResolver); !ok { + if _, ok := ir.Resolver.(resolver.ErrorWrapperResolver); !ok { t.Fatal("not the resolver we expected") } sad, ok := dnsd.Dialer.(dialer.SaverDialer) @@ -415,7 +415,7 @@ func TestNewDialerWithReadWriteSaver(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - if _, ok := ir.Resolver.(resolver.AddressResolver); !ok { + if _, ok := ir.Resolver.(resolver.ErrorWrapperResolver); !ok { t.Fatal("not the resolver we expected") } scd, ok := dnsd.Dialer.(dialer.SaverConnDialer) @@ -468,7 +468,7 @@ func TestNewDialerWithContextByteCounting(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - if _, ok := ir.Resolver.(resolver.AddressResolver); !ok { + if _, ok := ir.Resolver.(resolver.ErrorWrapperResolver); !ok { t.Fatal("not the resolver we expected") } ewd, ok := dnsd.Dialer.(dialer.ErrorWrapperDialer)