From 33e96ac9ef65e74eba4ec03dcf172a9c871b71d5 Mon Sep 17 00:00:00 2001
From: Thibault Charbonnier <thibaultcha@me.com>
Date: Mon, 28 Jan 2019 16:45:44 -0800
Subject: [PATCH] fix(router) ensure rebuilds when Routes have 'regex_priority
 = nil'

If Routes have `regex_priority` properties mixed between numbers and nil
(e.g. one Route was created with a default `regex_priority`, the other
was created with a purposefully NULL `regex_priority`, the comparison in
the router rebuild sorting of Routes would error out, because we are
comparing one Route's `nil` value against another Route's `number`.

Fix #4254
---
 kong/runloop/handler.lua                      |  9 ++++--
 .../05-proxy/02-router_spec.lua               | 31 +++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/kong/runloop/handler.lua b/kong/runloop/handler.lua
index 8b2d82eea9e..196d0e4f97e 100644
--- a/kong/runloop/handler.lua
+++ b/kong/runloop/handler.lua
@@ -110,10 +110,15 @@ do
 
     sort(routes, function(r1, r2)
       r1, r2 = r1.route, r2.route
-      if r1.regex_priority == r2.regex_priority then
+
+      local rp1 = r1.regex_priority or 0
+      local rp2 = r2.regex_priority or 0
+
+      if rp1 == rp2 then
         return r1.created_at < r2.created_at
       end
-      return r1.regex_priority > r2.regex_priority
+
+      return rp1 > rp2
     end)
 
     local err
diff --git a/spec/02-integration/05-proxy/02-router_spec.lua b/spec/02-integration/05-proxy/02-router_spec.lua
index 0ea525f64bf..7eac039b055 100644
--- a/spec/02-integration/05-proxy/02-router_spec.lua
+++ b/spec/02-integration/05-proxy/02-router_spec.lua
@@ -49,6 +49,10 @@ local function insert_routes(routes)
 end
 
 local function remove_routes(routes)
+  if not routes then
+    return
+  end
+
   local services = {}
 
   for _, route in ipairs(routes) do
@@ -1093,7 +1097,34 @@ for _, strategy in helpers.each_strategy() do
         end
       end)
 
+      describe("router rebuilds", function()
+        local routes
+
+        lazy_teardown(function()
+          remove_routes(routes)
+        end)
 
+        it("when Routes have 'regex_priority = nil'", function()
+          -- Regression test for issue:
+          -- https://github.com/Kong/kong/issues/4254
+          routes = insert_routes {
+            {
+              methods = { "GET" },
+              regex_priority = 1,
+            },
+            {
+              methods = { "POST", "PUT" },
+              regex_priority = ngx.null,
+            },
+          }
+
+          local res = assert(proxy_client:send {
+            method  = "GET",
+          })
+
+          assert.response(res).has_status(200)
+        end)
+      end)
     end)
   end)
 end