From 2fbda536b03b8b32163ee8eab0a38d14dd5832d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Kapka?= Date: Sun, 26 Nov 2023 00:25:14 +0100 Subject: [PATCH] Fix handling POST requests in the REST VC (#13215) * Fix handling POST requests in the REST VC * tests for decodeResp --- .../client/beacon-api/json_rest_handler.go | 15 ++-- .../beacon-api/json_rest_handler_test.go | 74 +++++++++++++++++++ 2 files changed, 83 insertions(+), 6 deletions(-) diff --git a/validator/client/beacon-api/json_rest_handler.go b/validator/client/beacon-api/json_rest_handler.go index 6c1ef2d10e39..f834434db07a 100644 --- 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 index 98200eefa5db..b4c87460e665 100644 --- 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) + }) +}