From 0f38b5bcbfc4f322fbf310e5867244e92988dfd9 Mon Sep 17 00:00:00 2001 From: as992949791 Date: Fri, 30 Oct 2026 12:34:00 +0800 Subject: [PATCH 1/2] fix(http): preserve repeated request configuration headers --- pkg/http/middleware/request_config.go | 8 +++---- pkg/http/middleware/request_config_test.go | 25 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/pkg/http/middleware/request_config.go b/pkg/http/middleware/request_config.go index 6102825192..2c14edaa5c 100644 --- a/pkg/http/middleware/request_config.go +++ b/pkg/http/middleware/request_config.go @@ -26,12 +26,12 @@ func WithRequestConfig(next http.Handler) http.Handler { } // Toolsets - if toolsets := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsetsHeader)); len(toolsets) > 0 { + if toolsets := headers.ParseCommaSeparated(strings.Join(r.Header.Values(headers.MCPToolsetsHeader), ",")); len(toolsets) > 0 { ctx = ghcontext.WithToolsets(ctx, toolsets) } // Tools - if tools := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsHeader)); len(tools) > 0 { + if tools := headers.ParseCommaSeparated(strings.Join(r.Header.Values(headers.MCPToolsHeader), ",")); len(tools) > 0 { ctx = ghcontext.WithTools(ctx, tools) } @@ -41,7 +41,7 @@ func WithRequestConfig(next http.Handler) http.Handler { } // Excluded tools - if excludeTools := headers.ParseCommaSeparated(r.Header.Get(headers.MCPExcludeToolsHeader)); len(excludeTools) > 0 { + if excludeTools := headers.ParseCommaSeparated(strings.Join(r.Header.Values(headers.MCPExcludeToolsHeader), ",")); len(excludeTools) > 0 { ctx = ghcontext.WithExcludeTools(ctx, excludeTools) } @@ -54,7 +54,7 @@ func WithRequestConfig(next http.Handler) http.Handler { _, hasHeaderFeatures := r.Header[http.CanonicalHeaderKey(headers.MCPFeaturesHeader)] _, hasQueryFeatures := query[queryParamFeatures] if hasHeaderFeatures { - ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(r.Header.Get(headers.MCPFeaturesHeader))) + ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(strings.Join(r.Header.Values(headers.MCPFeaturesHeader), ","))) } else if hasQueryFeatures { ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(query.Get(queryParamFeatures))) } diff --git a/pkg/http/middleware/request_config_test.go b/pkg/http/middleware/request_config_test.go index 1937d0986b..ce572d5084 100644 --- a/pkg/http/middleware/request_config_test.go +++ b/pkg/http/middleware/request_config_test.go @@ -1,6 +1,7 @@ package middleware import ( + "context" "net/http" "net/http/httptest" "testing" @@ -10,6 +11,30 @@ import ( "github.com/stretchr/testify/assert" ) +func TestWithRequestConfigRepeatedListHeaders(t *testing.T) { + tests := []struct { + name string + header string + get func(context.Context) []string + }{ + {"toolsets", headers.MCPToolsetsHeader, ghcontext.GetToolsets}, + {"tools", headers.MCPToolsHeader, ghcontext.GetTools}, + {"excluded tools", headers.MCPExcludeToolsHeader, ghcontext.GetExcludeTools}, + {"features", headers.MCPFeaturesHeader, ghcontext.GetHeaderFeatures}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/?features=query_flag", nil) + req.Header.Add(tc.header, "first, second") + req.Header.Add(tc.header, " third, , fourth ") + handler := WithRequestConfig(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + assert.Equal(t, []string{"first", "second", "third", "fourth"}, tc.get(r.Context())) + })) + handler.ServeHTTP(httptest.NewRecorder(), req) + }) + } +} + func TestWithRequestConfigFeatureSelection(t *testing.T) { tests := []struct { name string From 4b907e38600f649ebf9d70a2698d214aaba37517 Mon Sep 17 00:00:00 2001 From: as992949791 Date: Sun, 8 Nov 2026 16:16:06 +0800 Subject: [PATCH 2/2] fix(http): retain repeated boolean and query configuration --- pkg/http/middleware/request_config.go | 8 +-- pkg/http/middleware/request_config_test.go | 79 ++++++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/pkg/http/middleware/request_config.go b/pkg/http/middleware/request_config.go index 2c14edaa5c..5405c378c7 100644 --- a/pkg/http/middleware/request_config.go +++ b/pkg/http/middleware/request_config.go @@ -21,7 +21,7 @@ func WithRequestConfig(next http.Handler) http.Handler { ctx := r.Context() // Readonly mode - if relaxedParseBool(r.Header.Get(headers.MCPReadOnlyHeader)) { + if slices.ContainsFunc(r.Header.Values(headers.MCPReadOnlyHeader), relaxedParseBool) { ctx = ghcontext.WithReadonly(ctx, true) } @@ -36,7 +36,7 @@ func WithRequestConfig(next http.Handler) http.Handler { } // Lockdown mode - if relaxedParseBool(r.Header.Get(headers.MCPLockdownHeader)) { + if slices.ContainsFunc(r.Header.Values(headers.MCPLockdownHeader), relaxedParseBool) { ctx = ghcontext.WithLockdownMode(ctx, true) } @@ -46,7 +46,7 @@ func WithRequestConfig(next http.Handler) http.Handler { } // Insiders mode - if relaxedParseBool(r.Header.Get(headers.MCPInsidersHeader)) { + if slices.ContainsFunc(r.Header.Values(headers.MCPInsidersHeader), relaxedParseBool) { ctx = ghcontext.WithInsidersMode(ctx, true) } @@ -56,7 +56,7 @@ func WithRequestConfig(next http.Handler) http.Handler { if hasHeaderFeatures { ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(strings.Join(r.Header.Values(headers.MCPFeaturesHeader), ","))) } else if hasQueryFeatures { - ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(query.Get(queryParamFeatures))) + ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(strings.Join(query[queryParamFeatures], ","))) } next.ServeHTTP(w, r.WithContext(ctx)) diff --git a/pkg/http/middleware/request_config_test.go b/pkg/http/middleware/request_config_test.go index ce572d5084..b9b3ecf041 100644 --- a/pkg/http/middleware/request_config_test.go +++ b/pkg/http/middleware/request_config_test.go @@ -35,6 +35,52 @@ func TestWithRequestConfigRepeatedListHeaders(t *testing.T) { } } +func TestWithRequestConfigRepeatedBooleanHeaders(t *testing.T) { + headersToTest := []struct { + name string + header string + get func(context.Context) bool + }{ + {"readonly", headers.MCPReadOnlyHeader, ghcontext.IsReadonly}, + {"lockdown", headers.MCPLockdownHeader, ghcontext.IsLockdownMode}, + {"insiders", headers.MCPInsidersHeader, ghcontext.IsInsidersMode}, + } + tests := []struct { + name string + values []string + want bool + }{ + {"absent", nil, false}, + {"all false", []string{"", " false ", "0", " NO ", "off", "N", "f"}, false}, + {"false then true", []string{"false", "true"}, true}, + {"true then false", []string{"true", "false"}, true}, + {"empty then one", []string{"", "1"}, true}, + {"zero then yes", []string{"0", "yes"}, true}, + {"no then on", []string{"no", "on"}, true}, + {"off then y", []string{"off", "y"}, true}, + {"n then t", []string{"n", "t"}, true}, + {"mixed case and whitespace", []string{" F ", " TrUe "}, true}, + {"arbitrary truthy value", []string{"false", "enabled"}, true}, + {"comma value retains scalar semantics", []string{"false,false"}, true}, + } + for _, header := range headersToTest { + t.Run(header.name, func(t *testing.T) { + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + for _, value := range tc.values { + req.Header.Add(header.header, value) + } + handler := WithRequestConfig(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + assert.Equal(t, tc.want, header.get(r.Context())) + })) + handler.ServeHTTP(httptest.NewRecorder(), req) + }) + } + }) + } +} + func TestWithRequestConfigFeatureSelection(t *testing.T) { tests := []struct { name string @@ -50,6 +96,39 @@ func TestWithRequestConfigFeatureSelection(t *testing.T) { wantFeatures: []string{"mcp_holdback_consolidated_projects"}, wantPresent: true, }, + { + name: "repeated query parameters combine comma separated values", + url: "/?features=first,%20second&features=&features=%20third,%20,%20fourth%20", + wantFeatures: []string{"first", "second", "third", "fourth"}, + wantPresent: true, + }, + { + name: "empty first query value does not hide later features", + url: "/?features=&features=flag_from_query", + wantFeatures: []string{"flag_from_query"}, + wantPresent: true, + }, + { + name: "repeated empty query values store an explicit empty selection", + url: "/?features=&features=%20,%20", + wantFeatures: []string{}, + wantPresent: true, + }, + { + name: "header wins over repeated query parameters", + url: "/?features=first&features=second", + headerSet: true, + headerValue: "flag_from_header", + wantFeatures: []string{"flag_from_header"}, + wantPresent: true, + }, + { + name: "empty header suppresses repeated query parameters", + url: "/?features=first&features=second", + headerSet: true, + wantFeatures: []string{}, + wantPresent: true, + }, { name: "header only", url: "/",