From ac1ff0e7e22c8fcb44d3be7913c8d810603f99e2 Mon Sep 17 00:00:00 2001 From: Michal Niewrzal Date: Tue, 20 Jun 2023 10:53:42 +0200 Subject: [PATCH] satellite/accounting/tally: handle well bucket names with escape char Some of tally queries are not passing bucket name as byte but as string. If bucket contains some special characters encoding to bytea can fail. This change makes sure all parts of tally passes bucket name correctly. Change-Id: I7330d546b44d86a2e4614c814580e9e5262370ed --- satellite/buckets/db_test.go | 6 ++++++ satellite/metabase/loop.go | 2 +- satellite/metabase/loop_test.go | 19 +++++++++++++++++++ satellite/satellitedb/bucketsdb.go | 2 +- satellite/satellitedb/projectaccounting.go | 2 +- .../satellitedb/projectaccounting_test.go | 16 ++++++++++++++++ 6 files changed, 44 insertions(+), 3 deletions(-) diff --git a/satellite/buckets/db_test.go b/satellite/buckets/db_test.go index 14c05298b..1acea8d54 100644 --- a/satellite/buckets/db_test.go +++ b/satellite/buckets/db_test.go @@ -289,6 +289,12 @@ func TestBatchBuckets(t *testing.T) { require.False(t, more) } } + + // check if invalid bucket name 'a\' won't throw an error + _, err := db.Buckets().IterateBucketLocations(ctx, uuid.UUID{}, "a\\", 1, func(bucketLocations []metabase.BucketLocation) (err error) { + return nil + }) + require.NoError(t, err) }) } diff --git a/satellite/metabase/loop.go b/satellite/metabase/loop.go index eae24a07b..2b299a45c 100644 --- a/satellite/metabase/loop.go +++ b/satellite/metabase/loop.go @@ -463,7 +463,7 @@ func (db *DB) CollectBucketTallies(ctx context.Context, opts CollectBucketTallie (expires_at IS NULL OR expires_at > $5) GROUP BY (project_id, bucket_name) ORDER BY (project_id, bucket_name) ASC - `, opts.From.ProjectID, opts.From.BucketName, opts.To.ProjectID, opts.To.BucketName, opts.Now))(func(rows tagsql.Rows) error { + `, opts.From.ProjectID, []byte(opts.From.BucketName), opts.To.ProjectID, []byte(opts.To.BucketName), opts.Now))(func(rows tagsql.Rows) error { for rows.Next() { var bucketTally BucketTally diff --git a/satellite/metabase/loop_test.go b/satellite/metabase/loop_test.go index 2a32fa09d..ded012c4f 100644 --- a/satellite/metabase/loop_test.go +++ b/satellite/metabase/loop_test.go @@ -819,6 +819,25 @@ func TestCollectBucketTallies(t *testing.T) { metabasetest.Verify{}.Check(ctx, t, db) }) + t.Run("invalid bucket name", func(t *testing.T) { + defer metabasetest.DeleteAll{}.Check(ctx, t, db) + + metabasetest.CollectBucketTallies{ + Opts: metabase.CollectBucketTallies{ + From: metabase.BucketLocation{ + ProjectID: testrand.UUID(), + BucketName: "a\\", + }, + To: metabase.BucketLocation{ + ProjectID: testrand.UUID(), + BucketName: "b\\", + }, + }, + Result: nil, + }.Check(ctx, t, db) + metabasetest.Verify{}.Check(ctx, t, db) + }) + t.Run("pending and committed", func(t *testing.T) { defer metabasetest.DeleteAll{}.Check(ctx, t, db) diff --git a/satellite/satellitedb/bucketsdb.go b/satellite/satellitedb/bucketsdb.go index ae7de2a1f..4583ca8b5 100644 --- a/satellite/satellitedb/bucketsdb.go +++ b/satellite/satellitedb/bucketsdb.go @@ -339,7 +339,7 @@ func (db *bucketsDB) IterateBucketLocations(ctx context.Context, projectID uuid. WHERE (project_id, name) > ($1, $2) GROUP BY (project_id, name) ORDER BY (project_id, name) ASC LIMIT $3 - `, projectID, bucketName, moreLimit) + `, projectID, []byte(bucketName), moreLimit) if err != nil { return false, buckets.ErrBucket.New("BatchBuckets query error: %s", err) } diff --git a/satellite/satellitedb/projectaccounting.go b/satellite/satellitedb/projectaccounting.go index 19ac7ff3a..37d924145 100644 --- a/satellite/satellitedb/projectaccounting.go +++ b/satellite/satellitedb/projectaccounting.go @@ -160,7 +160,7 @@ func (db *ProjectAccounting) GetNonEmptyTallyBucketsInRange(ctx context.Context, ORDER BY interval_start DESC LIMIT 1 ) - `, from.ProjectID, from.BucketName, to.ProjectID, to.BucketName), + `, from.ProjectID, []byte(from.BucketName), to.ProjectID, []byte(to.BucketName)), )(func(r tagsql.Rows) error { for r.Next() { loc := metabase.BucketLocation{} diff --git a/satellite/satellitedb/projectaccounting_test.go b/satellite/satellitedb/projectaccounting_test.go index 564834230..34f57de7f 100644 --- a/satellite/satellitedb/projectaccounting_test.go +++ b/satellite/satellitedb/projectaccounting_test.go @@ -15,11 +15,13 @@ import ( "storj.io/common/testrand" "storj.io/common/uuid" "storj.io/storj/private/testplanet" + "storj.io/storj/satellite" "storj.io/storj/satellite/accounting" "storj.io/storj/satellite/buckets" "storj.io/storj/satellite/console" "storj.io/storj/satellite/metabase" "storj.io/storj/satellite/orders" + "storj.io/storj/satellite/satellitedb/satellitedbtest" ) func Test_DailyUsage(t *testing.T) { @@ -498,3 +500,17 @@ func TestProjectUsageGap(t *testing.T) { require.EqualValues(t, expectedStorage, usage.Storage) }) } + +func TestProjectaccounting_GetNonEmptyTallyBucketsInRange(t *testing.T) { + // test if invalid bucket name will be handled correctly + satellitedbtest.Run(t, func(ctx *testcontext.Context, t *testing.T, db satellite.DB) { + _, err := db.ProjectAccounting().GetNonEmptyTallyBucketsInRange(ctx, metabase.BucketLocation{ + ProjectID: testrand.UUID(), + BucketName: "a\\", + }, metabase.BucketLocation{ + ProjectID: testrand.UUID(), + BucketName: "b\\", + }) + require.NoError(t, err) + }) +}