From f9f4e5d0a2eb40b5838f74a0e443762f73da40fe Mon Sep 17 00:00:00 2001 From: Roman Loginov Date: Mon, 16 Dec 2024 10:52:16 +0300 Subject: [PATCH] [#583] Fix list-buckets vhs routing The problem is that with VHS requests, the list-buckets operation does not work because the request is filtered on list-objects-v1. Since list-buckets can also have query parameters, in the end it is necessary to distinguish list-buckets from list-objects-v1 only by the presence of the bucket name in the URL (provided that the request is in VHS style). Signed-off-by: Roman Loginov --- api/middleware/policy.go | 17 ++++++---- api/router.go | 6 ++-- api/router_filter.go | 9 ++--- api/router_test.go | 72 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 15 deletions(-) diff --git a/api/middleware/policy.go b/api/middleware/policy.go index 9e36f0b..982fb3a 100644 --- a/api/middleware/policy.go +++ b/api/middleware/policy.go @@ -24,13 +24,16 @@ import ( ) const ( - QueryVersionID = "versionId" - QueryPrefix = "prefix" - QueryDelimiter = "delimiter" - QueryMaxKeys = "max-keys" - QueryMarker = "marker" - QueryEncodingType = "encoding-type" - amzTagging = "x-amz-tagging" + QueryVersionID = "versionId" + QueryPrefix = "prefix" + QueryDelimiter = "delimiter" + QueryMaxKeys = "max-keys" + QueryMarker = "marker" + QueryEncodingType = "encoding-type" + QueryMaxBuckets = "max-buckets" + QueryContinuationToken = "continuation-token" + QueryBucketRegion = "bucket-region" + amzTagging = "x-amz-tagging" unmatchedBucketOperation = "UnmatchedBucketOperation" ) diff --git a/api/router.go b/api/router.go index 765bec3..e3a744e 100644 --- a/api/router.go +++ b/api/router.go @@ -336,10 +336,8 @@ func bucketRouter(h Handler) chi.Router { Handler(named(s3middleware.ListBucketObjectVersionsOperation, h.ListBucketObjectVersionsHandler))). Add(NewFilter(). AllowedQueries(s3middleware.QueryDelimiter, s3middleware.QueryMaxKeys, s3middleware.QueryPrefix, - s3middleware.QueryMarker, s3middleware.QueryEncodingType). - Handler(named(s3middleware.ListObjectsV1Operation, h.ListObjectsV1Handler))). - Add(NewFilter(). - NoQueries(). + s3middleware.QueryMarker, s3middleware.QueryEncodingType, s3middleware.QueryBucketRegion, + s3middleware.QueryContinuationToken, s3middleware.QueryMaxBuckets). Handler(listWrapper(h))). DefaultHandler(notSupportedHandler())) }) diff --git a/api/router_filter.go b/api/router_filter.go index 7742ce5..06282be 100644 --- a/api/router_filter.go +++ b/api/router_filter.go @@ -138,13 +138,14 @@ func (hf *HandlerFilters) ServeHTTP(w http.ResponseWriter, r *http.Request) { } func (hf *HandlerFilters) match(r *http.Request) http.Handler { + queries := r.URL.Query() + LOOP: for _, filter := range hf.filters { - if filter.noQueries && len(r.URL.Query()) > 0 { + if filter.noQueries && len(queries) > 0 { continue } if len(filter.allowedQueries) > 0 { - queries := r.URL.Query() for key := range queries { if _, ok := filter.allowedQueries[key]; !ok { continue LOOP @@ -158,8 +159,8 @@ LOOP: } } for _, query := range filter.queries { - queryVal := r.URL.Query().Get(query.Key) - if !r.URL.Query().Has(query.Key) || query.Value != "" && query.Value != queryVal { + queryVal := queries.Get(query.Key) + if !queries.Has(query.Key) || query.Value != "" && query.Value != queryVal { continue LOOP } } diff --git a/api/router_test.go b/api/router_test.go index 18fe336..981b024 100644 --- a/api/router_test.go +++ b/api/router_test.go @@ -903,6 +903,78 @@ func TestRouterListObjectsV2Domains(t *testing.T) { require.Equal(t, s3middleware.ListObjectsV2Operation, resp.Method) } +func TestRouterListingVHS(t *testing.T) { + baseDomain := "domain.com" + baseDomainWithBkt := "bucket.domain.com" + chiRouter := prepareRouter(t, enableVHSDomains(baseDomain)) + chiRouter.handler.buckets["bucket"] = &data.BucketInfo{} + + for _, tc := range []struct { + name string + host string + queries string + expectedOperation string + notSupported bool + }{ + { + name: "list-object-v1 without query params", + host: baseDomainWithBkt, + expectedOperation: s3middleware.ListObjectsV1Operation, + }, + { + name: "list-buckets without query params", + host: baseDomain, + expectedOperation: s3middleware.ListBucketsOperation, + }, + { + name: "list-objects-v1 with prefix param", + host: baseDomainWithBkt, + queries: func() string { + query := make(url.Values) + query.Set(s3middleware.QueryPrefix, "prefix") + return query.Encode() + }(), + expectedOperation: s3middleware.ListObjectsV1Operation, + }, + { + name: "list-buckets with prefix param", + host: baseDomain, + queries: func() string { + query := make(url.Values) + query.Set(s3middleware.QueryPrefix, "prefix") + return query.Encode() + }(), + expectedOperation: s3middleware.ListBucketsOperation, + }, + { + name: "not supported operation", + host: baseDomain, + queries: func() string { + query := make(url.Values) + query.Set("invalid", "invalid") + return query.Encode() + }(), + notSupported: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.URL.RawQuery = tc.queries + r.Host = tc.host + chiRouter.ServeHTTP(w, r) + + if tc.notSupported { + assertAPIError(t, w, apierr.ErrNotSupported) + return + } + + resp := readResponse(t, w) + require.Equal(t, tc.expectedOperation, resp.Method) + }) + } +} + func enableVHSDomains(domains ...string) option { return func(cfg *Config) { setting := cfg.MiddlewareSettings.(*middlewareSettingsMock)