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

Passing bogus JSON triggers a warning/exception #107

Closed
pascaldevink opened this issue Jan 25, 2017 · 4 comments
Closed

Passing bogus JSON triggers a warning/exception #107

pascaldevink opened this issue Jan 25, 2017 · 4 comments

Comments

@pascaldevink
Copy link
Contributor

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Version/Branch master#eed88d8e88f1fe7dea3835ab7371f146a6806543

When doing a valid JSON request, with a valid JSON body, a warning is thrown if the JSON body isn't valid GraphQL.

The message is Warning: mb_strlen() expects parameter 1 to be string, array given, in file webonyx/graphql-php/src/Language/Source.php line 24.
You can find the entire stack trace here: https://gist.github.com/pascaldevink/413fdd9d9436e138150ff14d6ae022ea

I guess there isn't enough error checking in Overblog\GraphQLBundle\Request\Parser::getParsedBody() when it come to JSON data. In fact, I'm not sure why it is supported, I can't find any documentation on how to send data this way.

Of course, it's a warning, which will be ignored in production, but I think it should be handled properly nonetheless.

@mcg-web
Copy link
Member

mcg-web commented Jan 25, 2017

Hi @pascaldevink can you give me an example of your request please?

@pascaldevink
Copy link
Contributor Author

Yes, of course, stupid of me to forget:

curl -X "POST" "<host>" \
     -H "Content-Type: application/json" \
     -d $'{
  "query": {
    "foo": "bar"
  }
}'

@mcg-web
Copy link
Member

mcg-web commented Jan 25, 2017

ok the right way to write is:

curl -X "POST" "<host>" \
     -H "Content-Type: application/json" \
     -d $'{
  "query": "{foo: bar}",
}'

query value must be a string.
variables a json object
I'll add a check to send a better error message and some documentation...

@mcg-web mcg-web added this to the v0.10 milestone Sep 29, 2017
@mcg-web mcg-web modified the milestones: v0.10, v0.11 Nov 22, 2017
@mcg-web
Copy link
Member

mcg-web commented Dec 23, 2017

closed in favor of #252

@mcg-web mcg-web closed this as completed Dec 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants