Skip to content

Commit

Permalink
CallResource: don't set Content-Type header if status is 204 (#50780) (
Browse files Browse the repository at this point in the history
…#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 480277f)

Co-authored-by: Ben Sully <ben.sully@grafana.com>
  • Loading branch information
2 people authored and Jguer committed Dec 6, 2022
1 parent 2617f63 commit a58c0b5
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pkg/api/plugin_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
}

Expand Down
34 changes: 33 additions & 1 deletion pkg/api/plugins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
})
Expand Down

0 comments on commit a58c0b5

Please sign in to comment.