fix(httpapi): don't return nil, nil with null JSON input

While working on ooni/probe#2396, I noticed
that the httpapi code could return nil, nil when the input from the
server was the `null` UTF-8 string. In such a case, golang translates
the `null` string into a `nil` pointer. However, if we pass the
pointer to a struct rather than the pointer to a pointer, the code
returns an empty struct, which is much safer.

This reasoning works as long as we assume the caller wants to get
a struct by pointer or nil on error. If the caller for some reason
wants the code to return a `**T` type, we're back in the case where
the code can return `nil`, `nil`. But all our code always wants
a `*T` back and this seems the only case that makes sense.

internal/httpapi/call_test.go

@@ -1014,6 +1014,11 @@ func TestCallWithJSON(t *testing.T) {
 		bodyToReturn: []byte(`{`),
 		want:         nil,
 		wantErr:      errors.New("unexpected end of JSON input"),
+	}, {
+		name:         "with literal null response",
+		bodyToReturn: []byte(`null`),
+		want:         &CallStructResponse{},
+		wantErr:      nil,
 	}, {
 		name:         "with good response",
 		bodyToReturn: []byte(`{"Name": "sbs", "AgeSquared": 1156}`),
@@ -1053,7 +1058,7 @@ func TestCallWithJSON(t *testing.T) {
 				Request: &RequestDescriptor[*CallStructRequest]{
 					Body: []byte(`{"Name": "sbs", "Age": 34}`),
 				},
-				Response: &JSONResponseDescriptor[*CallStructResponse]{},
+				Response: &JSONResponseDescriptor[CallStructResponse]{},
 				Timeout:  0,
 				URLPath:  "/",
 				URLQuery: nil,

internal/httpapi/descriptor.go

@@ -43,12 +43,22 @@ func (r *RawResponseDescriptor) Unmarshal(resp *http.Response, data []byte) ([]b
 type JSONResponseDescriptor[T any] struct{}
 
 // Unmarshal implements ResponseDescriptor
-func (r *JSONResponseDescriptor[T]) Unmarshal(resp *http.Response, data []byte) (T, error) {
-	value := *new(T)
+func (r *JSONResponseDescriptor[T]) Unmarshal(resp *http.Response, data []byte) (*T, error) {
+	// Important safety note: this implementation is tailored so that, when
+	// the raw JSON body is `null`, we DO NOT return `nil`, `nil`. Because
+	// we create a T on the stack and then let it escape, in such a case the
+	// code will instead return an empty T and nil. Returning an empty T is
+	// slightly better because the caller does not need to worry about the
+	// returned pointer also being nil, but they just need to worry about
+	// whether any field inside the returned struct is the zero value.
+	//
+	// Because this safety property is important, there is also a test that
+	// makes sure we don't return `nil`, `nil` with `null` input.
+	var value T
 	if err := json.Unmarshal(data, &value); err != nil {
-		return *new(T), err
+		return nil, err
 	}
-	return value, nil
+	return &value, nil
 }
 
 // Descriptor contains the parameters for calling a given HTTP

internal/ooapi/checkin.go

@@ -31,7 +31,7 @@ func NewDescriptorCheckIn(
 		Request: &httpapi.RequestDescriptor[*model.OOAPICheckInConfig]{
 			Body: rawRequest,
 		},
-		Response: &httpapi.JSONResponseDescriptor[*model.OOAPICheckInResult]{},
+		Response: &httpapi.JSONResponseDescriptor[model.OOAPICheckInResult]{},
 		Timeout:  0,
 		URLPath:  "/api/v1/check-in",
 		URLQuery: nil,

internal/ooapi/th.go

@@ -31,7 +31,7 @@ func NewDescriptorTH(
 		Request: &httpapi.RequestDescriptor[*model.THRequest]{
 			Body: rawRequest,
 		},
-		Response: &httpapi.JSONResponseDescriptor[*model.THResponse]{},
+		Response: &httpapi.JSONResponseDescriptor[model.THResponse]{},
 		Timeout:  0,
 		URLPath:  "/",
 		URLQuery: nil,