bugfix(CORS): using rewrite phase and add lru cache for multiple orig…

…in (#1531)
apache · May 6, 2020 · ffce4b7 · ffce4b7
1 parent 33b437d
commit ffce4b7
Show file tree

Hide file tree

Showing 2 changed files with 118 additions and 44 deletions.
diff --git a/apisix/plugins/cors.lua b/apisix/plugins/cors.lua
@@ -76,6 +76,30 @@ local _M = {
     schema = schema,
 }
 
+local function create_mutiple_origin_cache(conf)
+    if not str_find(conf.allow_origins, ",", 1, true) then
+        return nil
+    end
+    local origin_cache = {}
+    local iterator, err = re_gmatch(conf.allow_origins, "([^,]+)", "jiox")
+    if not iterator then
+        core.log.error("match origins failed: ", err)
+        return nil
+    end
+    while true do
+        local origin, err = iterator()
+        if err then
+            core.log.error("iterate origins failed: ", err)
+            return nil
+        end
+        if not origin then
+            break
+        end
+        origin_cache[origin[0]] = true
+    end
+    return origin_cache
+end
+
 function _M.check_schema(conf)
     local ok, err = core.schema.check(schema, conf)
     if not ok then
@@ -85,63 +109,48 @@ function _M.check_schema(conf)
     return true
 end
 
-function _M.access(conf, ctx)
+local function set_cors_headers(conf, ctx)
+    local allow_methods = conf.allow_methods
+    if allow_methods == "**" then
+        allow_methods = "GET,POST,PUT,DELETE,PATCH,HEAD,OPTIONS,CONNECT,TRACE"
+    end
+
+    ngx.header["Access-Control-Allow-Origin"] = ctx.cors_allow_origins
+    ngx.header["Access-Control-Allow-Methods"] = allow_methods
+    ngx.header["Access-Control-Allow-Headers"] = conf.allow_headers
+    ngx.header["Access-Control-Max-Age"] = conf.max_age
+    if conf.allow_credential then
+        ngx.header["Access-Control-Allow-Credentials"] = true
+    end
+    ngx.header["Access-Control-Expose-Headers"] = conf.expose_headers
+end
+
+function _M.rewrite(conf, ctx)
     local allow_origins = conf.allow_origins
+    local req_origin = core.request.header(ctx, "Origin")
     if allow_origins == "**" then
-        allow_origins = ngx.var.http_origin or '*'
+        allow_origins = req_origin or '*'
+    end
+    local multiple_origin, err = core.lrucache.plugin_ctx(plugin_name, ctx,
+                                                create_mutiple_origin_cache, conf)
+    if err then
+        return 500, {message = "get mutiple origin cache failed: " .. err}
     end
-    if str_find(allow_origins, ",", 1, true) then
-        local finded = false
-        local iterator, err = re_gmatch(allow_origins, "([^,]+)", "jiox")
-        if not iterator then
-            return 500, {message = "match origins failed", error = err}
-        end
-        while true do
-            local origin, err = iterator()
-            if err then
-                return 500, {message = "iterate origins failed", error = err}
-            end
-            if not origin then
-                break
-            end
 
-            if origin[0] == ngx.var.http_origin then
-                allow_origins = origin[0]
-                finded = true
-                break
-            end
-        end
-        if not finded then
+    if multiple_origin then
+        if multiple_origin[req_origin] then
+            allow_origins = req_origin
+        else
             return
         end
     end
 
     ctx.cors_allow_origins = allow_origins
+    set_cors_headers(conf, ctx)
 
     if ctx.var.request_method == "OPTIONS" then
         return 200
     end
 end
 
-function _M.header_filter(conf, ctx)
-    if not ctx.cors_allow_origins then
-        -- no origin matched, don't add headers
-        return
-    end
-
-    local allow_methods = conf.allow_methods
-    if allow_methods == "**" then
-        allow_methods = "GET,POST,PUT,DELETE,PATCH,HEAD,OPTIONS,CONNECT,TRACE"
-    end
-
-    ngx.header["Access-Control-Allow-Origin"] = ctx.cors_allow_origins
-    ngx.header["Access-Control-Allow-Methods"] = allow_methods
-    ngx.header["Access-Control-Allow-Headers"] = conf.allow_headers
-    ngx.header["Access-Control-Expose-Headers"] = conf.expose_headers
-    ngx.header["Access-Control-Max-Age"] = conf.max_age
-    if conf.allow_credential then
-        ngx.header["Access-Control-Allow-Credentials"] = true
-    end
-end
-
 return _M
diff --git a/t/plugin/cors.t b/t/plugin/cors.t
@@ -426,3 +426,68 @@ OPTIONS /hello HTTP/1.1
 
 --- no_error_log
 [error]
+
+
+
+=== TEST 15: set route(auth plugins faills)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                    "plugins": {
+                        "key-auth": {},
+                        "cors": {
+                            "allow_origins": "**",
+                            "allow_methods": "**",
+                            "allow_headers": "*",
+                            "expose_headers": "*"
+                        }
+                    },
+                    "upstream": {
+                        "nodes": {
+                            "127.0.0.1:1980": 1
+                        },
+                        "type": "roundrobin"
+                    },
+                    "uri": "/hello"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: auth failed still work
+--- request
+GET /hello HTTP/1.1
+--- more_headers
+Origin: https://sub.domain.com
+ExternalHeader1: val
+ExternalHeader2: val
+ExternalHeader3: val
+--- response_body
+{"message":"Missing API key found in request"}
+--- error_code: 401
+--- response_headers
+Access-Control-Allow-Origin: https://sub.domain.com
+Access-Control-Allow-Methods: GET,POST,PUT,DELETE,PATCH,HEAD,OPTIONS,CONNECT,TRACE
+Access-Control-Allow-Headers: *
+Access-Control-Expose-Headers: *
+Access-Control-Max-Age: 5
+Access-Control-Allow-Credentials:
+--- no_error_log
+[error]