Fix handling POST requests in the REST VC (#13215)

* Fix handling POST requests in the REST VC * tests for decodeResp
prysmaticlabs · Nov 25, 2023 · 2fbda53 · 2fbda53
1 parent 0498e0a
commit 2fbda53
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 6 deletions.
diff --git a/validator/client/beacon-api/json_rest_handler.go b/validator/client/beacon-api/json_rest_handler.go
@@ -60,9 +60,6 @@ func (c beaconApiJsonRestHandler) Post(
 	if data == nil {
 		return nil, errors.New("data is nil")
 	}
-	if resp == nil {
-		return nil, errors.New("resp is nil")
-	}
 
 	url := c.host + apiEndpoint
 	req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, data)
@@ -95,19 +92,25 @@ func decodeResp(httpResp *http.Response, resp interface{}) (*http2.DefaultErrorJ
 	}
 
 	if httpResp.Header.Get("Content-Type") != api.JsonMediaType {
+		if httpResp.StatusCode == http.StatusOK {
+			return nil, nil
+		}
 		return &http2.DefaultErrorJson{Code: httpResp.StatusCode, Message: string(body)}, nil
 	}
 
 	decoder := json.NewDecoder(bytes.NewBuffer(body))
 	if httpResp.StatusCode != http.StatusOK {
 		errorJson := &http2.DefaultErrorJson{}
-		if err := decoder.Decode(errorJson); err != nil {
+		if err = decoder.Decode(errorJson); err != nil {
 			return nil, errors.Wrapf(err, "failed to decode response body into error json for %s", httpResp.Request.URL)
 		}
 		return errorJson, nil
 	}
-	if err = decoder.Decode(resp); err != nil {
-		return nil, errors.Wrapf(err, "failed to decode response body into json for %s", httpResp.Request.URL)
+	// resp is nil for requests that do not return anything.
+	if resp != nil {
+		if err = decoder.Decode(resp); err != nil {
+			return nil, errors.Wrapf(err, "failed to decode response body into json for %s", httpResp.Request.URL)
+		}
 	}
 
 	return nil, nil

diff --git a/validator/client/beacon-api/json_rest_handler_test.go b/validator/client/beacon-api/json_rest_handler_test.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/prysmaticlabs/prysm/v4/api"
 	"github.com/prysmaticlabs/prysm/v4/beacon-chain/rpc/eth/beacon"
+	http2 "github.com/prysmaticlabs/prysm/v4/network/http"
 	"github.com/prysmaticlabs/prysm/v4/testing/assert"
 	"github.com/prysmaticlabs/prysm/v4/testing/require"
 )
@@ -102,3 +103,76 @@ func TestPost(t *testing.T) {
 	assert.NoError(t, err)
 	assert.DeepEqual(t, genesisJson, resp)
 }
+
+func decodeRespTest(t *testing.T) {
+	type j struct {
+		Foo string `json:"foo"`
+	}
+
+	t.Run("200 non-JSON", func(t *testing.T) {
+		r := &http.Response{StatusCode: http.StatusOK, Header: map[string][]string{"Content-Type": {api.OctetStreamMediaType}}}
+		errJson, err := decodeResp(r, nil)
+		require.Equal(t, true, errJson == nil)
+		require.NoError(t, err)
+	})
+	t.Run("non-200 non-JSON", func(t *testing.T) {
+		body := bytes.Buffer{}
+		_, err := body.WriteString("foo")
+		require.NoError(t, err)
+		r := &http.Response{StatusCode: http.StatusInternalServerError, Header: map[string][]string{"Content-Type": {api.OctetStreamMediaType}}}
+		errJson, err := decodeResp(r, nil)
+		require.NotNil(t, errJson)
+		assert.Equal(t, http.StatusInternalServerError, errJson.Code)
+		assert.Equal(t, "foo", errJson.Message)
+		require.NoError(t, err)
+	})
+	t.Run("200 JSON with resp", func(t *testing.T) {
+		body := bytes.Buffer{}
+		b, err := json.Marshal(&j{Foo: "foo"})
+		require.NoError(t, err)
+		body.Write(b)
+		r := &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(&body), Header: map[string][]string{"Content-Type": {api.JsonMediaType}}}
+		resp := &j{}
+		errJson, err := decodeResp(r, resp)
+		require.Equal(t, true, errJson == nil)
+		require.NoError(t, err)
+		assert.Equal(t, "foo", resp.Foo)
+	})
+	t.Run("200 JSON without resp", func(t *testing.T) {
+		r := &http.Response{StatusCode: http.StatusOK, Header: map[string][]string{"Content-Type": {api.JsonMediaType}}}
+		errJson, err := decodeResp(r, nil)
+		require.Equal(t, true, errJson == nil)
+		require.NoError(t, err)
+	})
+	t.Run("non-200 JSON", func(t *testing.T) {
+		body := bytes.Buffer{}
+		b, err := json.Marshal(&http2.DefaultErrorJson{Code: http.StatusInternalServerError, Message: "error"})
+		require.NoError(t, err)
+		body.Write(b)
+		r := &http.Response{StatusCode: http.StatusInternalServerError, Body: io.NopCloser(&body), Header: map[string][]string{"Content-Type": {api.JsonMediaType}}}
+		errJson, err := decodeResp(r, nil)
+		require.NotNil(t, errJson)
+		assert.Equal(t, http.StatusInternalServerError, errJson.Code)
+		assert.Equal(t, "error", errJson.Message)
+		require.NoError(t, err)
+	})
+	t.Run("200 JSON cannot decode", func(t *testing.T) {
+		body := bytes.Buffer{}
+		_, err := body.WriteString("foo")
+		require.NoError(t, err)
+		r := &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(&body), Header: map[string][]string{"Content-Type": {api.JsonMediaType}}}
+		resp := &j{}
+		errJson, err := decodeResp(r, resp)
+		require.Equal(t, true, errJson == nil)
+		assert.ErrorContains(t, "failed to decode response body into json", err)
+	})
+	t.Run("non-200 JSON cannot decode", func(t *testing.T) {
+		body := bytes.Buffer{}
+		_, err := body.WriteString("foo")
+		require.NoError(t, err)
+		r := &http.Response{StatusCode: http.StatusInternalServerError, Body: io.NopCloser(&body), Header: map[string][]string{"Content-Type": {api.JsonMediaType}}}
+		errJson, err := decodeResp(r, nil)
+		require.Equal(t, true, errJson == nil)
+		assert.ErrorContains(t, "failed to decode response body into error json", err)
+	})
+}