chore(webconnectivityqa): reproduce ooni/probe#2628 issue (#1479)

This PR improves webconnectivityqa to reproduce the ooni/probe#2628 issue. We model the v0.4 behavior as the correct behavior. We need to extend webconnectivitylte to correctly handle this case.
ooni · Jan 31, 2024 · 69a6c89 · 69a6c89
1 parent db5878c
commit 69a6c89
Show file tree

Hide file tree

Showing 7 changed files with 269 additions and 4 deletions.
diff --git a/internal/experiment/webconnectivityqa/redirect.go b/internal/experiment/webconnectivityqa/redirect.go
@@ -342,3 +342,57 @@ func redirectWithConsistentDNSAndThenTimeoutForHTTPS() *TestCase {
 		},
 	}
 }
+
+// redirectWithBrokenLocationForHTTP is a scenario where the redirect
+// returns a broken URL only containing `http://`.
+//
+// See https://github.com/ooni/probe/issues/2628 for more info.
+func redirectWithBrokenLocationForHTTP() *TestCase {
+	return &TestCase{
+		Name:     "redirectWithBrokenLocationForHTTP",
+		Flags:    TestCaseFlagNoLTE,
+		Input:    "http://httpbin.com/broken-redirect-http",
+		LongTest: true,
+		Configure: func(env *netemx.QAEnv) {
+			// nothing
+		},
+		ExpectErr: false,
+		ExpectTestKeys: &testKeys{
+			DNSExperimentFailure:  nil,
+			DNSConsistency:        "consistent",
+			HTTPExperimentFailure: "unknown_failure: http: no Host in request URL",
+			XStatus:               8192, // StatusExperimentHTTP
+			XDNSFlags:             0,
+			XBlockingFlags:        1, // AnalysisBlockingFlagDNSBlocking
+			Accessible:            nil,
+			Blocking:              nil,
+		},
+	}
+}
+
+// redirectWithBrokenLocationForHTTPS is a scenario where the redirect
+// returns a broken URL only containing `https://`.
+//
+// See https://github.com/ooni/probe/issues/2628 for more info.
+func redirectWithBrokenLocationForHTTPS() *TestCase {
+	return &TestCase{
+		Name:     "redirectWithBrokenLocationForHTTPS",
+		Flags:    TestCaseFlagNoLTE,
+		Input:    "https://httpbin.com/broken-redirect-https",
+		LongTest: true,
+		Configure: func(env *netemx.QAEnv) {
+			// nothing
+		},
+		ExpectErr: false,
+		ExpectTestKeys: &testKeys{
+			DNSExperimentFailure:  nil,
+			DNSConsistency:        "consistent",
+			HTTPExperimentFailure: "unknown_failure: http: no Host in request URL",
+			XStatus:               8192, // StatusExperimentHTTP
+			XDNSFlags:             0,
+			XBlockingFlags:        1, // AnalysisBlockingFlagDNSBlocking
+			Accessible:            nil,
+			Blocking:              nil,
+		},
+	}
+}
diff --git a/internal/experiment/webconnectivityqa/testcase.go b/internal/experiment/webconnectivityqa/testcase.go
@@ -71,6 +71,8 @@ func AllTestCases() []*TestCase {
 		localhostWithHTTP(),
 		localhostWithHTTPS(),
 
+		redirectWithBrokenLocationForHTTP(),
+		redirectWithBrokenLocationForHTTPS(),
 		redirectWithConsistentDNSAndThenConnectionRefusedForHTTP(),
 		redirectWithConsistentDNSAndThenConnectionRefusedForHTTPS(),
 		redirectWithConsistentDNSAndThenConnectionResetForHTTP(),

diff --git a/internal/netemx/address.go b/internal/netemx/address.go
@@ -71,5 +71,9 @@ const AddressYandexCom3 = "77.88.55.77"
 // AddressYandexCom4 is the fourth address associated with yandex.com.
 const AddressYandexCom4 = "77.88.55.80"
 
-// CloudflareCacheAddress1 is the first address associated with cloudflare caches.
-const CloudflareCacheAddress1 = "104.16.132.229"
+// AddressCloudflareCache1 is the first address associated with cloudflare caches.
+const AddressCloudflareCache1 = "104.16.132.229"
+
+// AddressHTTPBinCom1 is the first address associated an httpbin.com-like
+// service which our QA environment exports as httpbin.com.
+const AddressHTTPBinCom1 = "172.67.144.64"
diff --git a/internal/netemx/httpbin.go b/internal/netemx/httpbin.go
@@ -0,0 +1,70 @@
+package netemx
+
+import (
+	"net"
+	"net/http"
+
+	"github.com/ooni/netem"
+)
+
+// HTTPBinHandlerFactory constructs an [HTTPBinHandler].
+func HTTPBinHandlerFactory() HTTPHandlerFactory {
+	return HTTPHandlerFactoryFunc(func(env NetStackServerFactoryEnv, stack *netem.UNetStack) http.Handler {
+		return HTTPBinHandler()
+	})
+}
+
+// HTTPBinHandler returns the [http.Handler] implementing an httpbin.com-like service.
+//
+// We currently implement the following API endpoints:
+//
+//	/broken-redirect-http
+//		When accessed by the OONI Probe client redirects with 302 to http:// and
+//		otherwise redirects to the https://www.example.com/ URL.
+//
+//	/broken-redirect-https
+//		When accessed by the OONI Probe client redirects with 302 to https:// and
+//		otherwise redirects to the https://www.example.com/ URL.
+//
+// Any other request URL causes a 404 respose.
+func HTTPBinHandler() http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// missing address => 500
+		address, _, err := net.SplitHostPort(r.RemoteAddr)
+		if err != nil {
+			w.WriteHeader(http.StatusInternalServerError)
+			return
+		}
+
+		// compute variables used by the switch below
+		cleartextRedirect := r.URL.Path == "/broken-redirect-http"
+		client := address == DefaultClientAddress
+		secureRedirect := r.URL.Path == "/broken-redirect-https"
+
+		switch {
+		// broken HTTP redirect for clients
+		case cleartextRedirect && client:
+			w.Header().Set("Location", "http://")
+			w.WriteHeader(http.StatusFound)
+
+		// working HTTP redirect for anyone else
+		case cleartextRedirect && !client:
+			w.Header().Set("Location", "http://www.example.com/")
+			w.WriteHeader(http.StatusFound)
+
+		// broken HTTPS redirect for clients
+		case secureRedirect && client:
+			w.Header().Set("Location", "https://")
+			w.WriteHeader(http.StatusFound)
+
+		// working HTTPS redirect for anyone else
+		case secureRedirect && !client:
+			w.Header().Set("Location", "https://www.example.com/")
+			w.WriteHeader(http.StatusFound)
+
+		// otherwise
+		default:
+			w.WriteHeader(http.StatusNotFound)
+		}
+	})
+}
diff --git a/internal/netemx/httpbin_test.go b/internal/netemx/httpbin_test.go
@@ -0,0 +1,124 @@
+package netemx
+
+import (
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+)
+
+func TestHTTPBinHandler(t *testing.T) {
+	t.Run("missing client address", func(t *testing.T) {
+		req := &http.Request{
+			URL:   &url.URL{Scheme: "http://", Path: "/"},
+			Body:  http.NoBody,
+			Close: false,
+			Host:  "httpbin.com",
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusInternalServerError {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+	})
+
+	t.Run("/broken-redirect-http with client address", func(t *testing.T) {
+		req := &http.Request{
+			URL:        &url.URL{Scheme: "http://", Path: "/broken-redirect-http"},
+			Body:       http.NoBody,
+			Close:      false,
+			Host:       "httpbin.com",
+			RemoteAddr: net.JoinHostPort(DefaultClientAddress, "54321"),
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusFound {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+		if loc := result.Header.Get("Location"); loc != "http://" {
+			t.Fatal("unexpected location", loc)
+		}
+	})
+
+	t.Run("/broken-redirect-http with another address", func(t *testing.T) {
+		req := &http.Request{
+			URL:        &url.URL{Scheme: "http://", Path: "/broken-redirect-http"},
+			Body:       http.NoBody,
+			Close:      false,
+			Host:       "httpbin.com",
+			RemoteAddr: net.JoinHostPort("8.8.8.8", "54321"),
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusFound {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+		if loc := result.Header.Get("Location"); loc != "http://www.example.com/" {
+			t.Fatal("unexpected location", loc)
+		}
+	})
+
+	t.Run("/broken-redirect-https with client address", func(t *testing.T) {
+		req := &http.Request{
+			URL:        &url.URL{Scheme: "http://", Path: "/broken-redirect-https"},
+			Body:       http.NoBody,
+			Close:      false,
+			Host:       "httpbin.com",
+			RemoteAddr: net.JoinHostPort(DefaultClientAddress, "54321"),
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusFound {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+		if loc := result.Header.Get("Location"); loc != "https://" {
+			t.Fatal("unexpected location", loc)
+		}
+	})
+
+	t.Run("/broken-redirect-https with another address", func(t *testing.T) {
+		req := &http.Request{
+			URL:        &url.URL{Scheme: "http://", Path: "/broken-redirect-https"},
+			Body:       http.NoBody,
+			Close:      false,
+			Host:       "httpbin.com",
+			RemoteAddr: net.JoinHostPort("8.8.8.8", "54321"),
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusFound {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+		if loc := result.Header.Get("Location"); loc != "https://www.example.com/" {
+			t.Fatal("unexpected location", loc)
+		}
+	})
+
+	t.Run("/nonexistent URL", func(t *testing.T) {
+		req := &http.Request{
+			URL:        &url.URL{Scheme: "https://", Path: "/nonexistent"},
+			Body:       http.NoBody,
+			Close:      false,
+			Host:       "httpbin.com",
+			RemoteAddr: net.JoinHostPort("8.8.8.8", "54321"),
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusNotFound {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+	})
+}
diff --git a/internal/netemx/scenario.go b/internal/netemx/scenario.go
@@ -209,7 +209,7 @@ var InternetScenario = []*ScenarioDomainAddresses{{
 	WebServerFactory: YandexHandlerFactory(),
 }, {
 	Addresses: []string{
-		CloudflareCacheAddress1,
+		AddressCloudflareCache1,
 	},
 	Domains: []string{
 		"www.cloudflare-cache.com",
@@ -218,6 +218,17 @@ var InternetScenario = []*ScenarioDomainAddresses{{
 	ServerNameMain:   "www.cloudflare-cache.com",
 	ServerNameExtras: []string{},
 	WebServerFactory: CloudflareCAPTCHAHandlerFactory(),
+}, {
+	Addresses: []string{
+		AddressHTTPBinCom1,
+	},
+	Domains: []string{
+		"httpbin.com",
+	},
+	Role:             ScenarioRoleWebServer,
+	ServerNameMain:   "httpbin.com",
+	ServerNameExtras: []string{},
+	WebServerFactory: HTTPBinHandlerFactory(),
 }}
 
 // MustNewScenario constructs a complete testing scenario using the domains and IP

diff --git a/internal/netemx/yandex.go b/internal/netemx/yandex.go
@@ -14,7 +14,7 @@ func YandexHandlerFactory() HTTPHandlerFactory {
 	})
 }
 
-// YandexHandler returns the [http.Handler] for yandex.
+// YandexHandler returns the [http.Handler] for yandex.com.
 func YandexHandler() http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Add("Alt-Svc", `h3=":443"`)