From 11ccd7325f2ed875644f295141957f5bd3e1e1fa Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Mon, 4 Nov 2019 12:22:22 -0800 Subject: [PATCH] Disallow uid as a predicate name. (#4219) uid is used internally to report the UIDs of the nodes in Dgraph. Users should get an error when trying to add a predicate of that name to the schema or when sending mutations with uid as a predicate. This PR enables checks for those situations. --- systest/mutations_test.go | 19 +++++++++++++++++++ worker/draft.go | 4 +++- worker/mutation.go | 12 +++++++++--- worker/mutation_test.go | 7 ++++++- x/keys.go | 13 +++++++++++++ 5 files changed, 50 insertions(+), 5 deletions(-) diff --git a/systest/mutations_test.go b/systest/mutations_test.go index 8244fc326bd..7f690fb226e 100644 --- a/systest/mutations_test.go +++ b/systest/mutations_test.go @@ -85,6 +85,7 @@ func TestSystem(t *testing.T) { t.Run("drop type without specified type", wrap(DropTypeNoValue)) t.Run("reverse count index", wrap(ReverseCountIndex)) t.Run("type predicate check", wrap(TypePredicateCheck)) + t.Run("internal predicate check", wrap(InternalPredicateCheck)) } func FacetJsonInputSupportsAnyOfTerms(t *testing.T, c *dgo.Dgraph) { @@ -1715,3 +1716,21 @@ func TypePredicateCheck(t *testing.T, c *dgo.Dgraph) { err = c.Alter(ctx, op) require.NoError(t, err) } + +func InternalPredicateCheck(t *testing.T, c *dgo.Dgraph) { + // Schema update is rejected because uid is reserved for internal use. + op := &api.Operation{} + op.Schema = `uid: string .` + ctx := context.Background() + err := c.Alter(ctx, op) + require.Error(t, err) + require.Contains(t, err.Error(), "Cannot create user-defined predicate with internal name uid") + + txn := c.NewTxn() + _, err = txn.Mutate(ctx, &api.Mutation{ + CommitNow: true, + SetNquads: []byte(`_:bob "bobId" .`), + }) + require.Error(t, err) + require.Contains(t, err.Error(), "Cannot create user-defined predicate with internal name uid") +} diff --git a/worker/draft.go b/worker/draft.go index 5585bf9a17b..a805371a66f 100644 --- a/worker/draft.go +++ b/worker/draft.go @@ -253,7 +253,9 @@ func (n *node) applyMutations(ctx context.Context, proposal *pb.Proposal) (rerr for attr, storageType := range schemaMap { if _, err := schema.State().TypeOf(attr); err != nil { - createSchema(attr, storageType) + if err := createSchema(attr, storageType); err != nil { + return err + } } } diff --git a/worker/mutation.go b/worker/mutation.go index fce52a2575f..ea13f787297 100644 --- a/worker/mutation.go +++ b/worker/mutation.go @@ -190,7 +190,7 @@ func updateSchema(s *pb.SchemaUpdate) error { return txn.CommitAt(1, nil) } -func createSchema(attr string, typ types.TypeID) { +func createSchema(attr string, typ types.TypeID) error { // Don't overwrite schema blindly, acl's might have been set even though // type is not present s, ok := schema.State().Get(attr) @@ -204,9 +204,10 @@ func createSchema(attr string, typ types.TypeID) { s.List = true } } - if err := updateSchema(&s); err != nil { - glog.Errorf("Error while updating schema: %+v", err) + if err := checkSchema(&s); err != nil { + return err } + return updateSchema(&s) } func runTypeMutation(ctx context.Context, update *pb.TypeUpdate) error { @@ -263,6 +264,11 @@ func checkSchema(s *pb.SchemaUpdate) error { return errors.Errorf("No predicate specified in schema mutation") } + if x.IsInternalPredicate(s.Predicate) { + return errors.Errorf("Cannot create user-defined predicate with internal name %s", + s.Predicate) + } + if s.Directive == pb.SchemaUpdate_INDEX && len(s.Tokenizer) == 0 { return errors.Errorf("Tokenizer must be specified while indexing a predicate: %+v", s) } diff --git a/worker/mutation_test.go b/worker/mutation_test.go index 59a949ac186..dd12c7e271a 100644 --- a/worker/mutation_test.go +++ b/worker/mutation_test.go @@ -191,12 +191,17 @@ func TestCheckSchema(t *testing.T) { s1 = &pb.SchemaUpdate{Predicate: "friend", ValueType: pb.Posting_UID, Directive: pb.SchemaUpdate_REVERSE} require.NoError(t, checkSchema(s1)) + // Schema with internal predicate. + s1 = &pb.SchemaUpdate{Predicate: "uid", ValueType: pb.Posting_STRING} + require.Error(t, checkSchema(s1)) + s := `jobs: string @upsert .` result, err := schema.Parse(s) require.NoError(t, err) err = checkSchema(result.Preds[0]) require.Error(t, err) - require.Equal(t, "Index tokenizer is mandatory for: [jobs] when specifying @upsert directive", err.Error()) + require.Equal(t, "Index tokenizer is mandatory for: [jobs] when specifying @upsert directive", + err.Error()) s = ` jobs : string @index(exact) @upsert . diff --git a/x/keys.go b/x/keys.go index 3608e7cf062..2e04156c6f4 100644 --- a/x/keys.go +++ b/x/keys.go @@ -559,6 +559,13 @@ var aclPredicateMap = map[string]struct{}{ "dgraph.group.acl": {}, } +// internalPredicateMap stores a set of Dgraph's internal predicate. An internal +// predicate is a predicate that has a special meaning in Dgraph and its query +// language and should not be allowed as a user-defined predicate. +var internalPredicateMap = map[string]struct{}{ + "uid": {}, +} + // IsReservedPredicate returns true if the predicate is in the reserved predicate list. func IsReservedPredicate(pred string) bool { _, ok := reservedPredicateMap[strings.ToLower(pred)] @@ -583,3 +590,9 @@ func ReservedPredicates() []string { } return preds } + +// IsInternalPredicate returns true if the predicate is in the internal predicate list. +func IsInternalPredicate(pred string) bool { + _, ok := internalPredicateMap[strings.ToLower(pred)] + return ok +}