Skip to content
This repository has been archived by the owner on Dec 18, 2019. It is now read-only.

added custom scalar warning #402

Merged
merged 2 commits into from
Aug 30, 2018
Merged

added custom scalar warning #402

merged 2 commits into from
Aug 30, 2018

Conversation

StephenCoady
Copy link
Member

@@ -34,6 +34,8 @@ type Note {
Save the schema py pressing the `Save Schema` button. Upon saving the {sync-service} will check your schema for errors and compile it to a JSON representation.
You can download this JSON representation by clicking on `Download Compiled Schema`. This is needed in the client SDKs.

NOTE: When defining custom scalars on your schema it is important to assert that the data types are what you are actually expecting. For example if passing a JSON object consisting of String fields, it is important to validate they are actually Strings.
Copy link
Contributor

@wtrocki wtrocki Aug 30, 2018

Choose a reason for hiding this comment

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

scalars could have link to graphql docs.
data types are what you are actually expecting. => matching expected type

For example if passing a JSON object consisting of String fields, it is important to validate they are actually Strings. Example gives little context.
This is actually resolver specific (and client). I will skip that part completely and say that Scalar definitions could potentially lead to SQLInjection etc. IMHO first sentence will be enough if we mention why this is needed - SQL injection/GraphQL injection

@StephenCoady StephenCoady merged commit 0c87b21 into sync Aug 30, 2018
Copy link
Contributor

@wtrocki wtrocki left a comment

Choose a reason for hiding this comment

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

Just seen this. Really good stuff!

@wtrocki wtrocki deleted the AEROGEAR-7717 branch August 30, 2018 15:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants