refactor(session.go): replace engine/netx with netxlite

This diff replaces engine/netx code with netxlite code in
the engine/session.go file. To this end, we needed to move
some code from engine/netx to netxlite. While there, we
did review and improve the unit tests.

A notable change in this diff is (or seems to be) that in
engine/session.go we're not filtering for bogons anymore so
that, in principle, we could believe a resolver returning
to us bogon IP addresses for OONI services. However, I did
not bother with changing bogons filtering because the
sessionresolver package is already filtering for bogons,
so it is actually okay to avoid doing that again the
session.go code. See:

https://github.com/ooni/probe-cli/blob/v3.15.0-alpha.1/internal/engine/internal/sessionresolver/resolvermaker.go#L88

There are two reference issues for this cleanup:

1. ooni/probe#2115

2. ooni/probe#2121

internal/bytecounter/conn.go

@@ -1,5 +1,9 @@
 package bytecounter
 
+//
+// Code to wrap a net.Conn
+//
+
 import "net"
 
 // Conn wraps a network connection and counts bytes.

internal/bytecounter/context.go

@@ -1,5 +1,9 @@
 package bytecounter
 
+//
+// Implicit byte counting based on context
+//
+
 import (
 	"context"
 	"net"

internal/bytecounter/bytecounter.go → internal/bytecounter/counter.go

@@ -1,7 +1,9 @@
-// Package bytecounter contains code to track the number of
-// bytes sent and received by a probe.
 package bytecounter
 
+//
+// Implementation of Counter
+//
+
 import "github.com/ooni/probe-cli/v3/internal/atomicx"
 
 // Counter counts bytes sent and received.

internal/bytecounter/doc.go

@@ -0,0 +1,3 @@
+// Package bytecounter contains code to track the number of
+// bytes sent and received by a probe.
+package bytecounter

internal/bytecounter/http.go

@@ -0,0 +1,101 @@
+package bytecounter
+
+import (
+	"io"
+	"net/http"
+
+	"github.com/ooni/probe-cli/v3/internal/model"
+)
+
+// HTTPTransport is a model.HTTPTransport that counts bytes.
+type HTTPTransport struct {
+	HTTPTransport model.HTTPTransport
+	Counter       *Counter
+}
+
+// NewHTTPTransport creates a new byte-counting-aware HTTP transport.
+func NewHTTPTransport(txp model.HTTPTransport, counter *Counter) model.HTTPTransport {
+	return &HTTPTransport{
+		HTTPTransport: txp,
+		Counter:       counter,
+	}
+}
+
+var _ model.HTTPTransport = &HTTPTransport{}
+
+// CloseIdleConnections implements model.HTTPTransport.CloseIdleConnections.
+func (txp *HTTPTransport) CloseIdleConnections() {
+	txp.HTTPTransport.CloseIdleConnections()
+}
+
+// RoundTrip implements model.HTTPTRansport.RoundTrip
+func (txp *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	if req.Body != nil {
+		req.Body = &httpBodyWrapper{
+			account: txp.Counter.CountBytesSent,
+			rc:      req.Body,
+		}
+	}
+	txp.estimateRequestMetadata(req)
+	resp, err := txp.HTTPTransport.RoundTrip(req)
+	if err != nil {
+		return nil, err
+	}
+	txp.estimateResponseMetadata(resp)
+	resp.Body = &httpBodyWrapper{
+		account: txp.Counter.CountBytesReceived,
+		rc:      resp.Body,
+	}
+	return resp, nil
+}
+
+// Network implements model.HTTPTransport.Network.
+func (txp *HTTPTransport) Network() string {
+	return txp.HTTPTransport.Network()
+}
+
+func (txp *HTTPTransport) estimateRequestMetadata(req *http.Request) {
+	txp.Counter.CountBytesSent(len(req.Method))
+	txp.Counter.CountBytesSent(len(req.URL.String()))
+	for key, values := range req.Header {
+		for _, value := range values {
+			txp.Counter.CountBytesSent(len(key))
+			txp.Counter.CountBytesSent(len(": "))
+			txp.Counter.CountBytesSent(len(value))
+			txp.Counter.CountBytesSent(len("\r\n"))
+		}
+	}
+	txp.Counter.CountBytesSent(len("\r\n"))
+}
+
+func (txp *HTTPTransport) estimateResponseMetadata(resp *http.Response) {
+	txp.Counter.CountBytesReceived(len(resp.Status))
+	for key, values := range resp.Header {
+		for _, value := range values {
+			txp.Counter.CountBytesReceived(len(key))
+			txp.Counter.CountBytesReceived(len(": "))
+			txp.Counter.CountBytesReceived(len(value))
+			txp.Counter.CountBytesReceived(len("\r\n"))
+		}
+	}
+	txp.Counter.CountBytesReceived(len("\r\n"))
+}
+
+type httpBodyWrapper struct {
+	account func(int)
+	rc      io.ReadCloser
+}
+
+var _ io.ReadCloser = &httpBodyWrapper{}
+
+func (r *httpBodyWrapper) Read(p []byte) (int, error) {
+	count, err := r.rc.Read(p)
+	if count > 0 {
+		r.account(count)
+	}
+	return count, err
+}
+
+func (r *httpBodyWrapper) Close() error {
+	return r.rc.Close()
+}

internal/bytecounter/http_test.go

@@ -0,0 +1,162 @@
+package bytecounter
+
+import (
+	"context"
+	"errors"
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+func TestHTTPTransport(t *testing.T) {
+	t.Run("RoundTrip", func(t *testing.T) {
+		t.Run("failure", func(t *testing.T) {
+			counter := New()
+			txp := &HTTPTransport{
+				Counter: counter,
+				HTTPTransport: &mocks.HTTPTransport{
+					MockRoundTrip: func(req *http.Request) (*http.Response, error) {
+						return nil, io.EOF
+					},
+				},
+			}
+			req, err := http.NewRequest(
+				"POST", "https://www.google.com", strings.NewReader("AAAAAA"))
+			if err != nil {
+				t.Fatal(err)
+			}
+			req.Header.Set("User-Agent", "antani-browser/1.0.0")
+			resp, err := txp.RoundTrip(req)
+			if !errors.Is(err, io.EOF) {
+				t.Fatal("not the error we expected")
+			}
+			if resp != nil {
+				t.Fatal("expected nil response here")
+			}
+			if counter.Sent.Load() != 62 {
+				t.Fatal("expected 62 bytes sent", counter.Sent.Load())
+			}
+			if counter.Received.Load() != 0 {
+				t.Fatal("expected zero bytes received", counter.Received.Load())
+			}
+		})
+
+		t.Run("success", func(t *testing.T) {
+			counter := New()
+			txp := &HTTPTransport{
+				Counter: counter,
+				HTTPTransport: &mocks.HTTPTransport{
+					MockRoundTrip: func(req *http.Request) (*http.Response, error) {
+						resp := &http.Response{
+							Body: io.NopCloser(strings.NewReader("1234567")),
+							Header: http.Header{
+								"Server": []string{"antani/0.1.0"},
+							},
+							Status:     "200 OK",
+							StatusCode: http.StatusOK,
+						}
+						return resp, nil
+					},
+				},
+			}
+			req, err := http.NewRequest(
+				"POST", "https://www.google.com", strings.NewReader("AAAAAA"))
+			if err != nil {
+				t.Fatal(err)
+			}
+			req.Header.Set("User-Agent", "antani-browser/1.0.0")
+			resp, err := txp.RoundTrip(req)
+			if err != nil {
+				t.Fatal(err)
+			}
+			data, err := netxlite.ReadAllContext(context.Background(), resp.Body)
+			if err != nil {
+				t.Fatal(err)
+			}
+			resp.Body.Close()
+			if string(data) != "1234567" {
+				t.Fatal("expected a different body here")
+			}
+			if counter.Sent.Load() != 62 {
+				t.Fatal("expected 62 bytes sent", counter.Sent.Load())
+			}
+			if counter.Received.Load() != 37 {
+				t.Fatal("expected 37 bytes received", counter.Received.Load())
+			}
+		})
+
+		t.Run("success with EOF", func(t *testing.T) {
+			counter := New()
+			txp := &HTTPTransport{
+				Counter: counter,
+				HTTPTransport: &mocks.HTTPTransport{
+					MockRoundTrip: func(req *http.Request) (*http.Response, error) {
+						resp := &http.Response{
+							Body: io.NopCloser(&mocks.Reader{
+								MockRead: func(b []byte) (int, error) {
+									if len(b) < 1 {
+										panic("should not happen")
+									}
+									b[0] = 'A'
+									return 1, io.EOF // we want code to be robust to this
+								},
+							}),
+							Header: http.Header{
+								"Server": []string{"antani/0.1.0"},
+							},
+							Status:     "200 OK",
+							StatusCode: http.StatusOK,
+						}
+						return resp, nil
+					},
+				},
+			}
+			client := &http.Client{Transport: txp}
+			resp, err := client.Get("https://www.google.com")
+			if err != nil {
+				t.Fatal(err)
+			}
+			data, err := netxlite.ReadAllContext(context.Background(), resp.Body)
+			if err != nil {
+				t.Fatal(err)
+			}
+			resp.Body.Close()
+			if string(data) != "A" {
+				t.Fatal("expected a different body here")
+			}
+		})
+	})
+
+	t.Run("CloseIdleConnections", func(t *testing.T) {
+		var called bool
+		child := &mocks.HTTPTransport{
+			MockCloseIdleConnections: func() {
+				called = true
+			},
+		}
+		counter := New()
+		txp := NewHTTPTransport(child, counter)
+		txp.CloseIdleConnections()
+		if !called {
+			t.Fatal("not called")
+		}
+	})
+
+	t.Run("Network", func(t *testing.T) {
+		expected := "antani"
+		child := &mocks.HTTPTransport{
+			MockNetwork: func() string {
+				return expected
+			},
+		}
+		counter := New()
+		txp := NewHTTPTransport(child, counter)
+		if network := txp.Network(); network != expected {
+			t.Fatal("unexpected network", network)
+		}
+	})
+}

internal/engine/netx/dialer/proxy.go

@@ -1,56 +1,5 @@
 package dialer
 
-import (
-	"context"
-	"errors"
-	"net"
-	"net/url"
+import "github.com/ooni/probe-cli/v3/internal/netxlite"
 
-	"github.com/ooni/probe-cli/v3/internal/model"
-	"golang.org/x/net/proxy"
-)
-
-// proxyDialer is a dialer that uses a proxy. If the ProxyURL is not configured, this
-// dialer is a passthrough for the next Dialer in chain. Otherwise, it will internally
-// create a SOCKS5 dialer that will connect to the proxy using the underlying Dialer.
-type proxyDialer struct {
-	model.Dialer
-	ProxyURL *url.URL
-}
-
-// ErrProxyUnsupportedScheme indicates we don't support a protocol scheme.
-var ErrProxyUnsupportedScheme = errors.New("proxy: unsupported scheme")
-
-// DialContext implements Dialer.DialContext
-func (d *proxyDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) {
-	url := d.ProxyURL
-	if url == nil {
-		return d.Dialer.DialContext(ctx, network, address)
-	}
-	if url.Scheme != "socks5" {
-		return nil, ErrProxyUnsupportedScheme
-	}
-	// the code at proxy/socks5.go never fails; see https://git.io/JfJ4g
-	child, _ := proxy.SOCKS5(
-		network, url.Host, nil, &proxyDialerWrapper{d.Dialer})
-	return d.dial(ctx, child, network, address)
-}
-
-func (d *proxyDialer) dial(
-	ctx context.Context, child proxy.Dialer, network, address string) (net.Conn, error) {
-	cd := child.(proxy.ContextDialer) // will work
-	return cd.DialContext(ctx, network, address)
-}
-
-// proxyDialerWrapper is required because SOCKS5 expects a Dialer.Dial type but internally
-// it checks whether DialContext is available and prefers that. So, we need to use this
-// structure to cast our inner Dialer the way in which SOCKS5 likes it.
-//
-// See https://git.io/JfJ4g.
-type proxyDialerWrapper struct {
-	model.Dialer
-}
-
-func (d *proxyDialerWrapper) Dial(network, address string) (net.Conn, error) {
-	panic(errors.New("proxyDialerWrapper.Dial should not be called directly"))
-}
+type proxyDialer = netxlite.MaybeProxyDialer