Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject requests with predicates larger than the max size allowed. #3052

Merged
merged 3 commits into from
Feb 23, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,13 +348,17 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
return empty, err
}

// Reserved predicates cannot be altered.
for _, update := range result.Schemas {
// Reserved predicates cannot be altered.
if x.IsReservedPredicate(update.Predicate) {
err := fmt.Errorf("predicate %s is reserved and is not allowed to be modified",
update.Predicate)
return nil, err
}

if err := validatePredName(update.Predicate); err != nil {
return nil, err
}
}

glog.Infof("Got schema: %+v\n", result.Schemas)
Expand Down Expand Up @@ -560,6 +564,11 @@ func (s *Server) doQuery(ctx context.Context, req *api.Request) (resp *api.Respo
if err != nil {
return resp, err
}

if err = validateQuery(parsedReq.Query); err != nil {
return resp, err
}

if req.StartTs == 0 {
req.StartTs = State.getTimestamp(req.ReadOnly)
}
Expand Down Expand Up @@ -769,6 +778,9 @@ func validateAndConvertFacets(nquads []*api.NQuad) error {

func validateNQuads(set, del []*api.NQuad) error {
for _, nq := range set {
if err := validatePredName(nq.Predicate); err != nil {
return err
}
var ostar bool
if o, ok := nq.ObjectValue.GetVal().(*api.Value_DefaultVal); ok {
ostar = o.DefaultVal == x.Star
Expand All @@ -781,6 +793,9 @@ func validateNQuads(set, del []*api.NQuad) error {
}
}
for _, nq := range del {
if err := validatePredName(nq.Predicate); err != nil {
return err
}
var ostar bool
if o, ok := nq.ObjectValue.GetVal().(*api.Value_DefaultVal); ok {
ostar = o.DefaultVal == x.Star
Expand Down Expand Up @@ -821,3 +836,27 @@ func validateKeys(nq *api.NQuad) error {
}
return nil
}

// validateQuery verifies that the query does not contain any preds that
// are longer than the limit (2^16).
func validateQuery(queries []*gql.GraphQuery) error {
for _, q := range queries {
if err := validatePredName(q.Attr); err != nil {
return err
}

if err := validateQuery(q.Children); err != nil {
return err
}
}

return nil
}

func validatePredName(name string) error {
if len(name) > math.MaxUint16 {
return fmt.Errorf("Predicate name length cannot be bigger than 2^16. Predicate: %v",
name[:80])
martinmr marked this conversation as resolved.
Show resolved Hide resolved
}
return nil
}
25 changes: 25 additions & 0 deletions query/query3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package query
import (
"context"
"encoding/json"
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -1949,3 +1951,26 @@ func TestMultipleTypeDirectivesInPredicate(t *testing.T) {
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"me":[{"enemy":[{"name":"Margaret", "pet":[{"name":"Bear"}]}, {"name":"Leonard"}]}]}}`, js)
}

func TestMaxPredicateSize(t *testing.T) {
// Create a string that has more than than 2^16 chars.
var b strings.Builder
for i := 0; i < 10000; i++ {
b.WriteString("abcdefg")
}
largePred := b.String()

query := fmt.Sprintf(`
{
me(func: uid(0x2)) {
%s {
name
}
}
}
`, largePred)

_, err := processQuery(t, context.Background(), query)
require.Error(t, err)
require.Contains(t, err.Error(), "Predicate name length cannot be bigger than 2^16")
}
38 changes: 38 additions & 0 deletions systest/mutations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func TestSystem(t *testing.T) {
t.Run("has should not have deleted edge", wrap(HasDeletedEdge))
t.Run("has should have reverse edges", wrap(HasReverseEdge))
t.Run("facet json input supports anyofterms query", wrap(FacetJsonInputSupportsAnyOfTerms))
t.Run("max predicate size", wrap(MaxPredicateSize))
}

func FacetJsonInputSupportsAnyOfTerms(t *testing.T, c *dgo.Dgraph) {
Expand Down Expand Up @@ -1678,3 +1679,40 @@ func HasReverseEdge(t *testing.T, c *dgo.Dgraph) {
require.Equal(t, len(revs), 1)
require.Equal(t, revs[0].Name, "carol")
}

func MaxPredicateSize(t *testing.T, c *dgo.Dgraph) {
// Create a string that has more than than 2^16 chars.
var b strings.Builder
for i := 0; i < 10000; i++ {
b.WriteString("abcdefg")
}
largePred := b.String()

// Verify that Alter requests with predicates that are too large are rejected.
ctx := context.Background()
err := c.Alter(ctx, &api.Operation{
Schema: fmt.Sprintf(`%s: uid @reverse .`, largePred),
})
require.Error(t, err)
require.Contains(t, err.Error(), "Predicate name length cannot be bigger than 2^16")

// Verify that Mutate requests with predicates that are too large are rejected.
txn := c.NewTxn()
_, err = txn.Mutate(ctx, &api.Mutation{
CommitNow: true,
SetNquads: []byte(fmt.Sprintf(`_:test <%s> "value" .`, largePred)),
})
require.Error(t, err)
require.Contains(t, err.Error(), "Predicate name length cannot be bigger than 2^16")
_ = txn.Discard(ctx)

// Do the same thing as above but for the predicates in DelNquads.
txn = c.NewTxn()
defer txn.Discard(ctx)
_, err = txn.Mutate(ctx, &api.Mutation{
CommitNow: true,
DelNquads: []byte(fmt.Sprintf(`_:test <%s> "value" .`, largePred)),
})
require.Error(t, err)
require.Contains(t, err.Error(), "Predicate name length cannot be bigger than 2^16")
}