feat(dsl): do not include HTTP body by default (#11)

By making it explicit that we want to include the HTTP body into a
measurement we highlight that including the body should only be done
when the actual content of the body matters. It makes sense, for
example, for Web Connectivity but it does not make sense to do so in
many other contexts.

Noticed when working on ooni/probe#2502

pkg/dsl/httpcore.go

@@ -70,16 +70,17 @@ func (op *httpTransactionOperation) Run(ctx context.Context, rtx Runtime, conn *
 
 	// create configuration
 	config := &httpTransactionConfig{
-		AcceptHeader:             model.HTTPHeaderAccept,
-		AcceptLanguageHeader:     model.HTTPHeaderAcceptLanguage,
-		HostHeader:               conn.Domain,
-		RefererHeader:            "",
-		RequestMethod:            "GET",
-		ResponseBodySnapshotSize: 1 << 19,
-		URLHost:                  conn.Domain,
-		URLPath:                  "/",
-		URLScheme:                conn.Scheme,
-		UserAgentHeader:          model.HTTPHeaderUserAgent,
+		AcceptHeader:                model.HTTPHeaderAccept,
+		AcceptLanguageHeader:        model.HTTPHeaderAcceptLanguage,
+		HostHeader:                  conn.Domain,
+		IncludeResponseBodySnapshot: false,
+		RefererHeader:               "",
+		RequestMethod:               "GET",
+		ResponseBodySnapshotSize:    1 << 19,
+		URLHost:                     conn.Domain,
+		URLPath:                     "/",
+		URLScheme:                   conn.Scheme,
+		UserAgentHeader:             model.HTTPHeaderUserAgent,
 	}
 	for _, option := range op.options {
 		option(config)
@@ -106,7 +107,12 @@ func (op *httpTransactionOperation) Run(ctx context.Context, rtx Runtime, conn *
 
 	// mediate the transaction execution via the trace, which gets a chance
 	// to generate HTTP observations for this transaction
-	resp, body, err := conn.Trace.HTTPTransaction(conn, req, config.ResponseBodySnapshotSize)
+	resp, body, err := conn.Trace.HTTPTransaction(
+		conn,
+		config.IncludeResponseBodySnapshot,
+		req,
+		config.ResponseBodySnapshotSize,
+	)
 
 	// save trace-collected observations (if any)
 	rtx.SaveObservations(conn.Trace.ExtractObservations()...)

pkg/dsl/httpmodel.go

@@ -55,6 +55,14 @@ func HTTPTransactionOptionHost(value string) HTTPTransactionOption {
 	}
 }
 
+// HTTPTransactionOptionIncludeResponseBodySnapshot controls whether to include the
+// response body snapshot in the JSON measurement; the default is false.
+func HTTPTransactionOptionIncludeResponseBodySnapshot(value bool) HTTPTransactionOption {
+	return func(c *httpTransactionConfig) {
+		c.IncludeResponseBodySnapshot = value
+	}
+}
+
 // HTTPTransactionOptionMethod sets the method.
 func HTTPTransactionOptionMethod(value string) HTTPTransactionOption {
 	return func(c *httpTransactionConfig) {
@@ -104,7 +112,7 @@ func HTTPTransactionOptionUserAgent(value string) HTTPTransactionOption {
 	}
 }
 
-// TODO(bassosimone): we should probably autogenerate the config, the functional optionl
+// TODO(bassosimone): we should probably autogenerate the config, the functional optional
 // setters, and the conversion from config to list of options.
 
 type httpTransactionConfig struct {
@@ -117,6 +125,10 @@ type httpTransactionConfig struct {
 	// HostHeader is the host header to use.
 	HostHeader string `json:"host_header,omitempty"`
 
+	// IncludeResponseBodySnapshot tells the engine to include the response body snapshot
+	// we have read inside the JSON measurement.
+	IncludeResponseBodySnapshot bool `json:"include_response_body_snapshot,omitempty"`
+
 	// RefererHeader is the referer header to use.
 	RefererHeader string `json:"referer_header,omitempty"`
 
@@ -149,6 +161,9 @@ func (c *httpTransactionConfig) options() (options []HTTPTransactionOption) {
 	if value := c.HostHeader; value != "" {
 		options = append(options, HTTPTransactionOptionHost(value))
 	}
+	if value := c.IncludeResponseBodySnapshot; value {
+		options = append(options, HTTPTransactionOptionIncludeResponseBodySnapshot(value))
+	}
 	if value := c.RefererHeader; value != "" {
 		options = append(options, HTTPTransactionOptionReferer(value))
 	}

pkg/dsl/measurexlite.go

@@ -97,7 +97,16 @@ var _ Trace = &measurexliteTrace{}
 
 // HTTPTransaction implements Trace.
 func (t *measurexliteTrace) HTTPTransaction(
-	conn *HTTPConnection, req *http.Request, maxBodySnapshotSize int) (*http.Response, []byte, error) {
+	conn *HTTPConnection,
+	includeResponseBodySnapshot bool,
+	req *http.Request,
+	maxBodySnapshotSize int,
+) (*http.Response, []byte, error) {
+	// make sure the response body snapshot size is non-negative
+	if maxBodySnapshotSize < 0 {
+		maxBodySnapshotSize = 0
+	}
+
 	// create the beginning-of-transaction observation
 	started := t.trace.TimeSince(t.trace.ZeroTime)
 	t.runtime.saveNetworkEvents(measurexlite.NewAnnotationArchivalNetworkEvent(
@@ -151,7 +160,7 @@ func (t *measurexliteTrace) HTTPTransaction(
 		req,
 		resp,
 		int64(maxBodySnapshotSize),
-		body,
+		t.maybeIncludeBody(includeResponseBodySnapshot, body),
 		err,
 		finished,
 	))
@@ -166,6 +175,13 @@ func (t *measurexliteTrace) HTTPTransaction(
 	return resp, body, err
 }
 
+func (t *measurexliteTrace) maybeIncludeBody(includeBody bool, body []byte) []byte {
+	if !includeBody {
+		return []byte{}
+	}
+	return body
+}
+
 // Index implements Trace.
 func (t *measurexliteTrace) Index() int64 {
 	return t.trace.Index

pkg/dsl/measurexlite_test.go

@@ -0,0 +1,120 @@
+package dsl
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+	"time"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-engine/pkg/model"
+	"github.com/ooni/probe-engine/pkg/runtimex"
+)
+
+// TestMeasurexliteHTTPIncludeResponseBodySnapshot checks whether we include or not
+// include a body snapshot into the JSON measurement depending on the settings.
+func TestMeasurexliteHTTPIncludeResponseBodySnapshot(t *testing.T) {
+	// define the expected response body
+	expectedBody := []byte("Bonsoir, Elliot!\r\n")
+
+	// create local test server
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write(expectedBody)
+	}))
+	defer server.Close()
+
+	// parse the server's URL
+	URL := runtimex.Try1(url.Parse(server.URL))
+
+	// define the function to create a measurement pipeline
+	makePipeline := func(options ...HTTPTransactionOption) Stage[*Void, *HTTPResponse] {
+		return Compose4(
+			NewEndpoint(URL.Host, NewEndpointOptionDomain("example.com")),
+			TCPConnect(),
+			HTTPConnectionTCP(),
+			HTTPTransaction(options...),
+		)
+	}
+
+	// define the function to return the body in the pipeline
+	getPipelineBody := func(input Maybe[*HTTPResponse]) ([]byte, error) {
+		// handle the case of unexpected error
+		if input.Error != nil {
+			return nil, input.Error
+		}
+		return input.Value.ResponseBodySnapshot, nil
+	}
+
+	// define the function to return the body in the measurement
+	getMeasurementBody := func(observations *Observations) ([]byte, error) {
+		if len(observations.Requests) != 1 {
+			return nil, errors.New("expected a single request entry")
+		}
+		return []byte(observations.Requests[0].Response.Body.Value), nil
+	}
+
+	// define the function to run the measurement
+	measure := func(options ...HTTPTransactionOption) (Maybe[*HTTPResponse], *Observations) {
+		pipeline := makePipeline(options...)
+		rtx := NewMeasurexliteRuntime(model.DiscardLogger, &NullMetrics{}, time.Now())
+		input := NewValue(&Void{})
+		output := pipeline.Run(context.Background(), rtx, input)
+		observations := ReduceObservations(rtx.ExtractObservations()...)
+		return output, observations
+	}
+
+	t.Run("the default should be that of not including the body", func(t *testing.T) {
+		output, observations := measure( /* empty */ )
+
+		t.Run("the pipeline body should contain the body", func(t *testing.T) {
+			pipeBody, err := getPipelineBody(output)
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Log("pipeline body", pipeBody)
+			if diff := cmp.Diff(expectedBody, pipeBody); diff != "" {
+				t.Fatal(diff)
+			}
+		})
+
+		t.Run("the measurement body should be empty", func(t *testing.T) {
+			measBody, err := getMeasurementBody(observations)
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Log("measurement body", measBody)
+			if len(measBody) != 0 {
+				t.Fatal("expected empty body")
+			}
+		})
+	})
+
+	t.Run("but we can optionally request to include it", func(t *testing.T) {
+		output, observations := measure(HTTPTransactionOptionIncludeResponseBodySnapshot(true))
+
+		t.Run("the pipeline body should contain the body", func(t *testing.T) {
+			pipeBody, err := getPipelineBody(output)
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Log("pipeline body", pipeBody)
+			if diff := cmp.Diff(expectedBody, pipeBody); diff != "" {
+				t.Fatal(diff)
+			}
+		})
+
+		t.Run("the measurement body should contain the body", func(t *testing.T) {
+			measBody, err := getMeasurementBody(observations)
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Log("measurement body", measBody)
+			if diff := cmp.Diff(expectedBody, measBody); diff != "" {
+				t.Fatal(diff)
+			}
+		})
+	})
+}

pkg/dsl/metrics.go

@@ -14,7 +14,8 @@ type Metrics interface {
 	Success(name string)
 }
 
-// NullMetrics implements [Metrics] but ignores events.
+// NullMetrics implements [Metrics] but ignores events. The zero value of
+// this structure is ready to use.
 type NullMetrics struct{}
 
 // Error implements Metrics.

pkg/dsl/quicmodel.go

@@ -32,7 +32,7 @@ type QUICConnection struct {
 // QUICHandshakeOption is an option for configuring the QUIC handshake.
 type QUICHandshakeOption func(config *quicHandshakeConfig)
 
-// TODO(bassosimone): we should probably autogenerate the config, the functional optionl
+// TODO(bassosimone): we should probably autogenerate the config, the functional optional
 // setters, and the conversion from config to list of options.
 
 type quicHandshakeConfig struct {

pkg/dsl/runtime.go

@@ -156,7 +156,11 @@ func (t *minimalTrace) ExtractObservations() []*Observations {
 
 // HTTPTransaction implements Trace.
 func (t *minimalTrace) HTTPTransaction(
-	conn *HTTPConnection, req *http.Request, maxBodySnapshotSize int) (*http.Response, []byte, error) {
+	conn *HTTPConnection,
+	includeResponseBodySnapshot bool,
+	req *http.Request,
+	maxBodySnapshotSize int,
+) (*http.Response, []byte, error) {
 	// perform round trip
 	resp, err := conn.Transport.RoundTrip(req)
 	if err != nil {

pkg/dsl/tlsmodel.go

@@ -29,7 +29,7 @@ type TLSConnection struct {
 // TLSHandshakeOption is an option for configuring the TLS handshake.
 type TLSHandshakeOption func(config *tlsHandshakeConfig)
 
-// TODO(bassosimone): we should probably autogenerate the config, the functional optionl
+// TODO(bassosimone): we should probably autogenerate the config, the functional optional
 // setters, and the conversion from config to list of options.
 
 type tlsHandshakeConfig struct {

pkg/dsl/trace.go

@@ -11,9 +11,38 @@ type Trace interface {
 	// ExtractObservations removes and returns the observations saved so far.
 	ExtractObservations() []*Observations
 
-	// HTTPTransaction executes and measures an HTTP transaction. The n argument controls
-	// the maximum response body snapshot size that we are willing to read.
-	HTTPTransaction(c *HTTPConnection, r *http.Request, n int) (*http.Response, []byte, error)
+	// HTTPTransaction executes and measures an HTTP transaction.
+	//
+	// Arguments:
+	//
+	// - conn is the HTTP connection to use;
+	//
+	// - includeResponseBodySnapshot controls whether to include the response body
+	// snapshot into the JSON measurement;
+	//
+	// - request is the HTTP request;
+	//
+	// - responseBodySnapshotSize controls the maximum number of bytes of the
+	// body that we are willing to read (to avoid reading unbounded bodies).
+	//
+	// Return values:
+	//
+	// - resp is the HTTP response;
+	//
+	// - body is the HTTP response body (which MAY be empty if the response body
+	// snapshot size value is zero or negative);
+	//
+	// - err is the error that occurred (nil on success).
+	HTTPTransaction(
+		conn *HTTPConnection,
+		includeResponseBodySnapshot bool,
+		request *http.Request,
+		responseBodySnapshotSize int,
+	) (
+		resp *http.Response,
+		body []byte,
+		err error,
+	)
 
 	// Index is the unique index of this trace.
 	Index() int64