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

bug: ctx.lua#59 parse_graphql(ctx) #6266

Closed
pcyan opened this issue Feb 8, 2022 · 15 comments · Fixed by #6343
Closed

bug: ctx.lua#59 parse_graphql(ctx) #6266

pcyan opened this issue Feb 8, 2022 · 15 comments · Fixed by #6343

Comments

@pcyan
Copy link

pcyan commented Feb 8, 2022

Issue description

use whole request body to parse graphql will get parse error.
graphql request body is json , example :{"query":"query{getUser{name age}}","variables":null}, not query{getUser{name age}}

Environment

  • apisix version (cmd: apisix version): apache/apisix:2.12.0-alpine
  • OS (cmd: uname -a): docker
  • OpenResty / Nginx version (cmd: nginx -V or openresty -V): null
  • etcd version, if have (cmd: run curl http://127.0.0.1:9090/v1/server_info to get the info from server-info API): bitnami/etcd:3.4.15
  • apisix-dashboard version, if have: apache/apisix-dashboard:2.10.1-alpine
  • the plugin runner version, if the issue is about a plugin runner (cmd: depended on the kind of runner):
  • luarocks version, if the issue is about installation (cmd: luarocks --version):

Steps to reproduce

  1. define graphql
query {
    getUser:User
}

type User{
    name:String
    age:String
}
  1. add route
curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -i -d '
{
    "methods": ["POST"],
    "uri": "/graphql",
    "vars": [
        ["graphql_operation", "==", "query"],
        ["graphql_name", "==", "getUser"]
    ],
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "127.0.0.1:1980": 1
        }
    }
}'
  1. perform graphql request by curl
curl 'http://127.0.0.1:9080/graphql' \
  -H 'Content-Type: text/plain;charset=UTF-8' \
  -H 'Accept: */*' \
  --data-raw '{"query":"query getUser{getUser{name age}}","variables":null}' \
  --compressed

Actual result

HTTP/1.1 404 Not Found
Date: Tue, 08 Feb 2022 07:39:16 GMT
Content-Type: text/plain; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive

{
"error_msg": "404 Route Not Found"
}

Error log

2022/02/08 07:39:16 [error] 45#45: *1085159 [lua] ctx.lua:80: get_parsed_graphql(): failed to parse graphql: Syntax error near line 1 body:

Expected result

success

@tzssangglass
Copy link
Member

maybe bad graphql, see:

apisix/t/router/graphql.t

Lines 165 to 171 in ec0fc2c

=== TEST 7: bad graphql
--- request
POST /hello
AA
--- error_code: 404
--- error_log
failed to parse graphql: Syntax error near line 1 body: AA

@pcyan
Copy link
Author

pcyan commented Feb 8, 2022

maybe bad graphql, see:

apisix/t/router/graphql.t

Lines 165 to 171 in ec0fc2c

=== TEST 7: bad graphql
--- request
POST /hello
AA
--- error_code: 404
--- error_log
failed to parse graphql: Syntax error near line 1 body: AA

@tzssangglass it's not a bad graphql

curl 'http://127.0.0.1:9080/graphql' \
  -H 'Content-Type: text/plain;charset=UTF-8' \
  -H 'Accept: */*' \
  --data-raw 'query getUser{getUser{name age}}' \
  --compressed

this request will route to backend, but backend will parse error
see https://graphql.org/learn/serving-over-http/#post-request

@tzssangglass
Copy link
Member

tzssangglass commented Feb 8, 2022

try

--data-raw '{"query":"query getUser {\n    getUser {\n        name age\n    }\n}","variables":null}'

?

@pcyan
Copy link
Author

pcyan commented Feb 9, 2022

@tzssangglass

  1. 一个正常的graphql请求
curl 'https://api.mocki.io/v2/c4d7a195/graphql' \
  -H 'authority: api.mocki.io' \
  -H 'accept: */*' \
  -H 'content-type: application/json' \
  -H 'origin: https://api.mocki.io' \
  --data-raw '{"operationName":"getUser","variables":{},"query":"query getUser {\n  user(id: \"4dc70521-22bb-4396-b37a-4a927c66d43b\") {\n    id\n    email\n    name\n  }\n}\n"}' \
  --compressed

会返回

{
  "data": {
    "user": {
      "id": "Hello World",
      "email": "Hello World",
      "name": "Hello World"
    }
  }
}
  1. 参照apisix官方文档,通过apisix配置
### 添加测试的graphql请求
curl http://localhost:9080/apisix/admin/routes/7 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -i -d '
{
    "methods": ["POST"],
    "name":"mockGetUser",
    "uri": "/v2/c4d7a195/graphql",
    "vars": [
        ["graphql_operation", "==", "query"],
        ["graphql_name", "==", "getUser"]
    ],
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "api.mocki.io": 1
        }
    }
}'

### apisix graphql
curl 'https://localhost:9080/v2/c4d7a195/graphql' \
  -H 'authority: api.mocki.io' \
  -H 'accept: */*' \
  -H 'content-type: application/json' \
  -H 'origin: https://api.mocki.io' \
  --data-raw '{"operationName":"getUser","variables":{},"query":"query getUser {\n  user(id: \"4dc70521-22bb-4396-b37a-4a927c66d43b\") {\n    id\n    email\n    name\n  }\n}\n"}' \
  --compressed

会返回

{
  "error_msg": "404 Route Not Found"
}

错误日志是 ctx.lua:80: get_parsed_graphql(): failed to parse graphql: Syntax error near line 1 body:
麻烦你也试一下,或者告诉我是哪里配置错了

@leslie-tsang
Copy link
Member

@pcyan The route method has been set to POST, you should add -X POST in your curl CMD.

@Chever-John
Copy link
Contributor

@pcyan The route method has been set to POST, you should add -X POST in your curl CMD.

As stated above, this should be a must

@Chever-John
Copy link
Contributor

In fact, I tested both commands locally and they worked

config

curl http://127.0.0.1:9080/apisix/admin/routes/7 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -i -d '
{
    "methods": ["POST"],
    "name":"mockGetUser",
    "uri": "/graphql",
    "vars": [
        ["graphql_operation", "==", "query"],
        ["graphql_name", "==", "getUser"]
    ],
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "127.0.0.1:1980": 1
        }
    }
}'

curl command

curl -X POST http://127.0.0.1:9080/graphql -d '
query getUser {
    owner {
        name
    }
    repo {
        created
    }
}'

It could be that you missed '-POST'?

@pcyan
Copy link
Author

pcyan commented Feb 9, 2022

In fact, I tested both commands locally and they worked

config

curl http://127.0.0.1:9080/apisix/admin/routes/7 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -i -d '
{
    "methods": ["POST"],
    "name":"mockGetUser",
    "uri": "/graphql",
    "vars": [
        ["graphql_operation", "==", "query"],
        ["graphql_name", "==", "getUser"]
    ],
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "127.0.0.1:1980": 1
        }
    }
}'

curl command

curl -X POST http://127.0.0.1:9080/graphql -d '
query getUser {
    owner {
        name
    }
    repo {
        created
    }
}'

It could be that you missed '-POST'?

A standard GraphQL POST request should use the application/json content type, and include a JSON-encoded body of the following form:

{
  "query": "...",
  "operationName": "...",
  "variables": { "myVariable": "someValue", ... }
}

see official graphql document, https://graphql.org/learn/serving-over-http/#post-request

and --data will perform request with POST method ,see curl document

@tzssangglass
Copy link
Member

and --data will perform request with POST method ,see curl document

Where is it? I'm wondering where the error is caused by missing -X POST.

@Chever-John
Copy link
Contributor

Chever-John commented Feb 9, 2022

I could not find the relevant description in the curl document. Could you please provide.

Suggest like this:

### apisix graphql
curl 'https://localhost:9080/v2/c4d7a195/graphql' \
 -H 'authority: api.mocki.io' \
 -H 'accept: */*' \
 -H 'content-type: application/json' \
 -H 'origin: https://api.mocki.io' \
 -X PUT
 --data-raw '{"operationName":"getUser","variables":{},"query":"query getUser {\n  user(id: \"4dc70521-22bb-4396-b37a-4a927c66d43b\") {\n    id\n    email\n    name\n  }\n}\n"}' \
 --compressed

@pcyan
Copy link
Author

pcyan commented Feb 9, 2022

I could not find the relevant description in the curl document. Could you please provide.

Suggest like this:

### apisix graphql
curl 'https://localhost:9080/v2/c4d7a195/graphql' \
 -H 'authority: api.mocki.io' \
 -H 'accept: */*' \
 -H 'content-type: application/json' \
 -H 'origin: https://api.mocki.io' \
 -X PUT
 --data-raw '{"operationName":"getUser","variables":{},"query":"query getUser {\n  user(id: \"4dc70521-22bb-4396-b37a-4a927c66d43b\") {\n    id\n    email\n    name\n  }\n}\n"}' \
 --compressed
$ curl --help
Usage: curl [options...] <url>
 -d, --data <data>   **HTTP POST data**
...

use -v to print verbose log

curl -v 'https://api.mocki.io/v2/c4d7a195/graphql' \
  -H 'authority: api.mocki.io' \
  -H 'accept: */*' \
  -H 'content-type: application/json' \
  -H 'origin: https://api.mocki.io' \
  --data-raw '{"operationName":"getUser","variables":{},"query":"query getUser {\n  user(id: \"4dc70521-22bb-4396-b37a-4a927c66d43b\") {\n    id\n    email\n    name\n  }\n}\n"}' \
  --compressed

it will print something like this > POST /v2/c4d7a195/graphql HTTP/2, thought i'm not use -X POST

sorry, i'm try to discuss about how APISIX deal with graphql request. it seems that the mock GraphQL data of APISIX is not a standard GraphQL request.

@leslie-tsang
Copy link
Member

sorry, i'm try to discuss about how APISIX deal with graphql request. it seems that the mock GraphQL data of APISIX is not a standard GraphQL request.

Oh, I see, the error caused by graphql/parse.lua, it seems the graphql can't process a standard GraphQL request.

@Chever-John
Copy link
Contributor

Chever-John commented Feb 10, 2022

After discussion and review, it was found that the cause of this issue was that we did not support the application/json content type. I will mainly solve this issue in the next 3 days. @pcyan

@Gary-Airwallex
Copy link
Contributor

@Chever-John May I also suggest that the parse_graphql function should handle the case of GET requests? https://graphql.org/learn/serving-over-http/#get-request
Currently an error message is returned regardless of HTTP method used.

local body, err = request.get_body(max_size, ctx)
if not body then
    return nil, "failed to read graphql body: " .. (err or "request body has zero size")
end

@Chever-John
Copy link
Contributor

Chever-John commented Feb 10, 2022

In fact, these are the two new features (support 'json' & 'GET') that we're going to release next.
As for the launch time, I personally expect it to be completed within a week.
@pcyan

@Chever-John May I also suggest that the parse_graphql function should handle the case of GET requests? https://graphql.org/learn/serving-over-http/#get-request Currently an error message is returned regardless of HTTP method used.

local body, err = request.get_body(max_size, ctx)
if not body then
    return nil, "failed to read graphql body: " .. (err or "request body has zero size")
end

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

Successfully merging a pull request may close this issue.

5 participants