cleanup: merge httpheader and httpfailure into model (#758)

These two small packages could easily be merged into the model package, since they're clearly model-like packages. Part of ooni/probe#2115
ooni · May 25, 2022 · 2d721ba · 2d721ba
1 parent 928de50
commit 2d721ba
Show file tree

Hide file tree

Showing 20 changed files with 74 additions and 90 deletions.
diff --git a/Readme.md b/Readme.md
@@ -18,8 +18,9 @@ This repository contains core OONI tools written in Go:
 
 Every top-level directory in this repository contains an explanatory README file. You
 may also notice that some internal packages live under [internal/engine](internal/engine)
-while most others are top-level. This is part of a long-standing refactoring started
-when we merged https://github.com/ooni/probe-engine into this repository. We'll slowly
+while most others are top-level. This is part of [a long-standing refactoring](
+https://github.com/ooni/probe/issues/2115) started when we merged
+https://github.com/ooni/probe-engine into this repository. We'll slowly
 ensure that all packages inside `engine` are moved out of it and inside `internal`.
 
 ## Semantic versioning policy

diff --git a/internal/cmd/oohelper/internal/client.go b/internal/cmd/oohelper/internal/client.go
@@ -12,7 +12,7 @@ import (
 
 	"github.com/apex/log"
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity"
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
+	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 	"github.com/ooni/probe-cli/v3/internal/version"
@@ -105,9 +105,9 @@ func (oo OOClient) Do(ctx context.Context, config OOConfig) (*CtrlResponse, erro
 	creq := ctrlRequest{
 		HTTPRequest: config.TargetURL,
 		HTTPRequestHeaders: map[string][]string{
-			"Accept":          {httpheader.Accept()},
-			"Accept-Language": {httpheader.AcceptLanguage()},
-			"User-Agent":      {httpheader.UserAgent()},
+			"Accept":          {model.HTTPHeaderAccept},
+			"Accept-Language": {model.HTTPHeaderAcceptLanguage},
+			"User-Agent":      {model.HTTPHeaderUserAgent},
 		},
 		TCPConnect: endpoints,
 	}

diff --git a/internal/engine/experiment/hhfm/hhfm.go b/internal/engine/experiment/hhfm/hhfm.go
@@ -15,7 +15,6 @@ import (
 	"time"
 
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter"
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx/archival"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
@@ -121,12 +120,12 @@ func (m Measurer) Run(
 		return err
 	}
 	headers := map[string]string{
-		randx.ChangeCapitalization("Accept"):          httpheader.Accept(),
+		randx.ChangeCapitalization("Accept"):          model.HTTPHeaderAccept,
 		randx.ChangeCapitalization("Accept-Charset"):  "ISO-8859-1,utf-8;q=0.7,*;q=0.3",
 		randx.ChangeCapitalization("Accept-Encoding"): "gzip,deflate,sdch",
-		randx.ChangeCapitalization("Accept-Language"): httpheader.AcceptLanguage(),
+		randx.ChangeCapitalization("Accept-Language"): model.HTTPHeaderAcceptLanguage,
 		randx.ChangeCapitalization("Host"):            randx.Letters(15) + ".com",
-		randx.ChangeCapitalization("User-Agent"):      httpheader.UserAgent(),
+		randx.ChangeCapitalization("User-Agent"):      model.HTTPHeaderUserAgent,
 	}
 	for key, value := range headers {
 		// Implementation note: Golang will normalize the header names. We will use

diff --git a/internal/engine/experiment/urlgetter/runner.go b/internal/engine/experiment/urlgetter/runner.go
@@ -9,8 +9,8 @@ import (
 	"net/http/cookiejar"
 	"net/url"
 
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx"
+	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
@@ -55,7 +55,7 @@ func (r Runner) Run(ctx context.Context) error {
 // returns httpheader.RandomUserAgent().
 func MaybeUserAgent(ua string) string {
 	if ua == "" {
-		ua = httpheader.UserAgent()
+		ua = model.HTTPHeaderUserAgent
 	}
 	return ua
 }
@@ -65,8 +65,8 @@ func (r Runner) httpGet(ctx context.Context, url string) error {
 	req, err := http.NewRequest(r.Config.Method, url, nil)
 	runtimex.PanicOnError(err, "http.NewRequest failed")
 	req = req.WithContext(ctx)
-	req.Header.Set("Accept", httpheader.Accept())
-	req.Header.Set("Accept-Language", httpheader.AcceptLanguage())
+	req.Header.Set("Accept", model.HTTPHeaderAccept)
+	req.Header.Set("Accept-Language", model.HTTPHeaderAcceptLanguage)
 	req.Header.Set("User-Agent", MaybeUserAgent(r.Config.UserAgent))
 	if r.Config.HTTPHost != "" {
 		req.Host = r.Config.HTTPHost

diff --git a/internal/engine/experiment/urlgetter/runner_test.go b/internal/engine/experiment/urlgetter/runner_test.go
@@ -10,7 +10,7 @@ import (
 
 	"github.com/ooni/probe-cli/v3/internal/atomicx"
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter"
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
+	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
 func TestRunnerWithInvalidURLScheme(t *testing.T) {
@@ -260,7 +260,7 @@ func TestRunnerWeCanForceUserAgent(t *testing.T) {
 }
 
 func TestRunnerDefaultUserAgent(t *testing.T) {
-	expected := httpheader.UserAgent()
+	expected := model.HTTPHeaderUserAgent
 	found := &atomicx.Int64{}
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.Header.Get("User-Agent") == expected {

diff --git a/internal/engine/experiment/webconnectivity/webconnectivity.go b/internal/engine/experiment/webconnectivity/webconnectivity.go
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity/internal"
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx/archival"
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
@@ -174,9 +173,9 @@ func (m Measurer) Run(
 	tk.Control, err = Control(ctx, sess, testhelper.Address, ControlRequest{
 		HTTPRequest: URL.String(),
 		HTTPRequestHeaders: map[string][]string{
-			"Accept":          {httpheader.Accept()},
-			"Accept-Language": {httpheader.AcceptLanguage()},
-			"User-Agent":      {httpheader.UserAgent()},
+			"Accept":          {model.HTTPHeaderAccept},
+			"Accept-Language": {model.HTTPHeaderAcceptLanguage},
+			"User-Agent":      {model.HTTPHeaderUserAgent},
 		},
 		TCPConnect: epnts.Endpoints(),
 	})

diff --git a/internal/engine/experiment/whatsapp/whatsapp.go b/internal/engine/experiment/whatsapp/whatsapp.go
@@ -13,7 +13,6 @@ import (
 	"time"
 
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/httpfailure"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
@@ -109,11 +108,11 @@ func (tk *TestKeys) Update(v urlgetter.MultiOutput) {
 		if failure != nil {
 			// nothing to do here
 		} else if v.TestKeys.HTTPResponseStatus != 302 {
-			failure = &httpfailure.UnexpectedStatusCode
+			failure = &model.HTTPUnexpectedStatusCode
 		} else if len(v.TestKeys.HTTPResponseLocations) != 1 {
-			failure = &httpfailure.UnexpectedRedirectURL
+			failure = &model.HTTPUnexpectedRedirectURL
 		} else if v.TestKeys.HTTPResponseLocations[0] != WebHTTPSURL {
-			failure = &httpfailure.UnexpectedRedirectURL
+			failure = &model.HTTPUnexpectedRedirectURL
 		}
 		tk.WhatsappHTTPFailure = failure
 	}

diff --git a/internal/engine/experiment/whatsapp/whatsapp_test.go b/internal/engine/experiment/whatsapp/whatsapp_test.go
@@ -12,7 +12,6 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/atomicx"
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter"
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/whatsapp"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/httpfailure"
 	"github.com/ooni/probe-cli/v3/internal/engine/mockable"
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
@@ -414,7 +413,7 @@ func TestTestKeysOnlyWebHTTPFailureNo302(t *testing.T) {
 	if tk.WhatsappEndpointsStatus != "ok" {
 		t.Fatal("invalid WhatsappEndpointsStatus")
 	}
-	if *tk.WhatsappWebFailure != httpfailure.UnexpectedStatusCode {
+	if *tk.WhatsappWebFailure != model.HTTPUnexpectedStatusCode {
 		t.Fatal("invalid WhatsappWebFailure")
 	}
 	if tk.WhatsappWebStatus != "blocked" {
@@ -459,7 +458,7 @@ func TestTestKeysOnlyWebHTTPFailureNoLocations(t *testing.T) {
 	if tk.WhatsappEndpointsStatus != "ok" {
 		t.Fatal("invalid WhatsappEndpointsStatus")
 	}
-	if *tk.WhatsappWebFailure != httpfailure.UnexpectedRedirectURL {
+	if *tk.WhatsappWebFailure != model.HTTPUnexpectedRedirectURL {
 		t.Fatal("invalid WhatsappWebFailure")
 	}
 	if tk.WhatsappWebStatus != "blocked" {
@@ -504,7 +503,7 @@ func TestTestKeysOnlyWebHTTPFailureNotExpectedURL(t *testing.T) {
 	if tk.WhatsappEndpointsStatus != "ok" {
 		t.Fatal("invalid WhatsappEndpointsStatus")
 	}
-	if *tk.WhatsappWebFailure != httpfailure.UnexpectedRedirectURL {
+	if *tk.WhatsappWebFailure != model.HTTPUnexpectedRedirectURL {
 		t.Fatal("invalid WhatsappWebFailure")
 	}
 	if tk.WhatsappWebStatus != "blocked" {
@@ -549,7 +548,7 @@ func TestTestKeysOnlyWebHTTPFailureTooManyURLs(t *testing.T) {
 	if tk.WhatsappEndpointsStatus != "ok" {
 		t.Fatal("invalid WhatsappEndpointsStatus")
 	}
-	if *tk.WhatsappWebFailure != httpfailure.UnexpectedRedirectURL {
+	if *tk.WhatsappWebFailure != model.HTTPUnexpectedRedirectURL {
 		t.Fatal("invalid WhatsappWebFailure")
 	}
 	if tk.WhatsappWebStatus != "blocked" {

diff --git a/internal/engine/geolocate/cloudflare.go b/internal/engine/geolocate/cloudflare.go
@@ -6,7 +6,6 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
 	"github.com/ooni/probe-cli/v3/internal/httpx"
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
@@ -21,7 +20,7 @@ func cloudflareIPLookup(
 		BaseURL:    "https://www.cloudflare.com",
 		HTTPClient: httpClient,
 		Logger:     logger,
-		UserAgent:  httpheader.CLIUserAgent(),
+		UserAgent:  model.HTTPHeaderUserAgent,
 	}).WithBodyLogging().Build().FetchResource(ctx, "/cdn-cgi/trace")
 	if err != nil {
 		return DefaultProbeIP, err

diff --git a/internal/engine/geolocate/cloudflare_test.go b/internal/engine/geolocate/cloudflare_test.go
@@ -7,15 +7,15 @@ import (
 	"testing"
 
 	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
+	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
 func TestIPLookupWorksUsingcloudlflare(t *testing.T) {
 	ip, err := cloudflareIPLookup(
 		context.Background(),
 		http.DefaultClient,
 		log.Log,
-		httpheader.UserAgent(),
+		model.HTTPHeaderUserAgent,
 	)
 	if err != nil {
 		t.Fatal(err)

diff --git a/internal/engine/geolocate/stun_test.go b/internal/engine/geolocate/stun_test.go
@@ -9,7 +9,7 @@ import (
 	"time"
 
 	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
+	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/pion/stun"
 )
 
@@ -128,7 +128,7 @@ func TestIPLookupWorksUsingSTUNEkiga(t *testing.T) {
 		context.Background(),
 		http.DefaultClient,
 		log.Log,
-		httpheader.UserAgent(),
+		model.HTTPHeaderUserAgent,
 	)
 	if err != nil {
 		t.Fatal(err)
@@ -143,7 +143,7 @@ func TestIPLookupWorksUsingSTUNGoogle(t *testing.T) {
 		context.Background(),
 		http.DefaultClient,
 		log.Log,
-		httpheader.UserAgent(),
+		model.HTTPHeaderUserAgent,
 	)
 	if err != nil {
 		t.Fatal(err)

diff --git a/internal/engine/geolocate/ubuntu_test.go b/internal/engine/geolocate/ubuntu_test.go
@@ -9,7 +9,7 @@ import (
 	"testing"
 
 	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
+	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
 func TestUbuntuParseError(t *testing.T) {
@@ -22,7 +22,7 @@ func TestUbuntuParseError(t *testing.T) {
 			},
 		}},
 		log.Log,
-		httpheader.UserAgent(),
+		model.HTTPHeaderUserAgent,
 	)
 	if err == nil || !strings.HasPrefix(err.Error(), "XML syntax error") {
 		t.Fatalf("not the error we expected: %+v", err)
@@ -37,7 +37,7 @@ func TestIPLookupWorksUsingUbuntu(t *testing.T) {
 		context.Background(),
 		http.DefaultClient,
 		log.Log,
-		httpheader.UserAgent(),
+		model.HTTPHeaderUserAgent,
 	)
 	if err != nil {
 		t.Fatal(err)

diff --git a/internal/engine/httpheader/accept.go b/internal/engine/httpheader/accept.go
diff --git a/internal/engine/httpheader/acceptlanguage.go b/internal/engine/httpheader/acceptlanguage.go
diff --git a/internal/engine/httpheader/useragent.go b/internal/engine/httpheader/useragent.go
diff --git a/internal/engine/internal/httpfailure/httpfailure.go b/internal/engine/internal/httpfailure/httpfailure.go
diff --git a/internal/measurex/http.go b/internal/measurex/http.go
@@ -26,7 +26,6 @@ import (
 	"unicode/utf8"
 
 	"github.com/lucas-clemente/quic-go"
-	"github.com/ooni/probe-cli/v3/internal/engine/httpheader"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
@@ -289,9 +288,9 @@ func NewCookieJar() http.CookieJar {
 // the headers are the ones we use for measuring.
 func NewHTTPRequestHeaderForMeasuring() http.Header {
 	h := http.Header{}
-	h.Set("Accept", httpheader.Accept())
-	h.Set("Accept-Language", httpheader.AcceptLanguage())
-	h.Set("User-Agent", httpheader.UserAgent())
+	h.Set("Accept", model.HTTPHeaderAccept)
+	h.Set("Accept-Language", model.HTTPHeaderAcceptLanguage)
+	h.Set("User-Agent", model.HTTPHeaderUserAgent)
 	return h
 }
 

diff --git a/internal/model/http.go b/internal/model/http.go
@@ -0,0 +1,33 @@
+package model
+
+//
+// Common HTTP definitions.
+//
+
+// Headers we use for measuring.
+const (
+	// HTTPHeaderAccept is the Accept header used for measuring.
+	HTTPHeaderAccept = "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"
+
+	// HTTPHeaderAcceptLanguage is the Accept-Language header used for measuring.
+	HTTPHeaderAcceptLanguage = "en-US,en;q=0.9"
+
+	// HTTPHeaderUserAgent is the User-Agent header used for measuring. The current header
+	// is 13.7% of the browser population as of May 20, 2022 according to the
+	// https://techblog.willshouse.com/2012/01/03/most-common-user-agents/ webpage.
+	HTTPHeaderUserAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.127 Safari/537.36"
+)
+
+// Additional strings used to report HTTP errors. They're currently only used by
+// experiment/whatsapp but may be used by more experiments in the future. They must
+// be addressable (i.e., var and not const) because experiments typically want to
+// take their addresses to fill fields with `string|null` type.
+var (
+	// HTTPUnexpectedStatusCode indicates that we re not getting
+	// the expected (range of) HTTP status code(s).
+	HTTPUnexpectedStatusCode = "http_unexpected_status_code"
+
+	// HTTPUnexpectedRedirectURL indicates that the redirect URL
+	// returned by the server is not the expected one.
+	HTTPUnexpectedRedirectURL = "http_unexpected_redirect_url"
+)