Copilot commented on code in PR #13372: URL: https://github.com/apache/apisix/pull/13372#discussion_r3239399776
########## apisix/plugins/graphql-limit-count.lua: ########## @@ -0,0 +1,147 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local limit_count = require("apisix.plugins.limit-count.init") +local core = require("apisix.core") +local gq_parse = require("graphql").parse +local limit_count_ver = require("resty.limit.count")._VERSION + +local type = type +local pairs = pairs +local pcall = pcall + +local plugin_name = "graphql-limit-count" +local _M = { + version = 0.1, + priority = 1004, + name = plugin_name, + schema = limit_count.schema, +} + + +function _M.check_schema(conf) + return limit_count.check_schema(conf) +end + + +local GRAPHQL_REQ_QUERY = "query" +local GRAPHQL_REQ_MIME_JSON = "application/json" +local GRAPHQL_REQ_MIME_GQL = "application/graphql" + + +local fetch_graphql_body = { + ["POST"] = function(ctx, max_size) + local body, err = core.request.get_body(max_size, ctx) + if not body then + return nil, "failed to read graphql data, " .. (err or "request body has zero size") + end + + return body + end +} + + +local check_graphql_request = { + ["POST"] = function(ctx, body) + local content_type = core.request.header(ctx, "Content-Type") + if content_type == GRAPHQL_REQ_MIME_JSON then + local res, err = core.json.decode(body) + if not res then + return false, "invalid graphql request, " .. err + end + + if not res[GRAPHQL_REQ_QUERY] then + return false, "invalid graphql request, json body[" .. + GRAPHQL_REQ_QUERY .. "] is nil" + end + + return true, res[GRAPHQL_REQ_QUERY] + end + + if content_type == GRAPHQL_REQ_MIME_GQL then + if not core.string.find(body, GRAPHQL_REQ_QUERY) then + return false, "invalid graphql request, can't find '" .. + GRAPHQL_REQ_QUERY .. "' in request body" + end + return true, body + end + + return false, "invalid graphql request, error content-type: " .. (content_type or "") + end +} + + +-- Finds the depth of the graphql query from the given AST table. +local function node_depth(t) + local depth = 0 + if type(t) ~= "table" then + return depth + end + + for k, v in pairs(t) do + if k == "selections" then + depth = depth + 1 + end + depth = depth + node_depth(v) + end + + return depth +end + + +function _M.access(conf, ctx) + if limit_count_ver < '1.0.0' then + core.log.error("need to build APISIX-Base to support GraphQL limit count") + return 501 + end + + local method = core.request.get_method() + if method ~= "POST" then + return 405 + end + + local body, err = fetch_graphql_body[method](ctx) Review Comment: The request body is read without a maximum size. APISIX already has a `graphql.max_size` default used by `apisix/core/ctx.lua`, but this path passes no limit, so a very large GraphQL body can be read into memory/from a temp file before being rejected. ########## apisix/plugins/graphql-limit-count.lua: ########## @@ -0,0 +1,147 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local limit_count = require("apisix.plugins.limit-count.init") +local core = require("apisix.core") +local gq_parse = require("graphql").parse +local limit_count_ver = require("resty.limit.count")._VERSION + +local type = type +local pairs = pairs +local pcall = pcall + +local plugin_name = "graphql-limit-count" +local _M = { + version = 0.1, + priority = 1004, + name = plugin_name, + schema = limit_count.schema, +} + + +function _M.check_schema(conf) + return limit_count.check_schema(conf) +end + + +local GRAPHQL_REQ_QUERY = "query" +local GRAPHQL_REQ_MIME_JSON = "application/json" +local GRAPHQL_REQ_MIME_GQL = "application/graphql" + + +local fetch_graphql_body = { + ["POST"] = function(ctx, max_size) + local body, err = core.request.get_body(max_size, ctx) + if not body then + return nil, "failed to read graphql data, " .. (err or "request body has zero size") + end + + return body + end +} + + +local check_graphql_request = { + ["POST"] = function(ctx, body) + local content_type = core.request.header(ctx, "Content-Type") + if content_type == GRAPHQL_REQ_MIME_JSON then Review Comment: This exact Content-Type comparison rejects valid JSON media types with parameters or different casing, such as `application/json; charset=utf-8`, which many clients send by default. Other APISIX plugins handle this with prefix/substring matching for `application/json`. ########## apisix/plugins/graphql-limit-count.lua: ########## @@ -0,0 +1,147 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local limit_count = require("apisix.plugins.limit-count.init") +local core = require("apisix.core") +local gq_parse = require("graphql").parse +local limit_count_ver = require("resty.limit.count")._VERSION + +local type = type +local pairs = pairs +local pcall = pcall + +local plugin_name = "graphql-limit-count" +local _M = { + version = 0.1, + priority = 1004, + name = plugin_name, + schema = limit_count.schema, +} + + +function _M.check_schema(conf) + return limit_count.check_schema(conf) +end + + +local GRAPHQL_REQ_QUERY = "query" +local GRAPHQL_REQ_MIME_JSON = "application/json" +local GRAPHQL_REQ_MIME_GQL = "application/graphql" + + +local fetch_graphql_body = { + ["POST"] = function(ctx, max_size) + local body, err = core.request.get_body(max_size, ctx) + if not body then + return nil, "failed to read graphql data, " .. (err or "request body has zero size") + end + + return body + end +} + + +local check_graphql_request = { + ["POST"] = function(ctx, body) + local content_type = core.request.header(ctx, "Content-Type") + if content_type == GRAPHQL_REQ_MIME_JSON then + local res, err = core.json.decode(body) + if not res then + return false, "invalid graphql request, " .. err + end + + if not res[GRAPHQL_REQ_QUERY] then + return false, "invalid graphql request, json body[" .. + GRAPHQL_REQ_QUERY .. "] is nil" + end + + return true, res[GRAPHQL_REQ_QUERY] + end + + if content_type == GRAPHQL_REQ_MIME_GQL then + if not core.string.find(body, GRAPHQL_REQ_QUERY) then + return false, "invalid graphql request, can't find '" .. + GRAPHQL_REQ_QUERY .. "' in request body" + end + return true, body + end + + return false, "invalid graphql request, error content-type: " .. (content_type or "") + end +} + + +-- Finds the depth of the graphql query from the given AST table. +local function node_depth(t) + local depth = 0 + if type(t) ~= "table" then + return depth + end + + for k, v in pairs(t) do + if k == "selections" then + depth = depth + 1 + end + depth = depth + node_depth(v) + end + + return depth +end + + +function _M.access(conf, ctx) + if limit_count_ver < '1.0.0' then + core.log.error("need to build APISIX-Base to support GraphQL limit count") + return 501 + end + + local method = core.request.get_method() + if method ~= "POST" then + return 405 + end + + local body, err = fetch_graphql_body[method](ctx) + if not body then + core.log.error(err) + return 400, {message = "Invalid graphql request: cant't get graphql request body"} Review Comment: Typo in the response message: `cant't` should be `can't`. ########## t/plugin/graphql-limit-count.t: ########## @@ -0,0 +1,341 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); Review Comment: This test file is stateful: route setup tests must run before the subsequent hit tests, but it does not call `no_shuffle()`. If the test suite is run with shuffling enabled, requests can execute before their setup route and fail nondeterministically. ########## apisix/plugins/graphql-limit-count.lua: ########## @@ -0,0 +1,147 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local limit_count = require("apisix.plugins.limit-count.init") +local core = require("apisix.core") +local gq_parse = require("graphql").parse +local limit_count_ver = require("resty.limit.count")._VERSION + +local type = type +local pairs = pairs +local pcall = pcall + +local plugin_name = "graphql-limit-count" +local _M = { + version = 0.1, + priority = 1004, + name = plugin_name, + schema = limit_count.schema, +} + + +function _M.check_schema(conf) + return limit_count.check_schema(conf) +end + + +local GRAPHQL_REQ_QUERY = "query" +local GRAPHQL_REQ_MIME_JSON = "application/json" +local GRAPHQL_REQ_MIME_GQL = "application/graphql" + + +local fetch_graphql_body = { + ["POST"] = function(ctx, max_size) + local body, err = core.request.get_body(max_size, ctx) + if not body then + return nil, "failed to read graphql data, " .. (err or "request body has zero size") + end + + return body + end +} + + +local check_graphql_request = { + ["POST"] = function(ctx, body) + local content_type = core.request.header(ctx, "Content-Type") + if content_type == GRAPHQL_REQ_MIME_JSON then + local res, err = core.json.decode(body) + if not res then + return false, "invalid graphql request, " .. err + end + + if not res[GRAPHQL_REQ_QUERY] then + return false, "invalid graphql request, json body[" .. + GRAPHQL_REQ_QUERY .. "] is nil" + end + + return true, res[GRAPHQL_REQ_QUERY] + end + + if content_type == GRAPHQL_REQ_MIME_GQL then + if not core.string.find(body, GRAPHQL_REQ_QUERY) then + return false, "invalid graphql request, can't find '" .. + GRAPHQL_REQ_QUERY .. "' in request body" + end + return true, body + end + + return false, "invalid graphql request, error content-type: " .. (content_type or "") + end +} + + +-- Finds the depth of the graphql query from the given AST table. +local function node_depth(t) + local depth = 0 + if type(t) ~= "table" then + return depth + end + + for k, v in pairs(t) do + if k == "selections" then + depth = depth + 1 + end + depth = depth + node_depth(v) + end + + return depth Review Comment: This counts every `selections` table in the AST instead of the maximum nesting depth. Wide shallow queries (or documents with fragments/multiple operations) will consume more quota than their actual depth, so the plugin does not enforce the depth-based budget described by the PR/docs. ########## docs/zh/latest/plugins/graphql-limit-count.md: ########## @@ -0,0 +1,274 @@ +--- +title: graphql-limit-count +keywords: + - Apache APISIX + - API Gateway + - Plugin + - graphql-limit-count + - 限流 + - GraphQL +description: graphql-limit-count 插件使用固定窗口算法，基于 GraphQL 查询 AST 深度对请求速率进行限制，采用与 limit-count 插件相同的计数机制。 +--- + +<!-- +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +--> + Review Comment: New plugin docs should include the canonical `<head>` block before imports, consistent with existing Chinese plugin docs such as `docs/zh/latest/plugins/limit-count.md:30-35` and `docs/zh/latest/plugins/degraphql.md:30-32`. ########## t/plugin/graphql-limit-count.t: ########## @@ -0,0 +1,341 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); + +add_block_preprocessor(sub { + my ($block) = @_; + + my $http_config = $block->http_config // <<_EOC_; + lua_shared_dict plugin-graphql-limit-count 10m; + lua_shared_dict plugin-graphql-limit-count-reset-header 10m; +_EOC_ + + $block->set_value("http_config", $http_config); + + my $extra_yaml_config = $block->extra_yaml_config // <<_EOC_; +plugins: + - graphql-limit-count +_EOC_ + + $block->set_value("extra_yaml_config", $extra_yaml_config); + Review Comment: The Redis and Redis-cluster cases later in this file rely on exact remaining quota values, but the preprocessor never flushes Redis. Existing limit-count Redis tests clear Redis in `extra_init_worker_by_lua`; without that, counters left by a previous run can make these tests flaky. ########## apisix/plugins/graphql-limit-count.lua: ########## @@ -0,0 +1,147 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local limit_count = require("apisix.plugins.limit-count.init") +local core = require("apisix.core") +local gq_parse = require("graphql").parse +local limit_count_ver = require("resty.limit.count")._VERSION + +local type = type +local pairs = pairs +local pcall = pcall + +local plugin_name = "graphql-limit-count" +local _M = { + version = 0.1, + priority = 1004, + name = plugin_name, + schema = limit_count.schema, +} + + +function _M.check_schema(conf) + return limit_count.check_schema(conf) +end + + +local GRAPHQL_REQ_QUERY = "query" +local GRAPHQL_REQ_MIME_JSON = "application/json" +local GRAPHQL_REQ_MIME_GQL = "application/graphql" + + +local fetch_graphql_body = { + ["POST"] = function(ctx, max_size) + local body, err = core.request.get_body(max_size, ctx) + if not body then + return nil, "failed to read graphql data, " .. (err or "request body has zero size") + end + + return body + end +} + + +local check_graphql_request = { + ["POST"] = function(ctx, body) + local content_type = core.request.header(ctx, "Content-Type") + if content_type == GRAPHQL_REQ_MIME_JSON then + local res, err = core.json.decode(body) + if not res then + return false, "invalid graphql request, " .. err + end + + if not res[GRAPHQL_REQ_QUERY] then + return false, "invalid graphql request, json body[" .. + GRAPHQL_REQ_QUERY .. "] is nil" + end + + return true, res[GRAPHQL_REQ_QUERY] + end + + if content_type == GRAPHQL_REQ_MIME_GQL then Review Comment: This exact Content-Type comparison also rejects valid `application/graphql` requests with parameters such as `application/graphql; charset=utf-8`. Media type parameters should be tolerated before deciding the request is not GraphQL. ########## docs/en/latest/plugins/graphql-limit-count.md: ########## @@ -0,0 +1,274 @@ +--- +title: graphql-limit-count +keywords: + - Apache APISIX + - API Gateway + - Plugin + - graphql-limit-count + - Rate Limiting + - GraphQL +description: The graphql-limit-count Plugin limits the rate of GraphQL requests based on the query AST depth within a given time window, using the same counting mechanism as the limit-count Plugin. +--- + +<!-- +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +--> + Review Comment: New plugin docs should include the canonical `<head>` block before imports, consistent with existing plugin docs such as `docs/en/latest/plugins/limit-count.md:29-34` and `docs/en/latest/plugins/degraphql.md:30-32`. ########## apisix/plugins/graphql-limit-count.lua: ########## @@ -0,0 +1,147 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local limit_count = require("apisix.plugins.limit-count.init") +local core = require("apisix.core") +local gq_parse = require("graphql").parse +local limit_count_ver = require("resty.limit.count")._VERSION + +local type = type +local pairs = pairs +local pcall = pcall + +local plugin_name = "graphql-limit-count" +local _M = { + version = 0.1, + priority = 1004, + name = plugin_name, + schema = limit_count.schema, +} + + +function _M.check_schema(conf) + return limit_count.check_schema(conf) +end + + +local GRAPHQL_REQ_QUERY = "query" +local GRAPHQL_REQ_MIME_JSON = "application/json" +local GRAPHQL_REQ_MIME_GQL = "application/graphql" + + +local fetch_graphql_body = { + ["POST"] = function(ctx, max_size) + local body, err = core.request.get_body(max_size, ctx) + if not body then + return nil, "failed to read graphql data, " .. (err or "request body has zero size") + end + + return body + end +} + + +local check_graphql_request = { + ["POST"] = function(ctx, body) + local content_type = core.request.header(ctx, "Content-Type") + if content_type == GRAPHQL_REQ_MIME_JSON then + local res, err = core.json.decode(body) + if not res then + return false, "invalid graphql request, " .. err + end + + if not res[GRAPHQL_REQ_QUERY] then + return false, "invalid graphql request, json body[" .. + GRAPHQL_REQ_QUERY .. "] is nil" + end + + return true, res[GRAPHQL_REQ_QUERY] + end + + if content_type == GRAPHQL_REQ_MIME_GQL then + if not core.string.find(body, GRAPHQL_REQ_QUERY) then + return false, "invalid graphql request, can't find '" .. + GRAPHQL_REQ_QUERY .. "' in request body" + end + return true, body + end + + return false, "invalid graphql request, error content-type: " .. (content_type or "") + end +} + + +-- Finds the depth of the graphql query from the given AST table. +local function node_depth(t) + local depth = 0 + if type(t) ~= "table" then + return depth + end + + for k, v in pairs(t) do + if k == "selections" then + depth = depth + 1 + end + depth = depth + node_depth(v) + end + + return depth +end + + +function _M.access(conf, ctx) + if limit_count_ver < '1.0.0' then + core.log.error("need to build APISIX-Base to support GraphQL limit count") + return 501 + end + + local method = core.request.get_method() + if method ~= "POST" then + return 405 + end + + local body, err = fetch_graphql_body[method](ctx) + if not body then + core.log.error(err) + return 400, {message = "Invalid graphql request: cant't get graphql request body"} + end + + local is_graphql_req, query_or_err = check_graphql_request[method](ctx, body) + if not is_graphql_req then + core.log.error(query_or_err) + return 400, {message = "Invalid graphql request: no query"} Review Comment: All request validation failures are returned to clients as `Invalid graphql request: no query`, including invalid JSON and unsupported Content-Type. This makes client-side debugging difficult; return a message that reflects the actual validation failure. ########## apisix/plugins/graphql-limit-count.lua: ########## @@ -0,0 +1,147 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local limit_count = require("apisix.plugins.limit-count.init") +local core = require("apisix.core") +local gq_parse = require("graphql").parse +local limit_count_ver = require("resty.limit.count")._VERSION + +local type = type +local pairs = pairs +local pcall = pcall + +local plugin_name = "graphql-limit-count" +local _M = { + version = 0.1, + priority = 1004, + name = plugin_name, + schema = limit_count.schema, +} + + +function _M.check_schema(conf) + return limit_count.check_schema(conf) +end + + +local GRAPHQL_REQ_QUERY = "query" +local GRAPHQL_REQ_MIME_JSON = "application/json" +local GRAPHQL_REQ_MIME_GQL = "application/graphql" + + +local fetch_graphql_body = { + ["POST"] = function(ctx, max_size) + local body, err = core.request.get_body(max_size, ctx) + if not body then + return nil, "failed to read graphql data, " .. (err or "request body has zero size") + end + + return body + end +} + + +local check_graphql_request = { + ["POST"] = function(ctx, body) + local content_type = core.request.header(ctx, "Content-Type") + if content_type == GRAPHQL_REQ_MIME_JSON then + local res, err = core.json.decode(body) + if not res then + return false, "invalid graphql request, " .. err + end + + if not res[GRAPHQL_REQ_QUERY] then + return false, "invalid graphql request, json body[" .. + GRAPHQL_REQ_QUERY .. "] is nil" + end + + return true, res[GRAPHQL_REQ_QUERY] + end + + if content_type == GRAPHQL_REQ_MIME_GQL then + if not core.string.find(body, GRAPHQL_REQ_QUERY) then + return false, "invalid graphql request, can't find '" .. + GRAPHQL_REQ_QUERY .. "' in request body" + end + return true, body Review Comment: The `application/graphql` success branch is not covered by the new tests; the only raw GraphQL case is an invalid body. Add a successful raw GraphQL request test so regressions in this content-type path are caught. ########## apisix/cli/ngx_tpl.lua: ########## @@ -327,6 +327,11 @@ http { lua_shared_dict plugin-limit-count-reset-header {* http.lua_shared_dict["plugin-limit-count"] *}; {% end %} + {% if enabled_plugins["graphql-limit-count"] then %} + lua_shared_dict plugin-graphql-limit-count {* http.lua_shared_dict["plugin-graphql-limit-count"] *}; + lua_shared_dict plugin-graphql-limit-count-reset-header {* http.lua_shared_dict["plugin-graphql-limit-count-reset-header"] *}; + {% end %} Review Comment: When only `graphql-limit-count` is enabled, the Redis-cluster policy can fail because `limit-count-redis-cluster.lua` hard-codes the shared dict `plugin-limit-count-redis-cluster-slot-lock`, but that dict is declared only under the `limit-count` plugin block. Ensure the slot-lock dict is declared when either plugin is enabled, without declaring it twice when both are enabled. ########## apisix/plugins/graphql-limit-count.lua: ########## @@ -0,0 +1,147 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local limit_count = require("apisix.plugins.limit-count.init") +local core = require("apisix.core") +local gq_parse = require("graphql").parse +local limit_count_ver = require("resty.limit.count")._VERSION + +local type = type +local pairs = pairs +local pcall = pcall + +local plugin_name = "graphql-limit-count" +local _M = { + version = 0.1, + priority = 1004, + name = plugin_name, + schema = limit_count.schema, +} + + +function _M.check_schema(conf) + return limit_count.check_schema(conf) +end + + +local GRAPHQL_REQ_QUERY = "query" +local GRAPHQL_REQ_MIME_JSON = "application/json" +local GRAPHQL_REQ_MIME_GQL = "application/graphql" + + +local fetch_graphql_body = { + ["POST"] = function(ctx, max_size) + local body, err = core.request.get_body(max_size, ctx) + if not body then + return nil, "failed to read graphql data, " .. (err or "request body has zero size") + end + + return body + end +} + + +local check_graphql_request = { + ["POST"] = function(ctx, body) + local content_type = core.request.header(ctx, "Content-Type") + if content_type == GRAPHQL_REQ_MIME_JSON then + local res, err = core.json.decode(body) + if not res then + return false, "invalid graphql request, " .. err + end + + if not res[GRAPHQL_REQ_QUERY] then + return false, "invalid graphql request, json body[" .. + GRAPHQL_REQ_QUERY .. "] is nil" + end + + return true, res[GRAPHQL_REQ_QUERY] + end + + if content_type == GRAPHQL_REQ_MIME_GQL then + if not core.string.find(body, GRAPHQL_REQ_QUERY) then + return false, "invalid graphql request, can't find '" .. + GRAPHQL_REQ_QUERY .. "' in request body" + end + return true, body + end + + return false, "invalid graphql request, error content-type: " .. (content_type or "") + end +} + + +-- Finds the depth of the graphql query from the given AST table. +local function node_depth(t) + local depth = 0 + if type(t) ~= "table" then + return depth + end + + for k, v in pairs(t) do + if k == "selections" then + depth = depth + 1 + end + depth = depth + node_depth(v) + end + + return depth +end + + +function _M.access(conf, ctx) + if limit_count_ver < '1.0.0' then + core.log.error("need to build APISIX-Base to support GraphQL limit count") + return 501 + end + + local method = core.request.get_method() + if method ~= "POST" then + return 405 + end + + local body, err = fetch_graphql_body[method](ctx) + if not body then + core.log.error(err) + return 400, {message = "Invalid graphql request: cant't get graphql request body"} + end + + local is_graphql_req, query_or_err = check_graphql_request[method](ctx, body) + if not is_graphql_req then + core.log.error(query_or_err) + return 400, {message = "Invalid graphql request: no query"} + end + + local ok, res = pcall(gq_parse, query_or_err) + if not ok then + core.log.error("failed to parse graphql: ", res, ", body: ", body) + return 400, {message = "Invalid graphql request: failed to parse graphql query"} + end + + local n = #res.definitions + if n == 0 then + core.log.error("failed to parse graphql: empty query, body: ", body) + return 400, {message = "Invalid graphql request: empty graphql query"} + end + + local depth = node_depth(res) + core.log.info("graphql node depth: ", depth) + + return limit_count.rate_limit(conf, ctx, plugin_name, depth) Review Comment: The new tests assert remaining quota after a single request, but they never exercise the rejection path where accumulated depth exceeds `count`. Add a case that sends enough GraphQL requests to exceed the configured budget and verifies the configured `rejected_code`/headers. ########## apisix/plugins/graphql-limit-count.lua: ########## @@ -0,0 +1,147 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local limit_count = require("apisix.plugins.limit-count.init") +local core = require("apisix.core") +local gq_parse = require("graphql").parse +local limit_count_ver = require("resty.limit.count")._VERSION + +local type = type +local pairs = pairs +local pcall = pcall + +local plugin_name = "graphql-limit-count" +local _M = { + version = 0.1, + priority = 1004, + name = plugin_name, + schema = limit_count.schema, +} + + +function _M.check_schema(conf) + return limit_count.check_schema(conf) +end + + +local GRAPHQL_REQ_QUERY = "query" +local GRAPHQL_REQ_MIME_JSON = "application/json" +local GRAPHQL_REQ_MIME_GQL = "application/graphql" + + +local fetch_graphql_body = { + ["POST"] = function(ctx, max_size) + local body, err = core.request.get_body(max_size, ctx) + if not body then + return nil, "failed to read graphql data, " .. (err or "request body has zero size") + end + + return body + end +} + + +local check_graphql_request = { + ["POST"] = function(ctx, body) + local content_type = core.request.header(ctx, "Content-Type") + if content_type == GRAPHQL_REQ_MIME_JSON then + local res, err = core.json.decode(body) + if not res then + return false, "invalid graphql request, " .. err + end + + if not res[GRAPHQL_REQ_QUERY] then + return false, "invalid graphql request, json body[" .. + GRAPHQL_REQ_QUERY .. "] is nil" + end + + return true, res[GRAPHQL_REQ_QUERY] + end + + if content_type == GRAPHQL_REQ_MIME_GQL then + if not core.string.find(body, GRAPHQL_REQ_QUERY) then + return false, "invalid graphql request, can't find '" .. + GRAPHQL_REQ_QUERY .. "' in request body" + end Review Comment: Valid `application/graphql` bodies do not have to contain the literal word `query` (for example shorthand queries can start with `{`, and mutations/subscriptions start with their own operation keyword). Since the parser already validates GraphQL syntax, this pre-check incorrectly blocks valid GraphQL requests. ########## docs/zh/latest/plugins/graphql-limit-count.md: ########## @@ -0,0 +1,274 @@ +--- +title: graphql-limit-count +keywords: + - Apache APISIX + - API Gateway Review Comment: The Chinese plugin docs consistently localize this front-matter keyword as `API 网关`; this new page uses the English `API Gateway`, unlike examples such as `docs/zh/latest/plugins/degraphql.md:3-8` and `docs/zh/latest/plugins/limit-count.md:3-8`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
