Influxdata
/
influxdb
mirror of https://github.com/influxdata/influxdb.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix(http/bucket): lift bucket name validation to http layer (#16125) 

This removes the bucket name validation from the KV BucketService,
and moves it to the http implementation of the service.
The effect is that API user requests still get validated but direct KV access does not
pull/16140/head
Nathaniel Cook 2019-12-05 10:30:08 -07:00 committed by GitHub
parent 5f19c6cace
commit 423dc9703c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 124 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
http/bucket_service.go
View File

@ -7,6 +7,7 @@ import (
"fmt"
"net/http"
"path"
"strings"
"time"
"github.com/influxdata/httprouter"
@ -327,6 +328,12 @@ func (h *BucketHandler) handlePostBucket(w http.ResponseWriter, r *http.Request)
return
}
// names starting with an underscore are reserved for system buckets
if err := validBucketName(bucket); err != nil {
h.HandleHTTPError(ctx, err, w)
return
}
if err := h.BucketService.CreateBucket(ctx, bucket); err != nil {
h.HandleHTTPError(ctx, err, w)
return
@ -639,6 +646,19 @@ func (h *BucketHandler) handlePatchBucket(w http.ResponseWriter, r *http.Request
return
}
if req.Update.Name != nil {
b, err := h.BucketService.FindBucketByID(ctx, req.BucketID)
if err != nil {
h.HandleHTTPError(ctx, err, w)
return
}
b.Name = *req.Update.Name
if err := validBucketName(b); err != nil {
h.HandleHTTPError(ctx, err, w)
return
}
}
b, err := h.BucketService.UpdateBucket(ctx, req.BucketID, req.Update)
if err != nil {
h.HandleHTTPError(ctx, err, w)
@ -988,3 +1008,16 @@ func (s *BucketService) DeleteBucket(ctx context.Context, id influxdb.ID) error
return CheckError(resp)
}
// validBucketName reports any errors with bucket names
func validBucketName(bucket *influxdb.Bucket) error {
// names starting with an underscore are reserved for system buckets
if strings.HasPrefix(bucket.Name, "_") && bucket.Type != influxdb.BucketTypeSystem {
return &influxdb.Error{
Code: influxdb.EInvalid,
Op: "http/bucket",
Msg: fmt.Sprintf("bucket name %s is invalid. Buckets may not start with underscore", bucket.Name),
}
}
return nil
}

91
http/bucket_test.go
View File

@ -12,6 +12,7 @@ import (
"time"
"github.com/influxdata/httprouter"
"github.com/influxdata/influxdb"
platform "github.com/influxdata/influxdb"
"github.com/influxdata/influxdb/inmem"
"github.com/influxdata/influxdb/kv"
@ -431,11 +432,37 @@ func TestService_handlePostBucket(t *testing.T) {
`,
},
},
{
name: "create a new bucket with invalid name",
fields: fields{
BucketService: &mock.BucketService{
CreateBucketFn: func(ctx context.Context, c *platform.Bucket) error {
c.ID = platformtesting.MustIDBase16("020f755c3c082000")
return nil
},
},
OrganizationService: &mock.OrganizationService{
FindOrganizationF: func(ctx context.Context, f platform.OrganizationFilter) (*platform.Organization, error) {
return &platform.Organization{ID: platformtesting.MustIDBase16("6f626f7274697320")}, nil
},
},
},
args: args{
bucket: &platform.Bucket{
Name: "_hello",
OrgID: platformtesting.MustIDBase16("6f626f7274697320"),
},
},
wants: wants{
statusCode: http.StatusBadRequest,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
bucketBackend := NewMockBucketBackend(t)
bucketBackend.HTTPErrorHandler = ErrorHandler(0)
bucketBackend.BucketService = tt.fields.BucketService
bucketBackend.OrganizationService = tt.fields.OrganizationService
h := NewBucketHandler(zaptest.NewLogger(t), bucketBackend)
@ -600,6 +627,13 @@ func TestService_handlePatchBucket(t *testing.T) {
name: "update a bucket name and retention",
fields: fields{
&mock.BucketService{
FindBucketByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.Bucket, error) {
return &platform.Bucket{
ID: platformtesting.MustIDBase16("020f755c3c082000"),
Name: "hello",
OrgID: platformtesting.MustIDBase16("020f755c3c082000"),
}, nil
},
UpdateBucketFn: func(ctx context.Context, id platform.ID, upd platform.BucketUpdate) (*platform.Bucket, error) {
if id == platformtesting.MustIDBase16("020f755c3c082000") {
d := &platform.Bucket{
@ -654,10 +688,50 @@ func TestService_handlePatchBucket(t *testing.T) {
`,
},
},
{
name: "update a bucket name invalid",
fields: fields{
&mock.BucketService{
FindBucketByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.Bucket, error) {
if id == platformtesting.MustIDBase16("020f755c3c082000") {
return &platform.Bucket{
ID: platformtesting.MustIDBase16("020f755c3c082000"),
Name: "hello",
OrgID: platformtesting.MustIDBase16("020f755c3c082000"),
}, nil
}
return nil, fmt.Errorf("not found")
},
UpdateBucketFn: func(ctx context.Context, id platform.ID, upd platform.BucketUpdate) (*platform.Bucket, error) {
if id == platformtesting.MustIDBase16("020f755c3c082000") {
return &platform.Bucket{
ID: platformtesting.MustIDBase16("020f755c3c082000"),
Name: "hello",
OrgID: platformtesting.MustIDBase16("020f755c3c082000"),
}, nil
}
return nil, fmt.Errorf("not found")
},
},
},
args: args{
id: "020f755c3c082000",
name: "_example",
},
wants: wants{
statusCode: http.StatusBadRequest,
},
},
{
name: "bucket not found",
fields: fields{
&mock.BucketService{
FindBucketByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.Bucket, error) {
return nil, &platform.Error{
Code: platform.ENotFound,
Msg: "bucket not found",
}
},
UpdateBucketFn: func(ctx context.Context, id platform.ID, upd platform.BucketUpdate) (*platform.Bucket, error) {
return nil, &platform.Error{
Code: platform.ENotFound,
@ -679,6 +753,16 @@ func TestService_handlePatchBucket(t *testing.T) {
name: "update bucket to no retention and new name",
fields: fields{
&mock.BucketService{
FindBucketByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.Bucket, error) {
if id == platformtesting.MustIDBase16("020f755c3c082000") {
return &platform.Bucket{
ID: platformtesting.MustIDBase16("020f755c3c082000"),
Name: "hello",
OrgID: platformtesting.MustIDBase16("020f755c3c082000"),
}, nil
}
return nil, fmt.Errorf("not found")
},
UpdateBucketFn: func(ctx context.Context, id platform.ID, upd platform.BucketUpdate) (*platform.Bucket, error) {
if id == platformtesting.MustIDBase16("020f755c3c082000") {
d := &platform.Bucket{
@ -797,6 +881,13 @@ func TestService_handlePatchBucket(t *testing.T) {
name: "update a bucket name with invalid retention policy is an error",
fields: fields{
&mock.BucketService{
FindBucketByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.Bucket, error) {
return &platform.Bucket{
ID: platformtesting.MustIDBase16("020f755c3c082000"),
Name: "hello",
OrgID: platformtesting.MustIDBase16("020f755c3c082000"),
}, nil
},
UpdateBucketFn: func(ctx context.Context, id platform.ID, upd platform.BucketUpdate) (*platform.Bucket, error) {
if id == platformtesting.MustIDBase16("020f755c3c082000") {
d := &platform.Bucket{

16
kv/bucket.go
View File

@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"fmt"
"strings"
"time"
"github.com/influxdata/influxdb"
@ -648,11 +647,6 @@ func (s *Service) validBucketName(ctx context.Context, tx Tx, b *influxdb.Bucket
return BucketAlreadyExistsError(b)
}
// names starting with an underscore are reserved for system buckets
if strings.HasPrefix(b.Name, "_") && b.Type != influxdb.BucketTypeSystem {
return ReservedBucketNameError(b)
}
return err
}
@ -911,13 +905,3 @@ func BucketAlreadyExistsError(b *influxdb.Bucket) error {
Msg: fmt.Sprintf("bucket with name %s already exists", b.Name),
}
}
// ReservedBucketNameError is used when creating a bucket with a name that
// starts with an underscore.
func ReservedBucketNameError(b *influxdb.Bucket) error {
return &influxdb.Error{
Code: influxdb.EInvalid,
Op: "kv/bucket",
Msg: fmt.Sprintf("bucket name %s is invalid. Buckets may not start with underscore", b.Name),
}
}