From b82d26e13078addfa7ac1256494df829c18226d7 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Tue, 8 Nov 2022 12:31:45 +0100 Subject: [PATCH] CallResource: don't set Content-Type header if status is 204 (#50780) (#58362) Grafana's HTTPServer ensures that the Content-Type header is always set in the response to a CallResource call, but when the status code is 204 No Content this shouldn't be done; the body should be empty and no Content-Type header should be set. We ran into this in the Grafana ML plugin where we were sending an empty response with status 204, but the frontend client saw that the content type was JSON and tried to parse it, resulting in an error that made it to the JS console. (cherry picked from commit 480277f6129b61d4f23e85a31b0e46b052cbb498) Co-authored-by: Ben Sully --- pkg/api/plugin_resource.go | 2 +- pkg/api/plugins_test.go | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/pkg/api/plugin_resource.go b/pkg/api/plugin_resource.go index bccbb034ea8..25e2089090e 100644 --- a/pkg/api/plugin_resource.go +++ b/pkg/api/plugin_resource.go @@ -193,7 +193,7 @@ func (hs *HTTPServer) flushStream(stream callResourceClientResponseStream, w htt // Expected that headers and status are only part of first stream if processedStreams == 0 && resp.Headers != nil { // Make sure a content type always is returned in response - if _, exists := resp.Headers["Content-Type"]; !exists { + if _, exists := resp.Headers["Content-Type"]; !exists && resp.Status != http.StatusNoContent { resp.Headers["Content-Type"] = []string{"application/json"} } diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index 043b6f2a56e..e45e0f6c9cb 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -338,10 +338,35 @@ func TestMakePluginResourceRequest(t *testing.T) { } } + require.Equal(t, resp.Header().Get("Content-Type"), "application/json") require.Equal(t, "sandbox", resp.Header().Get("Content-Security-Policy")) require.Empty(t, req.Header.Get(customHeader)) } +func TestMakePluginResourceRequestContentTypeEmpty(t *testing.T) { + pluginClient := &fakePluginClient{ + statusCode: http.StatusNoContent, + } + hs := HTTPServer{ + Cfg: setting.NewCfg(), + log: log.New(), + pluginClient: pluginClient, + } + req := httptest.NewRequest(http.MethodGet, "/", nil) + resp := httptest.NewRecorder() + pCtx := backend.PluginContext{} + err := hs.makePluginResourceRequest(resp, req, pCtx) + require.NoError(t, err) + + for { + if resp.Flushed { + break + } + } + + require.Zero(t, resp.Header().Get("Content-Type")) +} + func callGetPluginAsset(sc *scenarioContext) { sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() } @@ -373,6 +398,8 @@ type fakePluginClient struct { req *backend.CallResourceRequest backend.QueryDataHandlerFunc + + statusCode int } func (c *fakePluginClient) CallResource(_ context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { @@ -384,8 +411,13 @@ func (c *fakePluginClient) CallResource(_ context.Context, req *backend.CallReso return err } + statusCode := http.StatusOK + if c.statusCode != 0 { + statusCode = c.statusCode + } + return sender.Send(&backend.CallResourceResponse{ - Status: http.StatusOK, + Status: statusCode, Headers: make(map[string][]string), Body: bytes, })