Skip to content

Commit

Permalink
fix: add quotation marks to bucket name validation
Browse files Browse the repository at this point in the history
  • Loading branch information
AlirieGray committed Aug 5, 2020
1 parent e82f2d9 commit 3d2ea1e
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
1. [19043](https://github.com/influxdata/influxdb/pull/19043): Enforce all influx CLI flag args are valid
1. [19188](https://github.com/influxdata/influxdb/pull/19188): Dashboard cells correctly map results when multiple queries exist
1. [19146](https://github.com/influxdata/influxdb/pull/19146): Dashboard cells and overlay use UTC as query time when toggling to UTC timezone
1. [19222](https://github.com/influxdata/influxdb/pull/19222): Bucket names may not include quotation marks

## v2.0.0-beta.15 [2020-07-23]

Expand Down
5 changes: 1 addition & 4 deletions http/bucket_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,10 +361,7 @@ func (b *postBucketRequest) OK() error {

// names starting with an underscore are reserved for system buckets
if err := validBucketName(b.toInfluxDB()); err != nil {
return &influxdb.Error{
Code: influxdb.EUnprocessableEntity,
Msg: err.Error(),
}
return err
}

return nil
Expand Down
13 changes: 9 additions & 4 deletions tenant/http_server_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,7 @@ func (b *postBucketRequest) OK() error {

// names starting with an underscore are reserved for system buckets
if err := validBucketName(b.toInfluxDB()); err != nil {
return &influxdb.Error{
Code: influxdb.EUnprocessableEntity,
Msg: err.Error(),
}
return err
}

return nil
Expand Down Expand Up @@ -498,5 +495,13 @@ func validBucketName(bucket *influxdb.Bucket) error {
Msg: fmt.Sprintf("bucket name %s is invalid. Buckets may not start with underscore", bucket.Name),
}
}
// quotation marks will cause queries to fail
if strings.Contains(bucket.Name, "\"") {
return &influxdb.Error{
Code: influxdb.EInvalid,
Op: "http/bucket",
Msg: fmt.Sprintf("bucket name %s is invalid. Bucket names may not include quotation marks", bucket.Name),
}
}
return nil
}
2 changes: 1 addition & 1 deletion tenant/http_server_bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,5 @@ func initBucketHttpService(f itesting.BucketFields, t *testing.T) (influxdb.Buck
}

func TestBucketService(t *testing.T) {
itesting.BucketService(initBucketHttpService, t, itesting.WithoutHooks())
itesting.BucketService(initBucketHttpService, t, itesting.WithoutHooks(), itesting.WithHTTPValidation())
}
113 changes: 107 additions & 6 deletions testing/bucket_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,24 @@ var bucketCmpOptions = cmp.Options{
}

type BucketSvcOpts struct {
NoHooks bool
NoHooks bool
HTTPValidation bool
}

type BucketOptsFn func(opts *BucketSvcOpts)

// WithoutHooks allows the test suite to be run without being able to hook into the underlying implementation of theservice
// in most cases that is to remove specific id generation controls.
func WithoutHooks() BucketSvcOpts {
return BucketSvcOpts{
NoHooks: true,
func WithoutHooks() BucketOptsFn {
return func(opts *BucketSvcOpts) {
opts.NoHooks = true
}
}

// WithHTTPValidation skips tests involving name validation that only occurs at the HTTP layer
func WithHTTPValidation() BucketOptsFn {
return func(opts *BucketSvcOpts) {
opts.HTTPValidation = true
}
}

Expand All @@ -76,7 +86,7 @@ type bucketServiceF func(
func BucketService(
init func(BucketFields, *testing.T) (influxdb.BucketService, string, func()),
t *testing.T,
opts ...BucketSvcOpts) {
opts ...BucketOptsFn) {
tests := []struct {
name string
fn bucketServiceF
Expand All @@ -89,6 +99,10 @@ func BucketService(
name: "IDUnique",
fn: IDUnique,
},
{
name: "HTTPValidation",
fn: HTTPValidation,
},
{
name: "FindBucketByID",
fn: FindBucketByID,
Expand All @@ -110,8 +124,19 @@ func BucketService(
fn: DeleteBucket,
},
}
opt := BucketSvcOpts{
NoHooks: false,
HTTPValidation: false,
}
for _, o := range opts {
o(&opt)
}

for _, tt := range tests {
if tt.name == "IDUnique" && len(opts) > 0 && opts[0].NoHooks {
if tt.name == "IDUnique" && opt.NoHooks {
continue
}
if tt.name == "HTTPValidation" && !opt.HTTPValidation {
continue
}
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -385,6 +410,82 @@ func CreateBucket(
}
}

func HTTPValidation(
init func(BucketFields, *testing.T) (influxdb.BucketService, string, func()),
t *testing.T,
) {
type args struct {
bucket *influxdb.Bucket
}
type wants struct {
err error
buckets []*influxdb.Bucket
}
tests := []struct {
name string
fields BucketFields
args args
wants wants
}{
{
name: "create bucket with illegal quotation mark",
fields: BucketFields{
IDGenerator: mock.NewIDGenerator(bucketOneID, t),
OrgBucketIDs: mock.NewIDGenerator(bucketOneID, t),
TimeGenerator: mock.TimeGenerator{FakeValue: time.Date(2006, 5, 4, 1, 2, 3, 0, time.UTC)},
Buckets: []*influxdb.Bucket{},
Organizations: []*influxdb.Organization{},
},
args: args{
bucket: &influxdb.Bucket{
Name: "namewith\"quote",
OrgID: MustIDBase16(orgOneID),
},
},
wants: wants{
buckets: []*influxdb.Bucket{},
err: &influxdb.Error{
Code: influxdb.EInvalid,
Msg: "bucket name namewith\"quote is invalid. Bucket names may not include quotation marks",
Op: influxdb.OpCreateBucket,
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s, opPrefix, done := init(tt.fields, t)
defer done()
ctx := context.Background()
err := s.CreateBucket(ctx, tt.args.bucket)
diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t)

// Delete only newly created buckets - ie., with a not nil ID
// if tt.args.bucket.ID.Valid() {
defer s.DeleteBucket(ctx, tt.args.bucket.ID)
// }

buckets, _, err := s.FindBuckets(ctx, influxdb.BucketFilter{})
if err != nil {
t.Fatalf("failed to retrieve buckets: %v", err)
}

// remove system buckets
filteredBuckets := []*influxdb.Bucket{}
for _, b := range buckets {
if b.Type != influxdb.BucketTypeSystem {
filteredBuckets = append(filteredBuckets, b)
}
}

if diff := cmp.Diff(filteredBuckets, tt.wants.buckets, bucketCmpOptions...); diff != "" {
t.Errorf("buckets are different -got/+want\ndiff %s", diff)
}
})
}
}

// CreateBucket testing
func IDUnique(
init func(BucketFields, *testing.T) (influxdb.BucketService, string, func()),
Expand Down

0 comments on commit 3d2ea1e

Please sign in to comment.