diff options
author | Pavlo Strokov <pstrokov@gitlab.com> | 2020-04-07 13:14:57 +0300 |
---|---|---|
committer | Pavlo Strokov <pstrokov@gitlab.com> | 2020-04-07 16:38:33 +0300 |
commit | 2e4ceea223134070550ce37bb3507b2832781a73 (patch) | |
tree | 8db05cd91b1064369d8859ef6c320112f6a07b59 | |
parent | 98103b3d885761f3b2ed6aff7beaee355ef9ec0b (diff) |
Race condition on `ProxyHeaderWhitelist`
Global variable `ProxyHeaderWhitelist` refactored to local.
Closes: https://gitlab.com/gitlab-org/gitaly/-/issues/2614
-rw-r--r-- | changelogs/unreleased/ps-proxyheaderwhitelist-race.yml | 5 | ||||
-rw-r--r-- | internal/rubyserver/proxy.go | 13 | ||||
-rw-r--r-- | internal/rubyserver/proxy_test.go | 1 |
3 files changed, 10 insertions, 9 deletions
diff --git a/changelogs/unreleased/ps-proxyheaderwhitelist-race.yml b/changelogs/unreleased/ps-proxyheaderwhitelist-race.yml new file mode 100644 index 000000000..d7a400caf --- /dev/null +++ b/changelogs/unreleased/ps-proxyheaderwhitelist-race.yml @@ -0,0 +1,5 @@ +--- +title: Race condition on ProxyHeaderWhitelist +merge_request: 2025 +author: +type: fixed diff --git a/internal/rubyserver/proxy.go b/internal/rubyserver/proxy.go index 05a35bac8..87052c900 100644 --- a/internal/rubyserver/proxy.go +++ b/internal/rubyserver/proxy.go @@ -11,10 +11,6 @@ import ( "google.golang.org/grpc/metadata" ) -// ProxyHeaderWhitelist is the list of http/2 headers that will be -// forwarded as-is to gitaly-ruby. -var ProxyHeaderWhitelist = []string{"gitaly-servers"} - // Headers prefixed with this string get whitelisted automatically const rubyFeaturePrefix = "gitaly-feature-ruby-" @@ -65,17 +61,18 @@ func setHeaders(ctx context.Context, repo *gitalypb.Repository, mustExist bool) repoAltDirsHeader, repoAltDirsCombined, ) + // list of http/2 headers that will be forwarded as-is to gitaly-ruby + proxyHeaderWhitelist := []string{"gitaly-servers"} + if inMD, ok := metadata.FromIncomingContext(ctx); ok { // Automatically whitelist any Ruby-specific feature flag for header := range inMD { if strings.HasPrefix(header, rubyFeaturePrefix) { - // TODO: this changes state of the global variable without any synchronization - // https://gitlab.com/gitlab-org/gitaly/-/issues/2614 - ProxyHeaderWhitelist = append(ProxyHeaderWhitelist, header) + proxyHeaderWhitelist = append(proxyHeaderWhitelist, header) } } - for _, header := range ProxyHeaderWhitelist { + for _, header := range proxyHeaderWhitelist { for _, v := range inMD[header] { md = metadata.Join(md, metadata.Pairs(header, v)) } diff --git a/internal/rubyserver/proxy_test.go b/internal/rubyserver/proxy_test.go index 03936f41c..7233a57d9 100644 --- a/internal/rubyserver/proxy_test.go +++ b/internal/rubyserver/proxy_test.go @@ -31,7 +31,6 @@ func TestSetHeadersPreservesWhitelistedMetadata(t *testing.T) { defer cancel() key := "gitaly-servers" - require.Contains(t, ProxyHeaderWhitelist, key, "sanity check") value := "test-value" inCtx := metadata.NewIncomingContext(ctx, metadata.Pairs(key, value)) |