[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #9841: change(ua-restriction): allowlist and denylist can't be enabled at the same time

[via GitHub]

monkeyDluffy6017 commented on code in PR #9841:
URL: https://github.com/apache/apisix/pull/9841#discussion_r1266131818


##########
apisix/plugins/ua-restriction.lua:
##########
@@ -128,12 +144,13 @@ function _M.access(conf, ctx)
             return 403, { message = conf.message }
         end
     end
-    local match = MATCH_NONE
+    local match
+
     if type(user_agent) == "table" then
         for _, v in ipairs(user_agent) do
             if type(v) == "string" then
                 match = lrucache_useragent(v, conf, match_user_agent, v, conf)
-                if match > MATCH_ALLOW then
+                if (conf.denylist and not match) or (conf.allowlist and match) 
then

Review Comment:
   The logic is very complicated, Why not add a check function for each of 
denylist and allowlist?
   For example
   ```
   local is_passed
   if conf.allowlist then
       is_passed = check_with_allow_list(user_agent, conf.allowlist)
   else 
       is_passed = check_with_deny_list(user_agent, conf.denylist)
   end
   
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org