feat(oonimkall): add generic HTTP transaction support

This diff adds generic HTTP transaction support so that @aanorbel can
manage OONI Run v2 URLs without a need to change the engine.

By doing that, we reduce coupling between components.

In the future, we may consider allowing the app to perform more
HTTP-based operations, through this API.

Closes ooni/probe#2693

pkg/oonimkall/httpx.go

@@ -0,0 +1,82 @@
+package oonimkall
+
+//
+// HTTP eXtensions
+//
+
+import (
+	"errors"
+	"net/http"
+
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+// Implementation note: I am keeping this API as simple as possible. Obviously, there
+// is room for improvements and possible caveats. For example:
+//
+// 1. we may want to send a POST request with a body (not yet implemented);
+//
+// 2. we may want to disable failing if status code is not 200 (not yet implemented);
+//
+// 3. we may want to see the response status code (not yet implemented);
+//
+// 4. we may want to efficiently support binary bodies (not yet implemented).
+//
+// If needed, we will adapt the API and implement new features.
+
+// HTTPRequest is an HTTP request to send.
+type HTTPRequest struct {
+	// Method is the MANDATORY request method.
+	Method string
+
+	// URL is the MANDATORY request URL.
+	URL string
+}
+
+// HTTPResponse is an HTTP response.
+type HTTPResponse struct {
+	// Body is the response body.
+	Body string
+}
+
+// HTTPDo performs an HTTP request and returns the response.
+//
+// This method uses the default HTTP client of the session, which is the same
+// client that the OONI engine uses to communicate with the OONI backend.
+//
+// This method throws an exception if the HTTP request status code is not 200.
+func (sess *Session) HTTPDo(ctx *Context, jreq *HTTPRequest) (*HTTPResponse, error) {
+	sess.mtx.Lock()
+	defer sess.mtx.Unlock()
+	return sess.httpDoLocked(ctx, jreq)
+}
+
+func (sess *Session) httpDoLocked(ctx *Context, jreq *HTTPRequest) (*HTTPResponse, error) {
+	clnt := sess.sessp.DefaultHTTPClient()
+
+	req, err := http.NewRequestWithContext(ctx.ctx, jreq.Method, jreq.URL, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	resp, err := clnt.Do(req)
+	if err != nil {
+		return nil, err
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != 200 {
+		return nil, errors.New("xoonirun: HTTP request failed")
+	}
+
+	rawResp, err := netxlite.ReadAllContext(ctx.ctx, resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	jResp := &HTTPResponse{
+		Body: string(rawResp),
+	}
+
+	return jResp, nil
+}

pkg/oonimkall/xoonirun_test.go → pkg/oonimkall/httpx_test.go

@@ -1,7 +1,6 @@
 package oonimkall_test
 
 import (
-	"encoding/json"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -12,57 +11,38 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 	"github.com/ooni/probe-cli/v3/internal/testingx"
+	"github.com/ooni/probe-cli/v3/pkg/oonimkall"
 )
 
-func TestOONIRunFetch(t *testing.T) {
-	t.Run("we can fetch a OONI Run link descriptor", func(t *testing.T) {
-		if testing.Short() {
-			t.Skip("skip test in short mode")
+func TestSessionHTTPDo(t *testing.T) {
+	t.Run("on success", func(t *testing.T) {
+		// Implementation note: because we need to backport this patch to the release/3.18
+		// branch, it would be quite verbose and burdensome use netem to implement this test,
+		// since release/3.18 is lagging behind from master in terms of netemx.
+		const expectedResponseBody = "Hello, World!\r\n"
+
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte(expectedResponseBody))
+		}))
+		defer server.Close()
+
+		req := &oonimkall.HTTPRequest{
+			Method: "GET",
+			URL:    server.URL,
 		}
 
 		sess, err := NewSessionForTesting()
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		rawResp, err := sess.OONIRunFetch(sess.NewContext(), 9408643002)
+		resp, err := sess.HTTPDo(sess.NewContext(), req)
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		expect := map[string]any{
-			"archived":                 false,
-			"descriptor_creation_time": "2023-07-18T15:38:21Z",
-			"descriptor": map[string]any{
-				"author":           "[email protected]",
-				"description":      "We use this OONI Run descriptor for writing integration tests for ooni/probe-cli/v3/pkg/oonimkall.",
-				"description_intl": map[string]any{},
-				"icon":             "",
-				"name":             "OONIMkAll Integration Testing",
-				"name_intl":        map[string]any{},
-				"nettests": []any{
-					map[string]any{
-						"backend_options":           map[string]any{},
-						"inputs":                    []any{string("https://www.example.com/")},
-						"is_background_run_enabled": false,
-						"is_manual_run_enabled":     false,
-						"options":                   map[string]any{},
-						"test_name":                 "web_connectivity",
-					},
-				},
-				"short_description":      "Integration testing descriptor for ooni/probe-cli/v3/pkg/oonimkall.",
-				"short_description_intl": map[string]any{},
-			},
-			"mine":                      false,
-			"translation_creation_time": "2023-07-18T15:38:21Z",
-			"v":                         1.0,
-		}
-
-		var got map[string]any
-		runtimex.Try0(json.Unmarshal([]byte(rawResp), &got))
-		t.Log(got)
-
-		if diff := cmp.Diff(expect, got); diff != "" {
+		if diff := cmp.Diff(expectedResponseBody, resp.Body); diff != "" {
 			t.Fatal(diff)
 		}
 	})
@@ -73,14 +53,17 @@ func TestOONIRunFetch(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		URL := &url.URL{Host: "\t"} // this URL is invalid
+		req := &oonimkall.HTTPRequest{
+			Method: "GET",
+			URL:    "\t", // this URL is invalid
+		}
 
-		rawResp, err := sess.OONIRunFetchWithURL(sess.NewContext(), URL)
-		if !strings.HasSuffix(err.Error(), `invalid URL escape "%09"`) {
+		resp, err := sess.HTTPDo(sess.NewContext(), req)
+		if !strings.HasSuffix(err.Error(), `invalid control character in URL`) {
 			t.Fatal("unexpected error", err)
 		}
-		if rawResp != "" {
-			t.Fatal("expected empty raw response")
+		if resp != nil {
+			t.Fatal("expected nil response")
 		}
 	})
 
@@ -90,19 +73,22 @@ func TestOONIRunFetch(t *testing.T) {
 		}))
 		defer server.Close()
 
-		URL := runtimex.Try1(url.Parse(server.URL))
+		req := &oonimkall.HTTPRequest{
+			Method: "GET",
+			URL:    server.URL,
+		}
 
 		sess, err := NewSessionForTesting()
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		rawResp, err := sess.OONIRunFetchWithURL(sess.NewContext(), URL)
+		resp, err := sess.HTTPDo(sess.NewContext(), req)
 		if !strings.HasSuffix(err.Error(), "HTTP request failed") {
 			t.Fatal("unexpected error", err)
 		}
-		if rawResp != "" {
-			t.Fatal("expected empty raw response")
+		if resp != nil {
+			t.Fatal("expected nil response")
 		}
 	})
 
@@ -119,17 +105,22 @@ func TestOONIRunFetch(t *testing.T) {
 			Path:   "/",
 		}
 
+		req := &oonimkall.HTTPRequest{
+			Method: "GET",
+			URL:    URL.String(),
+		}
+
 		sess, err := NewSessionForTesting()
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		rawResp, err := sess.OONIRunFetchWithURL(sess.NewContext(), URL)
+		resp, err := sess.HTTPDo(sess.NewContext(), req)
 		if !strings.HasSuffix(err.Error(), "connection_reset") {
 			t.Fatal("unexpected error", err)
 		}
-		if rawResp != "" {
-			t.Fatal("expected empty raw response")
+		if resp != nil {
+			t.Fatal("expected nil response")
 		}
 	})
 
@@ -149,19 +140,22 @@ func TestOONIRunFetch(t *testing.T) {
 		}))
 		defer server.Close()
 
-		URL := runtimex.Try1(url.Parse(server.URL))
+		req := &oonimkall.HTTPRequest{
+			Method: "GET",
+			URL:    server.URL,
+		}
 
 		sess, err := NewSessionForTesting()
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		rawResp, err := sess.OONIRunFetchWithURL(sess.NewContext(), URL)
+		resp, err := sess.HTTPDo(sess.NewContext(), req)
 		if !strings.HasSuffix(err.Error(), "connection_reset") {
 			t.Fatal("unexpected error", err)
 		}
-		if rawResp != "" {
-			t.Fatal("expected empty raw response")
+		if resp != nil {
+			t.Fatal("expected nil response")
 		}
 	})
 }