[GitHub] [trafficserver] ezelkow1 commented on a diff in pull request #9134: Adding regex capability based on pristine host name

[GitBox <gi...@apache.org>]

ezelkow1 commented on code in PR #9134:
URL: https://github.com/apache/trafficserver/pull/9134#discussion_r992808048


##########
plugins/experimental/maxmind_acl/mmdb.cc:
##########
@@ -694,19 +740,39 @@ Acl::eval_country(MMDB_entry_data_s *entry_data, const char *path, int path_len)
     ret = true;
   }
 
+  // test for path regex
   if (nullptr != path && 0 != path_len) {
-    if (!allow_regex[output].empty()) {
-      for (auto &i : allow_regex[output]) {
+    if (!allow_regex_path[output].empty()) {
+      for (auto &i : allow_regex_path[output]) {
         if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
-          TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+          TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
           ret = true;
         }
       }
     }
-    if (!deny_regex[output].empty()) {
-      for (auto &i : deny_regex[output]) {
+    if (!deny_regex_path[output].empty()) {
+      for (auto &i : deny_regex_path[output]) {
         if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
-          TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+          TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
+          ret = false;

Review Comment:
   true, thanks, yea the host section for the deny should probably check if it is already set to false and if it is leave it as false. Though the plugin can run in sort of a 'reverse' mode, where if you only set allow fields then by default it will deny everything else that isnt explicitly allowed.
   
   So let me think about this, see if there is a way to logic a way out based on if its running in deny-everything mode plus there has been an allow or the default mode and you've already gotten a denial based on path (this was always the most brain hurting part of this plugin)



-- 
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: github-unsubscribe@trafficserver.apache.org

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