You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "suryaparua-official (via GitHub)" <gi...@apache.org> on 2026/05/12 04:01:12 UTC

[I] fix(limit-count): incorrect error() call in gen_limit_key passes string as level argument [apisix]

suryaparua-official opened a new issue, #13357:
URL: https://github.com/apache/apisix/issues/13357

   ### Current Behavior
   
   In `apisix/plugins/limit-count/init.lua` at line 262, the `error()` 
   function is called with incorrect arguments:
   
   error("failed to generate key invalid parent: ", core.json.encode(parent))
   
   Lua's error(message, level) expects the second argument to be an integer 
   (stack level: 0, 1, or 2), not a string.
   
   Passing core.json.encode(parent) as the level causes LuaJIT to raise:
   bad argument #2 to 'error' (number expected, got string)
   
   This crashes the request with an unhelpful LuaJIT internal error instead 
   of the intended error message about the invalid parent configuration.
   
   ### Expected Behavior
   
   When conf._meta.parent is nil or missing resource_key, APISIX should log 
   a clear error message containing the JSON of the invalid parent object, 
   helping developers debug the issue quickly.
   
   ### Error Logs
   
   bad argument #2 to 'error' (number expected, got string)
   
   ### Steps to Reproduce
   
   1. Open apisix/plugins/limit-count/init.lua
   2. Go to line 262
   3. Observe the error() call:
      error("failed to generate key invalid parent: ", core.json.encode(parent))
   4. The second argument core.json.encode(parent) is a string, but Lua's 
      error() expects an integer as the second argument (stack level).
   5. This path is triggered when conf._meta.parent is nil or 
      parent.resource_key is missing during limit-count plugin execution.
   
   ### Environment
   
   APISIX version: master (3.16.0)
   OS: Linux
   OpenResty version: 1.29.2.3
   File: apisix/plugins/limit-count/init.lua
   Line: 262


-- 
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: notifications-unsubscribe@apisix.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] fix(limit-count): incorrect error() call in gen_limit_key passes string as level argument [apisix]

Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on issue #13357:
URL: https://github.com/apache/apisix/issues/13357#issuecomment-4549893373

   The second argument of Lua error() should be a numeric stack level, but the current code can pass an encoded string there, which causes a secondary bad argument #2 error and hides the original problem.
   
   The fix should format the full error message as a single string instead of passing the JSON string as the second argument. It would be good to add a regression test for the missing parent/resource-key path and quickly scan for similar error(message, string) patterns.


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] fix(limit-count): incorrect error() call in gen_limit_key passes string as level argument [apisix]

Posted by "hanzhenfang (via GitHub)" <gi...@apache.org>.
hanzhenfang commented on issue #13357:
URL: https://github.com/apache/apisix/issues/13357#issuecomment-4645747083

   Hi @Baoyuantop , I was able to reproduce this locally as well. Sharing the minimal setup and a bit of context in case it helps with reviewing or validating the fix.
   
   The repro intentionally exercises the defensive branch in `limit-count/init.lua` by calling `limit_count.rate_limit()` with a manually constructed config whose `_meta.parent` is missing. This is not meant to show that a normal Admin API / route configuration naturally loses `_meta.parent`; it is just a minimal way to trigger the exact `error()` call reported in this
   issue.
   
   Minimal standalone repro:
   
   `docker-compose.yaml`
   
   ```yaml
   services:
     apisix:
       image: apache/apisix:3.16.0-debian
       container_name: apisix-issue-13357
       depends_on:
         - upstream
       volumes:
         - ./config.yaml:/usr/local/apisix/conf/config.yaml:ro
         - ./apisix.yaml:/usr/local/apisix/conf/apisix.yaml:ro
       ports:
         - target: 9080
           host_ip: 127.0.0.1
           protocol: tcp
   
     upstream:
       image: nginx:alpine
       container_name: apisix-issue-13357-upstream
   ```
   
   `config.yaml`
   
   ```yaml
   deployment:
     role: data_plane
     role_data_plane:
       config_provider: yaml
   
   apisix:
     node_listen: 9080
   
   plugins:
     - serverless-pre-function
     - limit-count
   ```
   
   `apisix.yaml`
   
   ```yaml
   routes:
     - id: issue-13357
       uri: /issue-13357
       plugins:
         serverless-pre-function:
           phase: access
           functions:
             - |
               return function(conf, ctx)
                   local limit_count = require("apisix.plugins.limit-count.init")
   
                   local broken_limit_count_conf = {
                       policy = "local",
                       count = 5,
                       time_window = 60,
                       key = "remote_addr",
                       key_type = "var",
                       rejected_code = 503,
                       allow_degradation = false,
                       show_limit_quota_header = true,
                       _meta = {}
                   }
   
                   return limit_count.rate_limit(broken_limit_count_conf, ctx, "limit-count", 1)
               end
       upstream:
         type: roundrobin
         nodes:
           "upstream:80": 1
   #END
   ```
   
   Then run:
   
   ```bash
   cd issue-13357-limit-count-error-repro
   docker compose up -d
   APISIX_URL="http://$(docker compose port apisix 9080)"
   curl -i "$APISIX_URL/issue-13357"
   docker compose logs --tail=120 apisix
   ```
   
   On the current code, the request returns `500`, and the APISIX log shows:
   
   ```text
   /usr/local/apisix/apisix/plugins/limit-count/init.lua:262: bad argument #2 to 'error' (number expected, got string)
   stack traceback:
       [C]: in function 'error'
       /usr/local/apisix/apisix/plugins/limit-count/init.lua:262: in function 'gen_limit_key'
       /usr/local/apisix/apisix/plugins/limit-count/init.lua:398: in function 'run_rate_limit'
   ```
   
   With the proposed change, I would expect the request to still fail, but with
   the intended message instead:
   
   ```text
   failed to generate key invalid parent: null
   ```
   
   One extra context point: in the normal route/service/plugin_config loading path, APISIX should inject `conf._meta.parent.resource_key` via `set_plugins_meta_parent()`. So I think this issue is specifically about making
   the defensive error path report the intended message, while any real execution path that loses `_meta.parent.resource_key` would probably be a separate issue.
   


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] fix(limit-count): incorrect error() call in gen_limit_key passes string as level argument [apisix]

Posted by "suryaparua-official (via GitHub)" <gi...@apache.org>.
suryaparua-official commented on issue #13357:
URL: https://github.com/apache/apisix/issues/13357#issuecomment-4550540032

   Hi @Baoyuantop, thank you for the review!
   I've already added a regression test in t/plugin/limit-count5.t (TEST 9) that covers the missing _meta.parent path.
   Regarding similar patterns in the codebase — @hachi029 pointed out a similar issue in apisix/discovery/dns/init.lua:81. I will scan the codebase for all error(message, string) patterns and fix them in this PR.
   I'll update the PR shortly.


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] fix(limit-count): incorrect error() call in gen_limit_key passes string as level argument [apisix]

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan closed issue #13357: fix(limit-count): incorrect error() call in gen_limit_key passes string as level argument
URL: https://github.com/apache/apisix/issues/13357


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org