From fdeb834801c177f485ef9007e708dccb3f617dec Mon Sep 17 00:00:00 2001 From: Michal Niewrzal Date: Mon, 24 Jun 2019 11:52:25 +0200 Subject: [PATCH] Bucket name validation (#2244) --- pkg/metainfo/kvmetainfo/buckets_test.go | 106 ++++++++++++------------ pkg/miniogw/gateway_test.go | 2 +- pkg/storage/segments/store_test.go | 22 ++--- satellite/metainfo/metainfo_test.go | 65 +++++++++++++-- satellite/metainfo/validation.go | 69 +++++++++++++-- 5 files changed, 185 insertions(+), 79 deletions(-) diff --git a/pkg/metainfo/kvmetainfo/buckets_test.go b/pkg/metainfo/kvmetainfo/buckets_test.go index e750e1956..b07f1256a 100644 --- a/pkg/metainfo/kvmetainfo/buckets_test.go +++ b/pkg/metainfo/kvmetainfo/buckets_test.go @@ -165,7 +165,7 @@ func TestListBucketsEmpty(t *testing.T) { func TestListBuckets(t *testing.T) { runTest(t, func(ctx context.Context, planet *testplanet.Planet, db *kvmetainfo.DB, streams streams.Store) { - bucketNames := []string{"a", "aa", "b", "bb", "c"} + bucketNames := []string{"a00", "aa0", "b00", "bb0", "c00"} for _, name := range bucketNames { _, err := db.CreateBucket(ctx, name, nil) @@ -179,70 +179,70 @@ func TestListBuckets(t *testing.T) { more bool result []string }{ - {cursor: "", dir: storj.After, limit: 0, more: false, result: []string{"a", "aa", "b", "bb", "c"}}, - {cursor: "`", dir: storj.After, limit: 0, more: false, result: []string{"a", "aa", "b", "bb", "c"}}, - {cursor: "b", dir: storj.After, limit: 0, more: false, result: []string{"bb", "c"}}, - {cursor: "c", dir: storj.After, limit: 0, more: false, result: []string{}}, + {cursor: "", dir: storj.After, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0", "c00"}}, + {cursor: "`", dir: storj.After, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0", "c00"}}, + {cursor: "b00", dir: storj.After, limit: 0, more: false, result: []string{"bb0", "c00"}}, + {cursor: "c00", dir: storj.After, limit: 0, more: false, result: []string{}}, {cursor: "ca", dir: storj.After, limit: 0, more: false, result: []string{}}, - {cursor: "", dir: storj.After, limit: 1, more: true, result: []string{"a"}}, - {cursor: "`", dir: storj.After, limit: 1, more: true, result: []string{"a"}}, - {cursor: "aa", dir: storj.After, limit: 1, more: true, result: []string{"b"}}, - {cursor: "c", dir: storj.After, limit: 1, more: false, result: []string{}}, + {cursor: "", dir: storj.After, limit: 1, more: true, result: []string{"a00"}}, + {cursor: "`", dir: storj.After, limit: 1, more: true, result: []string{"a00"}}, + {cursor: "aa0", dir: storj.After, limit: 1, more: true, result: []string{"b00"}}, + {cursor: "c00", dir: storj.After, limit: 1, more: false, result: []string{}}, {cursor: "ca", dir: storj.After, limit: 1, more: false, result: []string{}}, - {cursor: "", dir: storj.After, limit: 2, more: true, result: []string{"a", "aa"}}, - {cursor: "`", dir: storj.After, limit: 2, more: true, result: []string{"a", "aa"}}, - {cursor: "aa", dir: storj.After, limit: 2, more: true, result: []string{"b", "bb"}}, - {cursor: "bb", dir: storj.After, limit: 2, more: false, result: []string{"c"}}, - {cursor: "c", dir: storj.After, limit: 2, more: false, result: []string{}}, + {cursor: "", dir: storj.After, limit: 2, more: true, result: []string{"a00", "aa0"}}, + {cursor: "`", dir: storj.After, limit: 2, more: true, result: []string{"a00", "aa0"}}, + {cursor: "aa0", dir: storj.After, limit: 2, more: true, result: []string{"b00", "bb0"}}, + {cursor: "bb0", dir: storj.After, limit: 2, more: false, result: []string{"c00"}}, + {cursor: "c00", dir: storj.After, limit: 2, more: false, result: []string{}}, {cursor: "ca", dir: storj.After, limit: 2, more: false, result: []string{}}, - {cursor: "", dir: storj.Forward, limit: 0, more: false, result: []string{"a", "aa", "b", "bb", "c"}}, - {cursor: "`", dir: storj.Forward, limit: 0, more: false, result: []string{"a", "aa", "b", "bb", "c"}}, - {cursor: "b", dir: storj.Forward, limit: 0, more: false, result: []string{"b", "bb", "c"}}, - {cursor: "c", dir: storj.Forward, limit: 0, more: false, result: []string{"c"}}, + {cursor: "", dir: storj.Forward, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0", "c00"}}, + {cursor: "`", dir: storj.Forward, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0", "c00"}}, + {cursor: "b00", dir: storj.Forward, limit: 0, more: false, result: []string{"b00", "bb0", "c00"}}, + {cursor: "c00", dir: storj.Forward, limit: 0, more: false, result: []string{"c00"}}, {cursor: "ca", dir: storj.Forward, limit: 0, more: false, result: []string{}}, - {cursor: "", dir: storj.Forward, limit: 1, more: true, result: []string{"a"}}, - {cursor: "`", dir: storj.Forward, limit: 1, more: true, result: []string{"a"}}, - {cursor: "aa", dir: storj.Forward, limit: 1, more: true, result: []string{"aa"}}, - {cursor: "c", dir: storj.Forward, limit: 1, more: false, result: []string{"c"}}, + {cursor: "", dir: storj.Forward, limit: 1, more: true, result: []string{"a00"}}, + {cursor: "`", dir: storj.Forward, limit: 1, more: true, result: []string{"a00"}}, + {cursor: "aa0", dir: storj.Forward, limit: 1, more: true, result: []string{"aa0"}}, + {cursor: "c00", dir: storj.Forward, limit: 1, more: false, result: []string{"c00"}}, {cursor: "ca", dir: storj.Forward, limit: 1, more: false, result: []string{}}, - {cursor: "", dir: storj.Forward, limit: 2, more: true, result: []string{"a", "aa"}}, - {cursor: "`", dir: storj.Forward, limit: 2, more: true, result: []string{"a", "aa"}}, - {cursor: "aa", dir: storj.Forward, limit: 2, more: true, result: []string{"aa", "b"}}, - {cursor: "bb", dir: storj.Forward, limit: 2, more: false, result: []string{"bb", "c"}}, - {cursor: "c", dir: storj.Forward, limit: 2, more: false, result: []string{"c"}}, + {cursor: "", dir: storj.Forward, limit: 2, more: true, result: []string{"a00", "aa0"}}, + {cursor: "`", dir: storj.Forward, limit: 2, more: true, result: []string{"a00", "aa0"}}, + {cursor: "aa0", dir: storj.Forward, limit: 2, more: true, result: []string{"aa0", "b00"}}, + {cursor: "bb0", dir: storj.Forward, limit: 2, more: false, result: []string{"bb0", "c00"}}, + {cursor: "c00", dir: storj.Forward, limit: 2, more: false, result: []string{"c00"}}, {cursor: "ca", dir: storj.Forward, limit: 2, more: false, result: []string{}}, - {cursor: "", dir: storj.Backward, limit: 0, more: false, result: []string{"a", "aa", "b", "bb", "c"}}, + {cursor: "", dir: storj.Backward, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0", "c00"}}, {cursor: "`", dir: storj.Backward, limit: 0, more: false, result: []string{}}, - {cursor: "b", dir: storj.Backward, limit: 0, more: false, result: []string{"a", "aa", "b"}}, - {cursor: "c", dir: storj.Backward, limit: 0, more: false, result: []string{"a", "aa", "b", "bb", "c"}}, - {cursor: "ca", dir: storj.Backward, limit: 0, more: false, result: []string{"a", "aa", "b", "bb", "c"}}, - {cursor: "", dir: storj.Backward, limit: 1, more: true, result: []string{"c"}}, + {cursor: "b00", dir: storj.Backward, limit: 0, more: false, result: []string{"a00", "aa0", "b00"}}, + {cursor: "c00", dir: storj.Backward, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0", "c00"}}, + {cursor: "ca", dir: storj.Backward, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0", "c00"}}, + {cursor: "", dir: storj.Backward, limit: 1, more: true, result: []string{"c00"}}, {cursor: "`", dir: storj.Backward, limit: 1, more: false, result: []string{}}, - {cursor: "aa", dir: storj.Backward, limit: 1, more: true, result: []string{"aa"}}, - {cursor: "c", dir: storj.Backward, limit: 1, more: true, result: []string{"c"}}, - {cursor: "ca", dir: storj.Backward, limit: 1, more: true, result: []string{"c"}}, - {cursor: "", dir: storj.Backward, limit: 2, more: true, result: []string{"bb", "c"}}, + {cursor: "aa0", dir: storj.Backward, limit: 1, more: true, result: []string{"aa0"}}, + {cursor: "c00", dir: storj.Backward, limit: 1, more: true, result: []string{"c00"}}, + {cursor: "ca", dir: storj.Backward, limit: 1, more: true, result: []string{"c00"}}, + {cursor: "", dir: storj.Backward, limit: 2, more: true, result: []string{"bb0", "c00"}}, {cursor: "`", dir: storj.Backward, limit: 2, more: false, result: []string{}}, - {cursor: "aa", dir: storj.Backward, limit: 2, more: false, result: []string{"a", "aa"}}, - {cursor: "bb", dir: storj.Backward, limit: 2, more: true, result: []string{"b", "bb"}}, - {cursor: "c", dir: storj.Backward, limit: 2, more: true, result: []string{"bb", "c"}}, - {cursor: "ca", dir: storj.Backward, limit: 2, more: true, result: []string{"bb", "c"}}, - {cursor: "", dir: storj.Before, limit: 0, more: false, result: []string{"a", "aa", "b", "bb", "c"}}, + {cursor: "aa0", dir: storj.Backward, limit: 2, more: false, result: []string{"a00", "aa0"}}, + {cursor: "bb0", dir: storj.Backward, limit: 2, more: true, result: []string{"b00", "bb0"}}, + {cursor: "c00", dir: storj.Backward, limit: 2, more: true, result: []string{"bb0", "c00"}}, + {cursor: "ca", dir: storj.Backward, limit: 2, more: true, result: []string{"bb0", "c00"}}, + {cursor: "", dir: storj.Before, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0", "c00"}}, {cursor: "`", dir: storj.Before, limit: 0, more: false, result: []string{}}, - {cursor: "b", dir: storj.Before, limit: 0, more: false, result: []string{"a", "aa"}}, - {cursor: "c", dir: storj.Before, limit: 0, more: false, result: []string{"a", "aa", "b", "bb"}}, - {cursor: "ca", dir: storj.Before, limit: 0, more: false, result: []string{"a", "aa", "b", "bb", "c"}}, - {cursor: "", dir: storj.Before, limit: 1, more: true, result: []string{"c"}}, + {cursor: "b00", dir: storj.Before, limit: 0, more: false, result: []string{"a00", "aa0"}}, + {cursor: "c00", dir: storj.Before, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0"}}, + {cursor: "ca", dir: storj.Before, limit: 0, more: false, result: []string{"a00", "aa0", "b00", "bb0", "c00"}}, + {cursor: "", dir: storj.Before, limit: 1, more: true, result: []string{"c00"}}, {cursor: "`", dir: storj.Before, limit: 1, more: false, result: []string{}}, - {cursor: "aa", dir: storj.Before, limit: 1, more: false, result: []string{"a"}}, - {cursor: "c", dir: storj.Before, limit: 1, more: true, result: []string{"bb"}}, - {cursor: "ca", dir: storj.Before, limit: 1, more: true, result: []string{"c"}}, - {cursor: "", dir: storj.Before, limit: 2, more: true, result: []string{"bb", "c"}}, + {cursor: "aa0", dir: storj.Before, limit: 1, more: false, result: []string{"a00"}}, + {cursor: "c00", dir: storj.Before, limit: 1, more: true, result: []string{"bb0"}}, + {cursor: "ca", dir: storj.Before, limit: 1, more: true, result: []string{"c00"}}, + {cursor: "", dir: storj.Before, limit: 2, more: true, result: []string{"bb0", "c00"}}, {cursor: "`", dir: storj.Before, limit: 2, more: false, result: []string{}}, - {cursor: "aa", dir: storj.Before, limit: 2, more: false, result: []string{"a"}}, - {cursor: "bb", dir: storj.Before, limit: 2, more: true, result: []string{"aa", "b"}}, - {cursor: "c", dir: storj.Before, limit: 2, more: true, result: []string{"b", "bb"}}, - {cursor: "ca", dir: storj.Before, limit: 2, more: true, result: []string{"bb", "c"}}, + {cursor: "aa0", dir: storj.Before, limit: 2, more: false, result: []string{"a00"}}, + {cursor: "bb0", dir: storj.Before, limit: 2, more: true, result: []string{"aa0", "b00"}}, + {cursor: "c00", dir: storj.Before, limit: 2, more: true, result: []string{"b00", "bb0"}}, + {cursor: "ca", dir: storj.Before, limit: 2, more: true, result: []string{"bb0", "c00"}}, } { errTag := fmt.Sprintf("%d. %+v", i, tt) diff --git a/pkg/miniogw/gateway_test.go b/pkg/miniogw/gateway_test.go index f3fd97bb7..7d774e09e 100644 --- a/pkg/miniogw/gateway_test.go +++ b/pkg/miniogw/gateway_test.go @@ -132,7 +132,7 @@ func TestListBuckets(t *testing.T) { assert.Empty(t, bucketInfos) // Create all expected buckets using the Metainfo API - bucketNames := []string{"bucket 1", "bucket 2", "bucket 3"} + bucketNames := []string{"bucket-1", "bucket-2", "bucket-3"} buckets := make([]storj.Bucket, len(bucketNames)) for i, bucketName := range bucketNames { bucket, err := m.CreateBucket(ctx, bucketName, nil) diff --git a/pkg/storage/segments/store_test.go b/pkg/storage/segments/store_test.go index 35a599704..b71bb8b0c 100644 --- a/pkg/storage/segments/store_test.go +++ b/pkg/storage/segments/store_test.go @@ -41,7 +41,7 @@ func TestSegmentStoreMeta(t *testing.T) { err string }{ {"l/path/1/2/3", []byte("content"), []byte("metadata"), time.Now().UTC().Add(time.Hour * 12), ""}, - {"l/not_exists_path/1/2/3", []byte{}, []byte{}, time.Now(), "key not found"}, + {"l/not-exists-path/1/2/3", []byte{}, []byte{}, time.Now(), "key not found"}, {"", []byte{}, []byte{}, time.Now(), "invalid segment component"}, } { test := tt @@ -86,7 +86,7 @@ func TestSegmentStorePutGet(t *testing.T) { content []byte }{ {"test inline put/get", "l/path/1", []byte("metadata-intline"), time.Time{}, createTestData(t, 2*memory.KiB.Int64())}, - {"test remote put/get", "s0/test_bucket/mypath/1", []byte("metadata-remote"), time.Time{}, createTestData(t, 100*memory.KiB.Int64())}, + {"test remote put/get", "s0/test-bucket/mypath/1", []byte("metadata-remote"), time.Time{}, createTestData(t, 100*memory.KiB.Int64())}, } { test := tt runTest(t, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet, segmentStore segments.Store) { @@ -120,7 +120,7 @@ func TestSegmentStoreDelete(t *testing.T) { content []byte }{ {"test inline delete", "l/path/1", []byte("metadata"), time.Time{}, createTestData(t, 2*memory.KiB.Int64())}, - {"test remote delete", "s0/test_bucket/mypath/1", []byte("metadata"), time.Time{}, createTestData(t, 100*memory.KiB.Int64())}, + {"test remote delete", "s0/test-bucket/mypath/1", []byte("metadata"), time.Time{}, createTestData(t, 100*memory.KiB.Int64())}, } { test := tt runTest(t, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet, segmentStore segments.Store) { @@ -156,11 +156,11 @@ func TestSegmentStoreList(t *testing.T) { path string content []byte }{ - {"l/AAAA/afile1", []byte("content")}, - {"l/AAAA/bfile2", []byte("content")}, - {"l/BBBB/afile1", []byte("content")}, - {"l/BBBB/bfile2", []byte("content")}, - {"l/BBBB/bfolder/file1", []byte("content")}, + {"l/aaaa/afile1", []byte("content")}, + {"l/aaaa/bfile2", []byte("content")}, + {"l/bbbb/afile1", []byte("content")}, + {"l/bbbb/bfile2", []byte("content")}, + {"l/bbbb/bfolder/file1", []byte("content")}, } for _, seg := range segments { segment := seg @@ -189,19 +189,19 @@ func TestSegmentStoreList(t *testing.T) { require.Equal(t, 2, len(items)) // should list only BBBB bucket - items, more, err = segmentStore.List(ctx, "l/BBBB", "", "", false, 10, meta.None) + items, more, err = segmentStore.List(ctx, "l/bbbb", "", "", false, 10, meta.None) require.NoError(t, err) require.False(t, more) require.Equal(t, 3, len(items)) // should list only BBBB bucket after afile1 - items, more, err = segmentStore.List(ctx, "l/BBBB", "afile1", "", false, 10, meta.None) + items, more, err = segmentStore.List(ctx, "l/bbbb", "afile1", "", false, 10, meta.None) require.NoError(t, err) require.False(t, more) require.Equal(t, 2, len(items)) // should list nothing - items, more, err = segmentStore.List(ctx, "l/CCCC", "", "", true, 10, meta.None) + items, more, err = segmentStore.List(ctx, "l/cccc", "", "", true, 10, meta.None) require.NoError(t, err) require.False(t, more) require.Equal(t, 0, len(items)) diff --git a/satellite/metainfo/metainfo_test.go b/satellite/metainfo/metainfo_test.go index 2d8437917..b15bbbc15 100644 --- a/satellite/metainfo/metainfo_test.go +++ b/satellite/metainfo/metainfo_test.go @@ -26,6 +26,7 @@ import ( "storj.io/storj/pkg/storj" "storj.io/storj/satellite" "storj.io/storj/satellite/console" + satMetainfo "storj.io/storj/satellite/metainfo" "storj.io/storj/uplink/metainfo" ) @@ -294,11 +295,6 @@ func TestCommitSegment(t *testing.T) { _, err = metainfo.CommitSegment(ctx, "bucket", "path", -1, nil, []*pb.OrderLimit2{}) require.Error(t, err) } - { - // error if bucket contains slash - _, err = metainfo.CommitSegment(ctx, "bucket/storj", "path", -1, &pb.Pointer{}, []*pb.OrderLimit2{}) - require.Error(t, err) - } { // error if number of remote pieces is lower then repair threshold redundancy := &pb.RedundancyScheme{ @@ -456,10 +452,10 @@ func TestDoubleCommitSegment(t *testing.T) { pointer, limits := runCreateSegment(ctx, t, metainfo) - _, err = metainfo.CommitSegment(ctx, "myBucketName", "file/path", -1, pointer, limits) + _, err = metainfo.CommitSegment(ctx, "my-bucket-name", "file/path", -1, pointer, limits) require.NoError(t, err) - _, err = metainfo.CommitSegment(ctx, "myBucketName", "file/path", -1, pointer, limits) + _, err = metainfo.CommitSegment(ctx, "my-bucket-name", "file/path", -1, pointer, limits) require.Error(t, err) require.Contains(t, err.Error(), "missing create request or request expired") }) @@ -535,7 +531,7 @@ func TestCommitSegmentPointer(t *testing.T) { pointer, limits := runCreateSegment(ctx, t, metainfo) test.Modify(pointer) - _, err = metainfo.CommitSegment(ctx, "myBucketName", "file/path", -1, pointer, limits) + _, err = metainfo.CommitSegment(ctx, "my-bucket-name", "file/path", -1, pointer, limits) require.Error(t, err) require.Contains(t, err.Error(), test.ErrorMessage) } @@ -587,7 +583,7 @@ func runCreateSegment(ctx context.Context, t *testing.T, metainfo metainfo.Clien expirationDate, err := ptypes.Timestamp(pointer.ExpirationDate) require.NoError(t, err) - addressedLimits, rootPieceID, err := metainfo.CreateSegment(ctx, "myBucketName", "file/path", -1, pointer.Remote.Redundancy, memory.MiB.Int64(), expirationDate) + addressedLimits, rootPieceID, err := metainfo.CreateSegment(ctx, "my-bucket-name", "file/path", -1, pointer.Remote.Redundancy, memory.MiB.Int64(), expirationDate) require.NoError(t, err) pointer.Remote.RootPieceId = rootPieceID @@ -629,3 +625,54 @@ func createTestPointer(t *testing.T) *pb.Pointer { } return pointer } + +func TestBucketNameValidation(t *testing.T) { + if !satMetainfo.BucketNameRestricted { + t.Skip("Skip until bucket name validation is not enabled") + } + + testplanet.Run(t, testplanet.Config{ + SatelliteCount: 1, StorageNodeCount: 6, UplinkCount: 1, + }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { + apiKey := planet.Uplinks[0].APIKey[planet.Satellites[0].ID()] + + metainfo, err := planet.Uplinks[0].DialMetainfo(ctx, planet.Satellites[0], apiKey) + require.NoError(t, err) + + rs := &pb.RedundancyScheme{ + MinReq: 1, + RepairThreshold: 1, + SuccessThreshold: 3, + Total: 4, + ErasureShareSize: 1024, + Type: pb.RedundancyScheme_RS, + } + + validNames := []string{ + "tes", "testbucket", + "test-bucket", "testbucket9", + "9testbucket", "a.b", + "test.bucket", "test-one.bucket-one", + "test.bucket.one", + "testbucket-63-0123456789012345678901234567890123456789012345abc", + } + for _, name := range validNames { + _, _, err = metainfo.CreateSegment(ctx, name, "", -1, rs, 1, time.Now()) + require.NoError(t, err, "bucket name: %v", name) + } + + invalidNames := []string{ + "", "t", "te", "-testbucket", + "testbucket-", "-testbucket-", + "a.b.", "test.bucket-.one", + "test.-bucket.one", "1.2.3.4", + "192.168.1.234", "testBUCKET", + "test/bucket", + "testbucket-64-0123456789012345678901234567890123456789012345abcd", + } + for _, name := range invalidNames { + _, _, err = metainfo.CreateSegment(ctx, name, "", -1, rs, 1, time.Now()) + require.Error(t, err, "bucket name: %v", name) + } + }) +} diff --git a/satellite/metainfo/validation.go b/satellite/metainfo/validation.go index 03dc72791..fe99bc0e5 100644 --- a/satellite/metainfo/validation.go +++ b/satellite/metainfo/validation.go @@ -6,12 +6,12 @@ package metainfo import ( "bytes" "context" + "regexp" "sync" "time" "github.com/gogo/protobuf/proto" "github.com/golang/protobuf/ptypes/timestamp" - "github.com/zeebo/errs" "go.uber.org/zap" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -23,7 +23,16 @@ import ( "storj.io/storj/satellite/console" ) -const requestTTL = time.Hour * 4 +const ( + // BucketNameRestricted feature flag to toggle bucket name validation + BucketNameRestricted = false + + requestTTL = time.Hour * 4 +) + +var ( + ipRegexp = regexp.MustCompile(`^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$`) +) // TTLItem keeps association between serial number and ttl type TTLItem struct { @@ -223,14 +232,64 @@ func (endpoint *Endpoint) validateBucket(ctx context.Context, bucket []byte) (er defer mon.Task()(&ctx)(&err) if len(bucket) == 0 { - return errs.New("bucket not specified") + return Error.New("bucket not specified") } - if bytes.ContainsAny(bucket, "/") { - return errs.New("bucket should not contain slash") + + if !BucketNameRestricted { + return nil } + + if len(bucket) < 3 || len(bucket) > 63 { + return Error.New("bucket name must be at least 3 and no more than 63 characters long") + } + + // 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) + if err != nil { + return err + } + } + + if ipRegexp.MatchString(string(bucket)) { + return Error.New("bucket name cannot be formatted as an IP address") + } + return nil } +func validateBucketLabel(label []byte) error { + if len(label) == 0 { + return Error.New("bucket label cannot be empty") + } + + if !isLowerLetter(label[0]) && !isDigit(label[0]) { + return Error.New("bucket label must start with a lowercase letter or number") + } + + if label[0] == '-' || label[len(label)-1] == '-' { + return Error.New("bucket label cannot start or end with a hyphen") + } + + for i := 1; i < len(label)-1; i++ { + if !isLowerLetter(label[i]) && !isDigit(label[i]) && (label[i] != '-') && (label[i] != '.') { + return Error.New("bucket name must contain only lowercase letters, numbers or hyphens") + } + } + + return nil +} + +func isLowerLetter(r byte) bool { + return r >= 'a' && r <= 'z' +} + +func isDigit(r byte) bool { + return r >= '0' && r <= '9' +} + func (endpoint *Endpoint) validatePointer(ctx context.Context, pointer *pb.Pointer) (err error) { defer mon.Task()(&ctx)(&err)