Skip to content

Commit

Permalink
satellite/metainfo: do full bucket validation only on create
Browse files Browse the repository at this point in the history 
We are doing full bucket name validation for many requests but
we should do this only while creating bucket. Other requests will be
covered only by basic name length validation. Less strict validation for
other requests will make bucket usable in case of invalid bucket names
in DB (we have such cases from the past).

#6044

Change-Id: I3a41050e3637787f788705ef15b5dc4df4d01fc6
  • Loading branch information
@mniewrzal
mniewrzal committed Jul 17, 2023
1 parent 9576190 commit 5272fd8
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 23 deletions.
4 changes: 2 additions & 2 deletions satellite/metainfo/endpoint_bucket.go
View file
Expand Up @@ -71,7 +71,7 @@ func (endpoint *Endpoint) CreateBucket(ctx context.Context, req *pb.BucketCreate
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.Name)
err = endpoint.validateBucketName(req.Name)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -180,7 +180,7 @@ func (endpoint *Endpoint) DeleteBucket(ctx context.Context, req *pb.BucketDelete
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.Name)
err = endpoint.validateBucketNameLength(req.Name)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down
18 changes: 13 additions & 5 deletions satellite/metainfo/endpoint_bucket_test.go
View file
Expand Up @@ -118,15 +118,23 @@ func TestBucketNameValidation(t *testing.T) {
"test\\", "test%",
}
for _, name := range invalidNames {
_, err = metainfoClient.BeginObject(ctx, metaclient.BeginObjectParams{
Bucket: []byte(name),
EncryptedObjectKey: []byte("123"),

_, err = metainfoClient.CreateBucket(ctx, metaclient.CreateBucketParams{
Name: []byte(name),
})
require.Error(t, err, "bucket name: %v", name)
require.True(t, errs2.IsRPC(err, rpcstatus.InvalidArgument))
}

_, err = metainfoClient.CreateBucket(ctx, metaclient.CreateBucketParams{
Name: []byte(name),
invalidNames = []string{
"", "t", "te",
"testbucket-64-0123456789012345678901234567890123456789012345abcd",
}
for _, name := range invalidNames {
// BeginObject validates only bucket name length
_, err = metainfoClient.BeginObject(ctx, metaclient.BeginObjectParams{
Bucket: []byte(name),
EncryptedObjectKey: []byte("123"),
})
require.Error(t, err, "bucket name: %v", name)
require.True(t, errs2.IsRPC(err, rpcstatus.InvalidArgument))
Expand Down
25 changes: 13 additions & 12 deletions satellite/metainfo/endpoint_object.go
View file
Expand Up @@ -66,7 +66,8 @@ func (endpoint *Endpoint) BeginObject(ctx context.Context, req *pb.ObjectBeginRe
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, "Invalid expiration time")
}

err = endpoint.validateBucket(ctx, req.Bucket)
// we can do just basic name validation because later we are checking bucket in DB
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -310,7 +311,7 @@ func (endpoint *Endpoint) GetObject(ctx context.Context, req *pb.ObjectGetReques
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -409,7 +410,7 @@ func (endpoint *Endpoint) DownloadObject(ctx context.Context, req *pb.ObjectDown
return nil, err
}

err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -804,7 +805,7 @@ func (endpoint *Endpoint) ListObjects(ctx context.Context, req *pb.ObjectListReq
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -944,7 +945,7 @@ func (endpoint *Endpoint) ListPendingObjectStreams(ctx context.Context, req *pb.
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -1057,7 +1058,7 @@ func (endpoint *Endpoint) BeginDeleteObject(ctx context.Context, req *pb.ObjectB
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -1146,7 +1147,7 @@ func (endpoint *Endpoint) GetObjectIPs(ctx context.Context, req *pb.ObjectGetIPs
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -1237,7 +1238,7 @@ func (endpoint *Endpoint) UpdateObjectMetadata(ctx context.Context, req *pb.Obje
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -1615,7 +1616,7 @@ func (endpoint *Endpoint) BeginMoveObject(ctx context.Context, req *pb.ObjectBeg
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

for _, bucket := range [][]byte{req.Bucket, req.NewBucket} {
err = endpoint.validateBucket(ctx, bucket)
err = endpoint.validateBucketNameLength(bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -1761,7 +1762,7 @@ func (endpoint *Endpoint) FinishMoveObject(ctx context.Context, req *pb.ObjectFi
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.NewBucket)
err = endpoint.validateBucketNameLength(req.NewBucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -1840,7 +1841,7 @@ func (endpoint *Endpoint) BeginCopyObject(ctx context.Context, req *pb.ObjectBeg
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

for _, bucket := range [][]byte{req.Bucket, req.NewBucket} {
err = endpoint.validateBucket(ctx, bucket)
err = endpoint.validateBucketNameLength(bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down Expand Up @@ -1952,7 +1953,7 @@ func (endpoint *Endpoint) FinishCopyObject(ctx context.Context, req *pb.ObjectFi
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))

err = endpoint.validateBucket(ctx, req.NewBucket)
err = endpoint.validateBucketNameLength(req.NewBucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
Expand Down
14 changes: 10 additions & 4 deletions satellite/metainfo/validation.go
View file
Expand Up @@ -247,9 +247,7 @@ func (endpoint *Endpoint) checkRate(ctx context.Context, projectID uuid.UUID) (e
return nil
}

func (endpoint *Endpoint) validateBucket(ctx context.Context, bucket []byte) (err error) {
defer mon.Task()(&ctx)(&err)

func (endpoint *Endpoint) validateBucketNameLength(bucket []byte) (err error) {
if len(bucket) == 0 {
return Error.Wrap(buckets.ErrNoBucket.New(""))
}
Expand All @@ -258,11 +256,19 @@ func (endpoint *Endpoint) validateBucket(ctx context.Context, bucket []byte) (er
return Error.New("bucket name must be at least 3 and no more than 63 characters long")
}

return nil
}

func (endpoint *Endpoint) validateBucketName(bucket []byte) error {
if err := endpoint.validateBucketNameLength(bucket); err != nil {
return err
}

// Regexp not used because benchmark shows it will be slower for valid bucket names
// https://gist.github.com/mniewrzal/49de3af95f36e63e88fac24f565e444c
labels := bytes.Split(bucket, []byte("."))
for _, label := range labels {
err = validateBucketLabel(label)
err := validateBucketLabel(label)
if err != nil {
return err
}
Expand Down

0 comments on commit 5272fd8

Please sign in to comment.