fix(limit-req): ensure safe eviction of keys in redis (apache#12911)

Author: shreemaan-abhishek

apisix/plugins/limit-req/util.lua

@@ -57,14 +57,15 @@ function _M.incoming(self, red, key, commit)
     end
 
     if commit then
-        local ok
-        local err
-        ok, err = red:set(excess_key, excess)
+        local ttl = math.ceil(self.burst / self.rate) + 1
+        local ok, err
+
+        ok, err = red:set(excess_key, excess, "EX", ttl)
         if not ok then
             return nil, err
         end
 
-        ok, err = red:set(last_key, now)
+        ok, err = red:set(last_key, now, "EX", ttl)
         if not ok then
             return nil, err
         end

ci/common.sh

@@ -177,7 +177,7 @@ GRPC_SERVER_EXAMPLE_VER=20210819
 
 linux_get_dependencies () {
     apt update
-    apt install -y cpanminus build-essential libncurses5-dev libreadline-dev libssl-dev perl libpcre3 libpcre3-dev libpcre2-dev xz-utils
+    apt install -y cpanminus build-essential libncurses5-dev libreadline-dev libssl-dev perl libpcre3 libpcre3-dev libpcre2-dev xz-utils redis-tools
     apt remove -y curl
     apt-get install -y libyaml-dev
     wget https://github.com/mikefarah/yq/releases/download/3.4.1/yq_linux_amd64 -O /usr/bin/yq && sudo chmod +x /usr/bin/yq

ci/pod/docker-compose.common.yml

@@ -18,6 +18,23 @@
 version: "3.8"
 
 services:
+  ## Redis
+  apisix_redis:
+    # The latest image is the latest stable version
+    image: redis:latest
+    restart: unless-stopped
+    volumes:
+      - ./t/certs:/certs
+    command: "--tls-port 6380 \
+            --tls-cert-file /certs/mtls_server.crt \
+            --tls-key-file /certs/mtls_server.key \
+            --tls-ca-cert-file /certs/mtls_ca.crt \
+            --tls-auth-clients no \
+            --user alice on +@all ~* \\&* \\>somepassword"
+    ports:
+      - "6379:6379"
+      - "6380:6380"
+
   ## Etcd
   etcd_old:
     image: bitnamilegacy/etcd:3.3.8

ci/pod/docker-compose.plugin.yml

@@ -18,25 +18,6 @@
 version: "3.8"
 
 services:
-  ## Redis
-  apisix_redis:
-    # The latest image is the latest stable version
-    image: redis:latest
-    restart: unless-stopped
-    volumes:
-      - ./t/certs:/certs
-    command: "--tls-port 6380 \
-            --tls-cert-file /certs/mtls_server.crt \
-            --tls-key-file /certs/mtls_server.key \
-            --tls-ca-cert-file /certs/mtls_ca.crt \
-            --tls-auth-clients no \
-            --user alice on +@all ~* \\&* \\>somepassword"
-    ports:
-      - "6379:6379"
-      - "6380:6380"
-    networks:
-      apisix_net:
-
   ## keycloak
   apisix_keycloak:
     container_name: apisix_keycloak

t/cli/test_limit_req_redis_ttl.sh

@@ -0,0 +1,114 @@
+#!/usr/bin/env bash
+
+#
+# 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.
+#
+
+. ./t/cli/common.sh
+
+# Enable limit-req plugin
+
+rm logs/worker_events.sock || true
+
+echo '
+nginx_config:
+  worker_processes: 1
+  error_log_level: info
+deployment:
+  admin:
+    admin_key:
+      - name: "admin"
+        key: edd1c9f034335f136f87ad84b625c8f1
+        role: admin
+
+apisix:
+  enable_admin: true
+  control:
+    port: 9110
+plugins:
+  - limit-req
+' > conf/config.yaml
+
+make init
+make run
+
+admin_key="edd1c9f034335f136f87ad84b625c8f1"
+
+# Create a route with limit-req and redis policy
+# rate=1, burst=1 -> ttl = ceil(1/1) + 1 = 2s
+curl -X PUT http://127.0.0.1:9180/apisix/admin/routes/1 \
+    -H "X-API-KEY: $admin_key" \
+    -d '{
+    "methods": ["GET"],
+    "uri": "/hello",
+    "plugins": {
+        "limit-req": {
+            "rate": 1,
+            "burst": 1,
+            "key": "remote_addr",
+            "policy": "redis",
+            "redis_host": "127.0.0.1",
+            "redis_timeout": 1000
+        }
+    },
+    "upstream": {
+        "type": "roundrobin",
+        "nodes": {
+            "127.0.0.1:1980": 1
+        }
+    }
+}'
+
+if [ $? -ne 0 ]; then
+    echo "failed: verify route creation"
+    exit 1
+fi
+
+sleep 0.5
+
+# Make a request to create the Redis keys
+curl -v http://127.0.0.1:9080/hello > /dev/null 2>&1
+
+# Verify keys exist
+# Keys pattern: plugin-limit-req*
+keys=$(redis-cli keys "limit_req:*" | wc -l)
+if [ "$keys" -eq 0 ]; then
+    echo "failed: keys not found in Redis immediately after request"
+    exit 1
+fi
+echo "pass: keys found in Redis"
+
+# Wait for 3 seconds (TTL is 2s)
+echo "Waiting for 3s..."
+sleep 3
+
+# Verify keys are gone
+keys_list=$(redis-cli keys "limit_req:*")
+
+if [ -n "$keys_list" ]; then
+    echo "failed: keys still exist in Redis after TTL expiration"
+    echo "Keys found:"
+    echo "$keys_list"
+
+    first_key=$(echo "$keys_list" | head -n 1)
+    echo "TTL of $first_key:"
+    redis-cli ttl "$first_key"
+
+    exit 1
+fi
+
+echo "pass: keys expired correctly"
+make stop