From dbe4917df71068abd1af2acc7cd33c2ff2d0ba81 Mon Sep 17 00:00:00 2001
From: Eloy Coto <eloy.coto@gmail.com>
Date: Wed, 4 Sep 2019 18:00:38 +0200
Subject: [PATCH] Policy: Rate-limit fix window exception

On rate-limit window can be set as 0 so the division that happens on
resty will fail due to division by 0 is not allowed.

This commit fixes that and validates that window value is bigger than 0,
if not set the default to 1. Also, add an integration test to prove this
scenario and update the JSON schema to verify that the number is a valid
integer.

FIX: THREESCALE-3382

Signed-off-by: Eloy Coto <eloy.coto@gmail.com>
---
 CHANGELOG.md                                  |  2 +
 .../policy/rate_limit/apicast-policy.json     |  4 +-
 .../apicast/policy/rate_limit/rate_limit.lua  |  3 ++
 t/apicast-policy-rate-limit.t                 | 45 +++++++++++++++++++
 4 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 039424cf1..32ba8773b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Fix issues when OPENTRACING_FORWARD_HEADER was set [PR #1109](https://github.com/3scale/APIcast/pull/1109), [THREESCALE-1660](https://issues.jboss.org/browse/THREESCALE-1660)
 - New TLS termination policy [PR #1108](https://github.com/3scale/APIcast/pull/1108), [THREESCALE-2898](https://issues.jboss.org/browse/THREESCALE-2898)
+- Fix exception on rate limit policy when window was set as 0. [PR #1113](https://github.com/3scale/APIcast/pull/1108), [THREESCALE-3382](https://issues.jboss.org/browse/THREESCALE-3382)
+
 
 
 ## [3.6.0-rc1] - 2019-07-04
diff --git a/gateway/src/apicast/policy/rate_limit/apicast-policy.json b/gateway/src/apicast/policy/rate_limit/apicast-policy.json
index 82d50680e..41fe33862 100644
--- a/gateway/src/apicast/policy/rate_limit/apicast-policy.json
+++ b/gateway/src/apicast/policy/rate_limit/apicast-policy.json
@@ -181,6 +181,7 @@
         "type": "array",
         "items": {
           "type": "object",
+          "required": ["key", "count", "window"],
           "properties": {
             "key": {
               "$ref": "#/definitions/key"
@@ -196,7 +197,8 @@
             "window": {
               "type": "integer",
               "description": "The time window in seconds before the request count is reset",
-              "exclusiveMinimum": 0
+              "exclusiveMinimum": 0,
+              "default": 1
             }
           }
         }
diff --git a/gateway/src/apicast/policy/rate_limit/rate_limit.lua b/gateway/src/apicast/policy/rate_limit/rate_limit.lua
index 8cdb8b1d6..3d1315acc 100644
--- a/gateway/src/apicast/policy/rate_limit/rate_limit.lua
+++ b/gateway/src/apicast/policy/rate_limit/rate_limit.lua
@@ -34,6 +34,9 @@ local traffic_limiters = {
     return resty_limit_req.new(shdict_key, config.rate, config.burst)
   end,
   fixed_window = function(config)
+    if not config.window or config.window <= 0 then
+        config.window = 1
+    end
     return resty_limit_count.new(shdict_key, config.count, config.window)
   end
 }
diff --git a/t/apicast-policy-rate-limit.t b/t/apicast-policy-rate-limit.t
index a6a138ff1..99a33371c 100644
--- a/t/apicast-policy-rate-limit.t
+++ b/t/apicast-policy-rate-limit.t
@@ -1313,3 +1313,48 @@ limit. The limit is set to 2. Only the third one should fail.
 [200, 200, 429]
 --- no_error_log
 [error]
+
+
+=== TEST 22: Window is set to 0 and default is 1.
+Return 429 code.
+--- http_config
+  include $TEST_NGINX_UPSTREAM_CONFIG;
+  lua_package_path "$TEST_NGINX_LUA_PATH";
+
+  init_by_lua_block {
+    require "resty.core"
+    ngx.shared.limiter:flush_all()
+    require('apicast.configuration_loader').mock({
+      services = {
+        {
+          id = 42,
+          proxy = {
+            policy_chain = {
+              {
+                name = "apicast.policy.rate_limit",
+                configuration = {
+                  fixed_window_limiters = {
+                    {
+                      key = {name = "test9", scope = "global"},
+                      count = 1,
+                      window = 0
+                    }
+                  },
+                  limits_exceeded_error = { status_code = 429 }
+                }
+              }
+            }
+          }
+        }
+      }
+    })
+  }
+  lua_shared_dict limiter 1m;
+
+--- config
+  include $TEST_NGINX_APICAST_CONFIG;
+
+--- pipelined_requests eval
+["GET /","GET /"]
+--- error_code eval
+[200, 429]