Fix potential bugs with rewriting API backend redirects.

There were 2 bugs that could crop up when rewriting API backend
redirects to match the frontend URL prefix:

1. The first "URL match" that matched the redirect's path would be used,
   but that wasn't necessarily the "URL match" that was actually being
   used for routing to the API backend. This could be problematic for
   API backends with multiple URL matches, if a more general prefix
   matched before the actual match being used.

   Fix this by only using the "URL match" belonging to the active one
   that was used during routing.
2. If the API backend responded with URL paths that were already
   rewritten to match the frontend prefix, the frontend prefix could end
   up getting duplicated twice in the resulting URL.

   We now try to better detect this situation and skip redirect
   rewriting if it appears like the redirect being returned is already
   valid for the given frontend prefix.

Add more comprehensive redirect tests to try and better capture more of
the possible redirect scenarios.

Also make a couple tweaks to prefer regexes over lua string.gsub (since
regexes can be compiled and should be more efficient).

In the test suite, improve the unique hostname generation and better
ensure thread-safety for generating unique IPs and numbers.

src/api-umbrella/proxy/middleware/api_matcher.lua

@@ -3,7 +3,6 @@ local stringx = require "pl.stringx"
 local utils = require "api-umbrella.proxy.utils"
 
 local append_array = utils.append_array
-local gsub = string.gsub
 local set_uri = utils.set_uri
 local startswith = stringx.startswith
 
@@ -50,8 +49,10 @@ return function(active_config)
 
   if api and url_match then
     -- Rewrite the URL prefix path.
-    local new_path = gsub(request_path, url_match["_frontend_prefix_matcher"], url_match["backend_prefix"], 1)
-    if new_path ~= request_path then
+    local new_path, _, new_path_err = ngx.re.sub(request_path, url_match["_frontend_prefix_regex"], url_match["backend_prefix"], "jo")
+    if new_path_err then
+      ngx.log(ngx.ERR, "regex error: ", new_path_err)
+    elseif new_path ~= request_path then
       set_uri(new_path)
     end
 

src/api-umbrella/proxy/middleware/rewrite_response.lua

@@ -3,7 +3,6 @@ local url = require "socket.url"
 local utils = require "api-umbrella.proxy.utils"
 
 local append_args = utils.append_args
-local gsub = string.gsub
 local startswith = stringx.startswith
 local url_build = url.build
 local url_parse = url.parse
@@ -126,14 +125,30 @@ local function rewrite_redirects()
     changed = true
   end
 
-  if host_matches or relative then
-    if matched_api and matched_api["url_matches"] then
-      for _, url_match in ipairs(matched_api["url_matches"]) do
-        if startswith(parsed["path"], url_match["backend_prefix"]) then
-          parsed["path"] = gsub(parsed["path"], url_match["_backend_prefix_matcher"], url_match["frontend_prefix"], 1)
-          changed = true
-          break
-        end
+  -- If the redirect being returned possibly contains paths for the underlying
+  -- backend URL, then rewrite the path.
+  if (host_matches or relative) and matched_api then
+    -- If the redirect path begins with the backend prefix, then consider it
+    -- for rewriting.
+    local url_match = ngx.ctx.matched_api_url_match
+    if url_match and startswith(parsed["path"], url_match["backend_prefix"]) then
+      -- As long as the patah matches the backend prefix, mark as changed, so
+      -- the api key is appended (regardless of whether we actually replaced
+      -- the path).
+      changed = true
+
+      -- Don't rewrite the path if the frontend prefix contains the backend
+      -- prefix and the redirect path already contains the frontend prefix.
+      --
+      -- This helps ensure that if the API backend is already returning
+      -- public/frontend URLs, we don't try to rewrite these again. -
+      local rewrite_path = true
+      if url_match["_frontend_prefix_contains_backend_prefix"] and startswith(parsed["path"], url_match["frontend_prefix"]) then
+        rewrite_path = false
+      end
+
+      if rewrite_path then
+        parsed["path"] = ngx.re.sub(parsed["path"], url_match["_backend_prefix_regex"], url_match["frontend_prefix"], "jo")
       end
     end
   end

src/api-umbrella/proxy/models/active_config.lua

@@ -9,11 +9,11 @@ local random_token = require "api-umbrella.utils.random_token"
 local resolve_backend_dns = require "api-umbrella.proxy.jobs.resolve_backend_dns"
 local tablex = require "pl.tablex"
 local utils = require "api-umbrella.proxy.utils"
+local startswith = require("pl.stringx").startswith
 
 local append_array = utils.append_array
 local cache_computed_settings = utils.cache_computed_settings
 local deepcopy = tablex.deepcopy
-local escape = plutils.escape
 local set_packed = utils.set_packed
 local size = tablex.size
 local split = plutils.split
@@ -55,8 +55,18 @@ local function cache_computed_api(api)
 
   if api["url_matches"] then
     for _, url_match in ipairs(api["url_matches"]) do
-      url_match["_frontend_prefix_matcher"] = "^" .. escape(url_match["frontend_prefix"])
-      url_match["_backend_prefix_matcher"] = "^" .. escape(url_match["backend_prefix"])
+      url_match["_frontend_prefix_regex"] = "^" .. escape_regex(url_match["frontend_prefix"])
+      url_match["_backend_prefix_regex"] = "^" .. escape_regex(url_match["backend_prefix"])
+
+      url_match["_frontend_prefix_contains_backend_prefix"] = false
+      if startswith(url_match["frontend_prefix"], url_match["backend_prefix"]) then
+        url_match["_frontend_prefix_contains_backend_prefix"] = true
+      end
+
+      url_match["_backend_prefix_contains_frontend_prefix"] = false
+      if startswith(url_match["backend_prefix"], url_match["frontend_prefix"]) then
+        url_match["_backend_prefix_contains_frontend_prefix"] = true
+      end
     end
   end
 

templates/etc/test-env/nginx/apis.conf.mustache

@@ -92,7 +92,7 @@ server {
     echo -n "Hello World";
   }
 
-  location = /redirect {
+  location /redirect {
     rewrite_by_lua_block {
       ngx.redirect(ngx.unescape_uri(ngx.var.arg_to or "/hello"))
     }