Skip to content

Commit

Permalink
Disallow uid as a predicate name. (#4219)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
martinmr authored Nov 4, 2019
1 parent 07ed842 commit 11ccd73
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 5 deletions.
19 changes: 19 additions & 0 deletions systest/mutations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 <uid> "bobId" .`),
})
require.Error(t, err)
require.Contains(t, err.Error(), "Cannot create user-defined predicate with internal name uid")
}
4 changes: 3 additions & 1 deletion worker/draft.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand Down
12 changes: 9 additions & 3 deletions worker/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
7 changes: 6 additions & 1 deletion worker/mutation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 .
Expand Down
13 changes: 13 additions & 0 deletions x/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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
}

0 comments on commit 11ccd73

Please sign in to comment.