fix(web_connectivity@v0.5): limit number of redirects (#965)

Part of https://github.com/ooni/probe/issues/2237

internal/experiment/webconnectivity/cleartextflow.go

@@ -38,6 +38,9 @@ type CleartextFlow struct {
 	// Logger is the MANDATORY logger to use.
 	Logger model.Logger
 
+	// NumRedirects it the MANDATORY counter of the number of redirects.
+	NumRedirects *NumRedirects
+
 	// TestKeys is MANDATORY and contains the TestKeys.
 	TestKeys *TestKeys
 
@@ -259,8 +262,8 @@ func (t *CleartextFlow) httpTransaction(ctx context.Context, network, address, a
 
 // maybeFollowRedirects follows redirects if configured and needed
 func (t *CleartextFlow) maybeFollowRedirects(ctx context.Context, resp *http.Response) {
-	if !t.FollowRedirects {
-		return // not configured
+	if !t.FollowRedirects || !t.NumRedirects.CanFollowOneMoreRedirect() {
+		return // not configured or too many redirects
 	}
 	switch resp.StatusCode {
 	case 301, 302, 307, 308:
@@ -270,19 +273,20 @@ func (t *CleartextFlow) maybeFollowRedirects(ctx context.Context, resp *http.Res
 		}
 		t.Logger.Infof("redirect to: %s", location.String())
 		resolvers := &DNSResolvers{
-			CookieJar:   t.CookieJar,
-			DNSCache:    t.DNSCache,
-			Domain:      location.Hostname(),
-			IDGenerator: t.IDGenerator,
-			Logger:      t.Logger,
-			TestKeys:    t.TestKeys,
-			URL:         location,
-			ZeroTime:    t.ZeroTime,
-			WaitGroup:   t.WaitGroup,
-			Referer:     resp.Request.URL.String(),
-			Session:     nil, // no need to issue another control request
-			THAddr:      "",  // ditto
-			UDPAddress:  t.UDPAddress,
+			CookieJar:    t.CookieJar,
+			DNSCache:     t.DNSCache,
+			Domain:       location.Hostname(),
+			IDGenerator:  t.IDGenerator,
+			Logger:       t.Logger,
+			NumRedirects: t.NumRedirects,
+			TestKeys:     t.TestKeys,
+			URL:          location,
+			ZeroTime:     t.ZeroTime,
+			WaitGroup:    t.WaitGroup,
+			Referer:      resp.Request.URL.String(),
+			Session:      nil, // no need to issue another control request
+			THAddr:       "",  // ditto
+			UDPAddress:   t.UDPAddress,
 		}
 		resolvers.Start(ctx)
 	default:

internal/experiment/webconnectivity/dnsresolvers.go

@@ -39,6 +39,9 @@ type DNSResolvers struct {
 	// Logger is the MANDATORY logger to use.
 	Logger model.Logger
 
+	// NumRedirects it the MANDATORY counter of the number of redirects.
+	NumRedirects *NumRedirects
+
 	// TestKeys is MANDATORY and contains the TestKeys.
 	TestKeys *TestKeys
 
@@ -434,6 +437,7 @@ func (t *DNSResolvers) startCleartextFlows(
 			DNSCache:        t.DNSCache,
 			IDGenerator:     t.IDGenerator,
 			Logger:          t.Logger,
+			NumRedirects:    t.NumRedirects,
 			TestKeys:        t.TestKeys,
 			ZeroTime:        t.ZeroTime,
 			WaitGroup:       t.WaitGroup,
@@ -475,6 +479,7 @@ func (t *DNSResolvers) startSecureFlows(
 			DNSCache:        t.DNSCache,
 			IDGenerator:     t.IDGenerator,
 			Logger:          t.Logger,
+			NumRedirects:    t.NumRedirects,
 			TestKeys:        t.TestKeys,
 			ZeroTime:        t.ZeroTime,
 			WaitGroup:       t.WaitGroup,

internal/experiment/webconnectivity/measurer.go

@@ -36,7 +36,7 @@ func (m *Measurer) ExperimentName() string {
 
 // ExperimentVersion implements model.ExperimentMeasurer.
 func (m *Measurer) ExperimentVersion() string {
-	return "0.5.17"
+	return "0.5.18"
 }
 
 // Run implements model.ExperimentMeasurer.
@@ -108,19 +108,20 @@ func (m *Measurer) Run(ctx context.Context, sess model.ExperimentSession,
 
 	// start background tasks
 	resos := &DNSResolvers{
-		DNSCache:    NewDNSCache(),
-		Domain:      URL.Hostname(),
-		IDGenerator: idGenerator,
-		Logger:      sess.Logger(),
-		TestKeys:    tk,
-		URL:         URL,
-		ZeroTime:    measurement.MeasurementStartTimeSaved,
-		WaitGroup:   wg,
-		CookieJar:   jar,
-		Referer:     "",
-		Session:     sess,
-		THAddr:      thAddr,
-		UDPAddress:  "",
+		DNSCache:     NewDNSCache(),
+		Domain:       URL.Hostname(),
+		IDGenerator:  idGenerator,
+		Logger:       sess.Logger(),
+		NumRedirects: NewNumRedirects(5),
+		TestKeys:     tk,
+		URL:          URL,
+		ZeroTime:     measurement.MeasurementStartTimeSaved,
+		WaitGroup:    wg,
+		CookieJar:    jar,
+		Referer:      "",
+		Session:      sess,
+		THAddr:       thAddr,
+		UDPAddress:   "",
 	}
 	resos.Start(ctx)
 

internal/experiment/webconnectivity/redirects.go

@@ -0,0 +1,23 @@
+package webconnectivity
+
+import "github.com/ooni/probe-cli/v3/internal/atomicx"
+
+// NumRedirects counts the number of redirects left.
+type NumRedirects struct {
+	count *atomicx.Int64
+}
+
+// NewNumRedirects creates a new NumRedirects instance.
+func NewNumRedirects(n int64) *NumRedirects {
+	count := &atomicx.Int64{}
+	count.Add(n)
+	return &NumRedirects{
+		count: count,
+	}
+}
+
+// CanFollowOneMoreRedirect returns true if we are
+// allowed to follow one more redirect.
+func (nr *NumRedirects) CanFollowOneMoreRedirect() bool {
+	return nr.count.Add(-1) > 0
+}

internal/experiment/webconnectivity/secureflow.go

@@ -39,6 +39,9 @@ type SecureFlow struct {
 	// Logger is the MANDATORY logger to use.
 	Logger model.Logger
 
+	// NumRedirects it the MANDATORY counter of the number of redirects.
+	NumRedirects *NumRedirects
+
 	// TestKeys is MANDATORY and contains the TestKeys.
 	TestKeys *TestKeys
 
@@ -311,8 +314,8 @@ func (t *SecureFlow) httpTransaction(ctx context.Context, network, address, alpn
 
 // maybeFollowRedirects follows redirects if configured and needed
 func (t *SecureFlow) maybeFollowRedirects(ctx context.Context, resp *http.Response) {
-	if !t.FollowRedirects {
-		return // not configured
+	if !t.FollowRedirects || !t.NumRedirects.CanFollowOneMoreRedirect() {
+		return // not configured or too many redirects
 	}
 	switch resp.StatusCode {
 	case 301, 302, 307, 308:
@@ -322,19 +325,20 @@ func (t *SecureFlow) maybeFollowRedirects(ctx context.Context, resp *http.Respon
 		}
 		t.Logger.Infof("redirect to: %s", location.String())
 		resolvers := &DNSResolvers{
-			CookieJar:   t.CookieJar,
-			DNSCache:    t.DNSCache,
-			Domain:      location.Hostname(),
-			IDGenerator: t.IDGenerator,
-			Logger:      t.Logger,
-			TestKeys:    t.TestKeys,
-			URL:         location,
-			ZeroTime:    t.ZeroTime,
-			WaitGroup:   t.WaitGroup,
-			Referer:     resp.Request.URL.String(),
-			Session:     nil, // no need to issue another control request
-			THAddr:      "",  // ditto
-			UDPAddress:  t.UDPAddress,
+			CookieJar:    t.CookieJar,
+			DNSCache:     t.DNSCache,
+			Domain:       location.Hostname(),
+			IDGenerator:  t.IDGenerator,
+			Logger:       t.Logger,
+			NumRedirects: t.NumRedirects,
+			TestKeys:     t.TestKeys,
+			URL:          location,
+			ZeroTime:     t.ZeroTime,
+			WaitGroup:    t.WaitGroup,
+			Referer:      resp.Request.URL.String(),
+			Session:      nil, // no need to issue another control request
+			THAddr:       "",  // ditto
+			UDPAddress:   t.UDPAddress,
 		}
 		resolvers.Start(ctx)
 	default: