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

Sanity check for empty variables #3021

Merged
merged 6 commits into from
Feb 15, 2019

Conversation

srfrog
Copy link
Contributor

@srfrog srfrog commented Feb 15, 2019

Empty variables have nil value and default type, this change
adds sanity checks before the value is type asserted and cause panic.


This change is Reviewable

Empty variables have nil value and default type, this change
adds sanity checks before the value is type asserted and cause panic.
@srfrog srfrog requested a review from a team February 15, 2019 01:16
Copy link
Contributor

@codexnull codexnull left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

srfrog added 4 commits February 14, 2019 18:57
Val types use a field Value which is an interface. Type assertions
of Value will panic when it is nil. The Safe() function will
ensure a non-nil Value is returned. It won't change it, only make
type assertions safer. Note that invalid type assertions are still
possible.
A Val of type DefaultID could be a default value, play it safe.
Copy link
Contributor

@martinmr martinmr left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@srfrog srfrog merged commit e65f54b into master Feb 15, 2019
@srfrog srfrog deleted the srfrog/panic_on_comparison_with_default branch February 15, 2019 21:27
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
* types/sort.go: sanity check for empty variables

Empty variables have nil value and default type, this change
adds sanity checks before the value is type asserted and cause panic.

* query/query.go: enhanced default value vars comment

* types/scalar_types.go: added func Safe()

Val types use a field Value which is an interface. Type assertions
of Value will panic when it is nil. The Safe() function will
ensure a non-nil Value is returned. It won't change it, only make
type assertions safer. Note that invalid type assertions are still
possible.

* types/conversion.go: use Safe() in MarshalJSON() for DefaultID

A Val of type DefaultID could be a default value, play it safe.

* types/sort.go: use Safe() and narrow the field to DefaultID

* types/sort.go: revert unneeded change
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