From 7f2aed42a05bcac7fa17b98e76d6081a89bb18c1 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Mon, 30 Jun 2025 15:58:05 +0200 Subject: [PATCH] Plugins: Fix and encode invalid gRPC header values (#107339) * Plugins: Fix and encode invalid gRPC header values * Rename the method * Run sanitizeHTTPHeaderValueForGRPC only if string includes utf8 * Update test * Simplify * Update * do not double encode encoded characters * Update test * Update * Add test case based on review * Update test --- .../tracing_header_middleware.go | 33 ++++++ .../tracing_header_middleware_test.go | 110 ++++++++++++++++++ 2 files changed, 143 insertions(+) diff --git a/pkg/services/pluginsintegration/clientmiddleware/tracing_header_middleware.go b/pkg/services/pluginsintegration/clientmiddleware/tracing_header_middleware.go index ea7237734d8..b8bd1f2c9d9 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/tracing_header_middleware.go +++ b/pkg/services/pluginsintegration/clientmiddleware/tracing_header_middleware.go @@ -2,8 +2,13 @@ package clientmiddleware import ( "context" + "fmt" + "strings" + "unicode/utf8" "github.com/grafana/grafana-plugin-sdk-go/backend" + "golang.org/x/text/encoding/charmap" + "golang.org/x/text/transform" "github.com/grafana/grafana/pkg/services/contexthandler" "github.com/grafana/grafana/pkg/services/query" @@ -50,6 +55,9 @@ func (m *TracingHeaderMiddleware) applyHeaders(ctx context.Context, req backend. if gotVal == "" { continue } + if !utf8.ValidString(gotVal) { + gotVal = sanitizeHTTPHeaderValueForGRPC(gotVal) + } req.SetHTTPHeader(headerName, gotVal) } } @@ -102,3 +110,28 @@ func (m *TracingHeaderMiddleware) RunStream(ctx context.Context, req *backend.Ru m.applyHeaders(ctx, req) return m.BaseHandler.RunStream(ctx, req, sender) } + +// sanitizeHTTPHeaderValueForGRPC sanitizes header values according to HTTP/2 gRPC specification. +// The spec defines that header values must consist of printable ASCII characters 0x20 (space) - 0x7E(tilde) inclusive. +// First attempts to decode any percent-encoded characters, then encodes invalid characters. +func sanitizeHTTPHeaderValueForGRPC(value string) string { + // First try to decode characters that were encoded by the frontend + decoder := charmap.ISO8859_1.NewDecoder() + decoded, _, err := transform.Bytes(decoder, []byte(value)) + // If decoding fails, work with the original value + if err != nil { + decoded = []byte(value) + } + var sanitized strings.Builder + sanitized.Grow(len(decoded)) // Pre-allocate reasonable capacity + // Then encode invalid characters + for _, b := range decoded { + if b >= 0x20 && b <= 0x7E { + sanitized.WriteByte(b) + } else { + sanitized.WriteString(fmt.Sprintf("%%%02X", b)) + } + } + + return sanitized.String() +} diff --git a/pkg/services/pluginsintegration/clientmiddleware/tracing_header_middleware_test.go b/pkg/services/pluginsintegration/clientmiddleware/tracing_header_middleware_test.go index 60435b2b557..3c3b799a358 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/tracing_header_middleware_test.go +++ b/pkg/services/pluginsintegration/clientmiddleware/tracing_header_middleware_test.go @@ -238,5 +238,115 @@ func TestTracingHeaderMiddleware(t *testing.T) { require.Equal(t, `d26e337d-cb53-481a-9212-0112537b3c1a`, cdt.RunStreamReq.GetHTTPHeader(`X-Query-Group-Id`)) require.Equal(t, `true`, cdt.RunStreamReq.GetHTTPHeader(`X-Grafana-From-Expr`)) }) + + t.Run("sanitizes grpc header values for invalid utf-8", func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/some/thing", nil) + require.NoError(t, err) + + // Create invalid UTF-8 strings + invalidUTF8Dashboard := string([]byte{'d', 'a', 's', 'h', 0xFF, 0xFE, 'u', 'i', 'd'}) + invalidUTF8Panel := string([]byte{'p', 'a', 'n', 'e', 'l', 0x80, 'i', 'd'}) + + // Set headers with various characters that need to be sanitization + req.Header[`X-Dashboard-Title`] = []string{invalidUTF8Dashboard} // invalid UTF-8 + req.Header[`X-Panel-Title`] = []string{invalidUTF8Panel} // invalid UTF-8 + + // Set headers that don't need sanitization + req.Header[`X-Dashboard-Uid`] = []string{"dashboard\x00uid"} // control character + req.Header[`X-Datasource-Uid`] = []string{"datasource\tuid"} // tab character + req.Header[`X-Query-Group-Id`] = []string{"valid-text-123"} // valid characters + req.Header[`X-Grafana-From-Expr`] = []string{"café résumé"} // extended characters + + pluginCtx := backend.PluginContext{ + DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{}, + } + + cdt := handlertest.NewHandlerMiddlewareTest(t, + WithReqContext(req, &user.SignedInUser{ + IsAnonymous: true, + Login: "anonymous"}, + ), + handlertest.WithMiddlewares(NewTracingHeaderMiddleware()), + ) + + _, err = cdt.MiddlewareHandler.QueryData(req.Context(), &backend.QueryDataRequest{ + PluginContext: pluginCtx, + Headers: map[string]string{}, + }) + require.NoError(t, err) + + // Invalid UTF-8 should be sanitized + require.Equal(t, "dash%C3%BF%C3%BEuid", cdt.QueryDataReq.GetHTTPHeader(`X-Dashboard-Title`)) + require.Equal(t, "panel%C2%80id", cdt.QueryDataReq.GetHTTPHeader(`X-Panel-Title`)) + + // Valid characters should remain unchanged + require.Equal(t, "valid-text-123", cdt.QueryDataReq.GetHTTPHeader(`X-Query-Group-Id`)) + require.Equal(t, "café résumé", cdt.QueryDataReq.GetHTTPHeader(`X-Grafana-From-Expr`)) + require.Equal(t, "dashboard\x00uid", cdt.QueryDataReq.GetHTTPHeader(`X-Dashboard-Uid`)) + require.Equal(t, "datasource\tuid", cdt.QueryDataReq.GetHTTPHeader(`X-Datasource-Uid`)) + }) }) } + +func TestSanitizeHTTPHeaderValueForGRPC(t *testing.T) { + testCases := []struct { + name string + input string + expected string + }{ + { + name: "Valid printable ASCII characters remain unchanged", + input: "Hello World! 123 @#$%^&*()", + expected: "Hello World! 123 @#$%^&*()", + }, + { + name: "Extended characters remain unchanged", + // %C3%A9 is encoded é + input: "naiv%C3%A9", + expected: "naiv%C3%A9", + }, + { + name: "naivé coming in as an iso8859-1 string", + input: string([]byte{110, 97, 105, 118, 233}), + expected: "naiv%C3%A9", + }, + { + name: "Control characters are percent-encoded", + input: "hello\x00\x01\x1Fworld", + expected: "hello%00%01%1Fworld", + }, + { + name: "Tab character is percent-encoded", + input: "hello\tworld", + expected: "hello%09world", + }, + { + name: "Newline character is percent-encoded", + input: "hello\nworld", + expected: "hello%0Aworld", + }, + { + name: "Carriage return is percent-encoded", + input: "hello\rworld", + expected: "hello%0Dworld", + }, + { + name: "Mixed valid and invalid characters", + // %F0%9F%9A%80 is encoded 🚀 + input: "Valid text\x00invalid\x1Fmore valid %F0%9F%9A%80", + expected: "Valid text%00invalid%1Fmore valid %F0%9F%9A%80", + }, + { + name: "Empty string remains empty", + input: "", + expected: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := sanitizeHTTPHeaderValueForGRPC(tc.input) + require.Equal(t, tc.expected, result) + }) + } +}