Plugins: CallResource: Use canonical MIME headers when writing response (#58506)

* Plugins: CallResource: use canonical MIME headers when writing response

* Plugins: add tests for canonical mime headers and Set-Cookie filter

* Removed extra new line
This commit is contained in:
Giuseppe Guerra 2022-11-09 15:47:32 +01:00 committed by GitHub
parent 159607fe6f
commit 33f02839f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 81 additions and 10 deletions

View File

@ -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)

View File

@ -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,
})
}