From 4d4597c79df80a336edd15d67377b4c6eb396b10 Mon Sep 17 00:00:00 2001 From: John Maguire Date: Wed, 29 Jul 2026 13:20:15 -0400 Subject: [PATCH 1/3] Omit zero-valued data from marshalled APIResponse The API never sends data alongside errors, but Go test servers marshalling an error-only APIResponse emitted a zero-valued data object. Its empty network curve failed NetworkCurve validation on the client, turning the intended API errors into a JSON decode failure. --- message/message.go | 9 +++++++-- message/message_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 message/message_test.go diff --git a/message/message.go b/message/message.go index 8bceb09..57c6ce4 100644 --- a/message/message.go +++ b/message/message.go @@ -151,9 +151,14 @@ type ReauthenticateResponse struct { } // APIResponse is a standard format for the DN API. It does not apply to the DNClient API. +// The API sends exactly one of data or errors, so marshalling omits whichever +// is unset. Data requires omitzero (not omitempty) because omitempty never +// omits a struct: a zero-valued Data emitted alongside errors has fields that +// can fail validation on decode (such as NetworkCurve), masking the errors the +// response was meant to carry. type APIResponse[T any] struct { - Data T `json:"data"` - Errors APIResponseErrors `json:"errors"` + Data T `json:"data,omitzero"` + Errors APIResponseErrors `json:"errors,omitempty"` } // APIResponseError represents a single error returned in an API error response. diff --git a/message/message_test.go b/message/message_test.go new file mode 100644 index 0000000..e4c76f4 --- /dev/null +++ b/message/message_test.go @@ -0,0 +1,41 @@ +package message + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" +) + +// An error-only APIResponse must round-trip without emitting a zero-valued +// data object, whose empty curve would fail NetworkCurve validation on decode +// and mask the errors. +func TestAPIResponseErrorOnlyRoundTrip(t *testing.T) { + b, err := json.Marshal(APIResponse[EnrollResponseData]{ + Errors: APIResponseErrors{{Code: "ERR_TEST", Message: "test error"}}, + }) + require.NoError(t, err) + require.NotContains(t, string(b), `"data"`) + + var r APIResponse[EnrollResponseData] + require.NoError(t, json.Unmarshal(b, &r)) + require.EqualError(t, r.Errors.Err(), "test error") +} + +// A data-only APIResponse must omit the errors key, matching the API. +func TestAPIResponseDataOnlyOmitsErrors(t *testing.T) { + b, err := json.Marshal(APIResponse[EnrollResponseData]{ + Data: EnrollResponseData{ + HostID: "foobar", + // a valid curve is required for the decode below, as on the real API + Network: HostNetworkMetadata{Curve: NetworkCurve25519}, + }, + }) + require.NoError(t, err) + require.NotContains(t, string(b), `"errors"`) + + var r APIResponse[EnrollResponseData] + require.NoError(t, json.Unmarshal(b, &r)) + require.NoError(t, r.Errors.Err()) + require.Equal(t, "foobar", r.Data.HostID) +} From 2a5f8b85a053f36681921df983f9c7007da01403 Mon Sep 17 00:00:00 2001 From: John Maguire Date: Sun, 18 Oct 2026 02:54:13 -0400 Subject: [PATCH 2/3] Trim APIResponse and test comments --- message/message.go | 6 +----- message/message_test.go | 4 ---- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/message/message.go b/message/message.go index 57c6ce4..e76b33a 100644 --- a/message/message.go +++ b/message/message.go @@ -151,11 +151,7 @@ type ReauthenticateResponse struct { } // APIResponse is a standard format for the DN API. It does not apply to the DNClient API. -// The API sends exactly one of data or errors, so marshalling omits whichever -// is unset. Data requires omitzero (not omitempty) because omitempty never -// omits a struct: a zero-valued Data emitted alongside errors has fields that -// can fail validation on decode (such as NetworkCurve), masking the errors the -// response was meant to carry. +// The API sends exactly one of data or errors. type APIResponse[T any] struct { Data T `json:"data,omitzero"` Errors APIResponseErrors `json:"errors,omitempty"` diff --git a/message/message_test.go b/message/message_test.go index e4c76f4..57fd119 100644 --- a/message/message_test.go +++ b/message/message_test.go @@ -7,9 +7,6 @@ import ( "github.com/stretchr/testify/require" ) -// An error-only APIResponse must round-trip without emitting a zero-valued -// data object, whose empty curve would fail NetworkCurve validation on decode -// and mask the errors. func TestAPIResponseErrorOnlyRoundTrip(t *testing.T) { b, err := json.Marshal(APIResponse[EnrollResponseData]{ Errors: APIResponseErrors{{Code: "ERR_TEST", Message: "test error"}}, @@ -22,7 +19,6 @@ func TestAPIResponseErrorOnlyRoundTrip(t *testing.T) { require.EqualError(t, r.Errors.Err(), "test error") } -// A data-only APIResponse must omit the errors key, matching the API. func TestAPIResponseDataOnlyOmitsErrors(t *testing.T) { b, err := json.Marshal(APIResponse[EnrollResponseData]{ Data: EnrollResponseData{ From c5675c2618cf8b1079181fef3342893b1cecaee2 Mon Sep 17 00:00:00 2001 From: John Maguire Date: Mon, 19 Oct 2026 17:46:49 -0400 Subject: [PATCH 3/3] Pin the enrollment error message in TestEnroll require.Errorf treats its second argument as a failure message, so the error case passed on the decode error that a zero-valued data object produced. EqualError fails on that and passes on the intended API error. --- client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client_test.go b/client_test.go index 48bc339..37fc0b0 100644 --- a/client_test.go +++ b/client_test.go @@ -173,7 +173,7 @@ func TestEnroll(t *testing.T) { ctx, cancel = context.WithTimeout(context.Background(), 1*time.Second) defer cancel() cfg, pkey, creds, meta, err = client.Enroll(ctx, testutil.NewTestLogger(), code, "") - require.Errorf(t, err, fmt.Sprintf("unexpected error during enrollment: %s", errorMsg)) + require.EqualError(t, err, "unexpected error during enrollment: "+errorMsg) assert.Nil(t, cfg) assert.Nil(t, pkey)