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

Conversation

martinmr
Copy link
Contributor

@martinmr martinmr commented Feb 21, 2019

This change introduces checks in server.go to verify that Alter,
Mutation, and Query requests that includes predicates larger than 2^16
characters are rejected before being processed.

Fixes #3049


This change is Reviewable

This change introduces checks in server.go to verify that Alter,
Mutation, and Query requests that includes predicates larger than 2^16
characters are rejected before being processed.
@martinmr martinmr requested a review from a team February 21, 2019 01:01
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: One comment to consolidate the length checking of the predicate into a single function returning an error, which gets called from all the other places.

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @martinmr)


edgraph/server.go, line 360 at r1 (raw file):

		if len(update.Predicate) > math.MaxUint16 {
			return nil, fmt.Errorf("Predicate size cannot be bigger than 2^16")

Predicate name length cannot be


edgraph/server.go, line 782 at r1 (raw file):

	for _, nq := range set {
		if len(nq.Predicate) > math.MaxUint16 {
			return x.Errorf("Predicate size cannot be bigger than 2^16")

name length. Also, mention the predicate which has that length, maybe the first 80 bytes or something.

x.Errorf("Predicate name length cannot be bigger than 2^16. Predicate: %s...", nq.Predicate[:80])


edgraph/server.go, line 797 at r1 (raw file):

	for _, nq := range del {
		if len(nq.Predicate) > math.MaxUint16 {
			return x.Errorf("Predicate size cannot be bigger than 2^16")

name length.


edgraph/server.go, line 845 at r1 (raw file):

	for _, q := range queries {
		if len(q.Attr) > math.MaxUint16 {
			return fmt.Errorf("Predicate size cannot be bigger than 2^16")

Mention the predicate.

I see this error in multiple places, can you consolidate it all by creating a function to 'validatePred(...) error', so if it returns a non-nil, you can just return it directly.

edgraph/server.go Outdated Show resolved Hide resolved
@martinmr martinmr merged commit d5cb05d into master Feb 23, 2019
@martinmr martinmr deleted the martinmr/max-pred-size branch February 23, 2019 01:20
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
…raph-io#3052)

* Reject requests with predicates larger than the max size allowed.

This change introduces checks in server.go to verify that Alter,
Mutation, and Query requests that includes predicates larger than 2^16
characters are rejected before being processed.

* Address review comments.

* run go fmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants