diff --git a/pkg/api/plugin_resource.go b/pkg/api/plugin_resource.go index 25e2089090e..c73941a087b 100644 --- a/pkg/api/plugin_resource.go +++ b/pkg/api/plugin_resource.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "net/textproto" "net/url" "sync" @@ -191,17 +192,22 @@ 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 && resp.Status != http.StatusNoContent { - resp.Headers["Content-Type"] = []string{"application/json"} - } - + if processedStreams == 0 { + var hasContentType bool for k, values := range resp.Headers { - // Due to security reasons we don't want to forward - // cookies from a backend plugin to clients/browsers. - if k == "Set-Cookie" { + // Convert the keys to the canonical format of MIME headers. + // This ensures that we can safely add/overwrite headers + // even if the plugin returns them in non-canonical format + // and be sure they won't be present multiple times in the response. + k = textproto.CanonicalMIMEHeaderKey(k) + + switch k { + case "Set-Cookie": + // Due to security reasons we don't want to forward + // cookies from a backend plugin to clients/browsers. continue + case "Content-Type": + hasContentType = true } for _, v := range values { @@ -211,6 +217,11 @@ func (hs *HTTPServer) flushStream(stream callResourceClientResponseStream, w htt } } + // Make sure a content type always is returned in response + if !hasContentType && resp.Status != http.StatusNoContent { + w.Header().Set("Content-Type", "application/json") + } + proxyutil.SetProxyResponseHeaders(w.Header()) w.WriteHeader(resp.Status) diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index ab9a5f977c2..b32d81f7365 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -336,6 +336,65 @@ func TestMakePluginResourceRequest(t *testing.T) { require.Empty(t, req.Header.Get(customHeader)) } +func TestMakePluginResourceRequestSetCookieNotPresent(t *testing.T) { + hs := HTTPServer{ + Cfg: setting.NewCfg(), + log: log.New(), + pluginClient: &fakePluginClient{ + headers: map[string][]string{"Set-Cookie": {"monster"}}, + }, + } + 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 + } + } + assert.Empty(t, resp.Header().Values("Set-Cookie"), "Set-Cookie header should not be present") +} + +func TestMakePluginResourceRequestContentTypeUnique(t *testing.T) { + // Ensures Content-Type is present only once, even if it's present with + // a non-canonical key in the plugin response. + + // Test various upper/lower case combinations for content-type that may be returned by the plugin. + for _, ctHeader := range []string{"content-type", "Content-Type", "CoNtEnT-TyPe"} { + t.Run(ctHeader, func(t *testing.T) { + hs := HTTPServer{ + Cfg: setting.NewCfg(), + log: log.New(), + pluginClient: &fakePluginClient{ + headers: map[string][]string{ + // This should be "overwritten" by the HTTP server + ctHeader: {"application/json"}, + + // Another header that should still be present + "x-another": {"hello"}, + }, + }, + } + 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 + } + } + assert.Len(t, resp.Header().Values("Content-Type"), 1, "should have 1 Content-Type header") + assert.Len(t, resp.Header().Values("x-another"), 1, "should have 1 X-Another header") + }) + } +} + func TestMakePluginResourceRequestContentTypeEmpty(t *testing.T) { pluginClient := &fakePluginClient{ statusCode: http.StatusNoContent, @@ -393,6 +452,7 @@ type fakePluginClient struct { backend.QueryDataHandlerFunc statusCode int + headers map[string][]string } func (c *fakePluginClient) CallResource(_ context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { @@ -411,7 +471,7 @@ func (c *fakePluginClient) CallResource(_ context.Context, req *backend.CallReso return sender.Send(&backend.CallResourceResponse{ Status: statusCode, - Headers: make(map[string][]string), + Headers: c.headers, Body: bytes, }) }