diff --git a/db.go b/db.go index 23f678bf6..9348c97b6 100644 --- a/db.go +++ b/db.go @@ -48,7 +48,12 @@ type DB struct { } // NewDB returns a new instance of DB. -func NewDB(path, name string) *DB { +func NewDB(path, name string) (*DB, error) { + err := ValidateName(name) + if err != nil { + return nil, err + } + return &DB{ path: path, name: name, @@ -61,7 +66,7 @@ func NewDB(path, name string) *DB { stats: NopStatsClient, LogOutput: ioutil.Discard, - } + }, nil } // Name returns name of the database. @@ -141,7 +146,10 @@ func (db *DB) openFrames() error { continue } - fr := db.newFrame(db.FramePath(filepath.Base(fi.Name())), filepath.Base(fi.Name())) + fr, err := db.newFrame(db.FramePath(filepath.Base(fi.Name())), filepath.Base(fi.Name())) + if err != nil { + return ErrName + } if err := fr.Open(); err != nil { return fmt.Errorf("open frame: name=%s, err=%s", fr.Name(), err) } @@ -312,12 +320,16 @@ func (db *DB) CreateFrameIfNotExists(name string, opt FrameOptions) (*Frame, err } func (db *DB) createFrame(name string, opt FrameOptions) (*Frame, error) { + if name == "" { return nil, errors.New("frame name required") } // Initialize frame. - f := db.newFrame(db.FramePath(name), name) + f, err := db.newFrame(db.FramePath(name), name) + if err != nil { + return nil, err + } // Open frame. if err := f.Open(); err != nil { @@ -335,11 +347,14 @@ func (db *DB) createFrame(name string, opt FrameOptions) (*Frame, error) { return f, nil } -func (db *DB) newFrame(path, name string) *Frame { - f := NewFrame(path, db.name, name) +func (db *DB) newFrame(path, name string) (*Frame, error) { + f, err := NewFrame(path, db.name, name) + if err != nil { + return nil, err + } f.LogOutput = db.LogOutput f.stats = db.stats.WithTags(fmt.Sprintf("frame:%s", name)) - return f + return f, nil } // DeleteFrame removes a frame from the database. diff --git a/db_test.go b/db_test.go index b95f89e62..96120dc1e 100644 --- a/db_test.go +++ b/db_test.go @@ -89,7 +89,11 @@ func NewDB() *DB { if err != nil { panic(err) } - return &DB{DB: pilosa.NewDB(path, "d")} + db, err := pilosa.NewDB(path, "d") + if err != nil { + panic(err) + } + return &DB{DB: db} } // MustOpenDB returns a new, opened database at a temporary path. Panic on error. @@ -109,12 +113,16 @@ func (db *DB) Close() error { // Reopen closes the database and reopens it. func (db *DB) Reopen() error { + var err error if err := db.DB.Close(); err != nil { return err } path, name := db.Path(), db.Name() - db.DB = pilosa.NewDB(path, name) + db.DB, err = pilosa.NewDB(path, name) + if err != nil { + return err + } if err := db.Open(); err != nil { return err @@ -130,3 +138,15 @@ func (db *DB) MustSetBit(name string, bitmapID, profileID uint64, t *time.Time) } return changed } + +// Ensure database can delete a frame. +func TestDB_InvalidName(t *testing.T) { + path, err := ioutil.TempDir("", "pilosa-db-") + if err != nil { + panic(err) + } + db, err := pilosa.NewDB(path, "ABC") + if db != nil { + t.Fatalf("unexpected db name %s", db) + } +} diff --git a/executor_test.go b/executor_test.go index 1ad1fbbbb..50be72abb 100644 --- a/executor_test.go +++ b/executor_test.go @@ -182,7 +182,7 @@ func TestExecutor_Execute_SetBitmapAttrs(t *testing.T) { db := idx.MustCreateDBIfNotExists("d", pilosa.DBOptions{}) if _, err := db.CreateFrameIfNotExists("f", pilosa.FrameOptions{}); err != nil { t.Fatal(err) - } else if _, err := db.CreateFrameIfNotExists("XXX", pilosa.FrameOptions{}); err != nil { + } else if _, err := db.CreateFrameIfNotExists("xxx", pilosa.FrameOptions{}); err != nil { t.Fatal(err) } @@ -195,7 +195,7 @@ func TestExecutor_Execute_SetBitmapAttrs(t *testing.T) { if _, err := e.Execute(context.Background(), "d", MustParse(`SetBitmapAttrs(id=200, frame=f, YYY=1)`), nil, nil); err != nil { t.Fatal(err) } - if _, err := e.Execute(context.Background(), "d", MustParse(`SetBitmapAttrs(id=10, frame=XXX, YYY=1)`), nil, nil); err != nil { + if _, err := e.Execute(context.Background(), "d", MustParse(`SetBitmapAttrs(id=10, frame=xxx, YYY=1)`), nil, nil); err != nil { t.Fatal(err) } if _, err := e.Execute(context.Background(), "d", MustParse(`SetBitmapAttrs(id=10, frame=f, baz=123, bat=true)`), nil, nil); err != nil { diff --git a/frame.go b/frame.go index efbf5f13c..fa47517c5 100644 --- a/frame.go +++ b/frame.go @@ -46,7 +46,12 @@ type Frame struct { } // NewFrame returns a new instance of frame. -func NewFrame(path, db, name string) *Frame { +func NewFrame(path, db, name string) (*Frame, error) { + err := ValidateName(name) + if err != nil { + return nil, err + } + return &Frame{ path: path, db: db, @@ -60,7 +65,7 @@ func NewFrame(path, db, name string) *Frame { rowLabel: DefaultRowLabel, LogOutput: ioutil.Discard, - } + }, nil } // Name returns the name the frame was initialized with. diff --git a/frame_test.go b/frame_test.go index 186db0c62..042bd751e 100644 --- a/frame_test.go +++ b/frame_test.go @@ -65,8 +65,11 @@ func NewFrame() *Frame { if err != nil { panic(err) } - - return &Frame{Frame: pilosa.NewFrame(path, "d", "f")} + frame, err := pilosa.NewFrame(path, "d", "f") + if err != nil { + panic(err) + } + return &Frame{Frame: frame} } // MustOpenFrame returns a new, opened frame at a temporary path. Panic on error. @@ -86,15 +89,31 @@ func (f *Frame) Close() error { // Reopen closes the database and reopens it. func (f *Frame) Reopen() error { + var err error if err := f.Frame.Close(); err != nil { return err } path, db, name := f.Path(), f.DB(), f.Name() - f.Frame = pilosa.NewFrame(path, db, name) + f.Frame, err = pilosa.NewFrame(path, db, name) + if err != nil { + return err + } if err := f.Open(); err != nil { return err } return nil } + +// NewFrame returns a new instance of Frame d/0. +func TestFrame_NameRestriction(t *testing.T) { + path, err := ioutil.TempDir("", "pilosa-frame-") + if err != nil { + panic(err) + } + frame, err := pilosa.NewFrame(path, "d", "ABC") + if frame != nil { + t.Fatalf("unexpected frame name %s", err) + } +} diff --git a/index.go b/index.go index 7885d87dd..ee4710037 100644 --- a/index.go +++ b/index.go @@ -78,7 +78,10 @@ func (i *Index) Open() error { i.logger().Printf("opening database: %s", filepath.Base(fi.Name())) - db := i.newDB(i.DBPath(filepath.Base(fi.Name())), filepath.Base(fi.Name())) + db, err := i.newDB(i.DBPath(filepath.Base(fi.Name())), filepath.Base(fi.Name())) + if err != nil { + return ErrName + } if err := db.Open(); err != nil { return fmt.Errorf("open db: name=%s, err=%s", db.Name(), err) } @@ -193,7 +196,11 @@ func (i *Index) createDB(name string, opt DBOptions) (*DB, error) { } // Otherwise create a new database. - db := i.newDB(i.DBPath(name), name) + db, err := i.newDB(i.DBPath(name), name) + if err != nil { + return nil, err + } + if err := db.Open(); err != nil { return nil, err } @@ -208,11 +215,14 @@ func (i *Index) createDB(name string, opt DBOptions) (*DB, error) { return db, nil } -func (i *Index) newDB(path, name string) *DB { - db := NewDB(path, name) +func (i *Index) newDB(path, name string) (*DB, error) { + db, err := NewDB(path, name) + if err != nil { + return nil, err + } db.LogOutput = i.LogOutput db.stats = i.Stats.WithTags(fmt.Sprintf("db:%s", db.Name())) - return db + return db, nil } // DeleteDB removes a database from the index. diff --git a/pilosa.go b/pilosa.go index a136cb56f..b3620a900 100644 --- a/pilosa.go +++ b/pilosa.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/pilosa/pilosa/internal" + "regexp" ) // System errors. @@ -18,10 +19,18 @@ var ( ErrFrameExists = errors.New("frame already exists") ErrFrameNotFound = errors.New("frame not found") + // ErrFrameRequired is returned when no frame is specified. + ErrName = errors.New("name restricted to [a-z0-9_-]") + + // ErrFragmentNotFound is returned when a fragment does not exist. ErrFragmentNotFound = errors.New("fragment not found") ErrQueryRequired = errors.New("query required") ) +// Regular expression to valuate db and frame's name +// Todo: remove . when frame doesn't require . for topN +var nameRegexp = regexp.MustCompile(`^([a-z0-9._-]{1,64}$)`) + // Profile represents vertical column in a database. // A profile can have a set of attributes attached to it. type Profile struct { @@ -74,3 +83,13 @@ func decodeProfile(pb *internal.Profile) *Profile { // TimeFormat is the go-style time format used to parse string dates. const TimeFormat = "2006-01-02T15:04" + + +// Restrict name using regex +func ValidateName(name string) error { + validName := nameRegexp.Match([]byte(name)) + if validName == false{ + return ErrName + } + return nil +} \ No newline at end of file diff --git a/pilosactl/import.go b/pilosactl/import.go index 9d4ad63f4..313c5da99 100644 --- a/pilosactl/import.go +++ b/pilosactl/import.go @@ -104,7 +104,6 @@ func (cmd *ImportCommand) Run(ctx context.Context) error { } else if len(cmd.Paths) == 0 { return errors.New("path required") } - // Create a client to the server. client, err := pilosa.NewClient(cmd.Host) if err != nil {