You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by so...@apache.org on 2016/04/21 19:18:04 UTC

[trafficserver] 01/03: TS-4318 Fix a regression in regex rules

This is an automated email from the ASF dual-hosted git repository.

sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git

commit bad058eb152772f1df0fe4c133f4ac62fa0eaa12
Author: Leif Hedstrom <zw...@apache.org>
AuthorDate: Mon Apr 4 15:06:47 2016 -0600

    TS-4318 Fix a regression in regex rules
    
    The refactoring done earlier broke the config loading of rules using
    the regular expressions. This restore that functionality, but cleaner.
    
    (cherry picked from commit 431a8f838e75338cb685b95c213a6140f5cbdcc7)
---
 plugins/experimental/geoip_acl/acl.cc       | 25 ++++++++++++++++---------
 plugins/experimental/geoip_acl/acl.h        |  8 +++++---
 plugins/experimental/geoip_acl/geoip_acl.cc |  1 +
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/plugins/experimental/geoip_acl/acl.cc b/plugins/experimental/geoip_acl/acl.cc
index 9b30679..e5e09ff 100644
--- a/plugins/experimental/geoip_acl/acl.cc
+++ b/plugins/experimental/geoip_acl/acl.cc
@@ -120,7 +120,7 @@ Acl::read_html(const char *fn)
 
 // Implementations for the RegexAcl class
 bool
-RegexAcl::parse_line(const char *filename, const std::string &line, int lineno)
+RegexAcl::parse_line(const char *filename, const std::string &line, int lineno, int &tokens)
 {
   static const char _SEPARATOR[] = " \t\n";
   std::string regex, tmp;
@@ -155,6 +155,7 @@ RegexAcl::parse_line(const char *filename, const std::string &line, int lineno)
           pos2 = line.find_first_of(_SEPARATOR, pos1);
           tmp = line.substr(pos1, pos2 - pos1);
           _acl->add_token(tmp);
+          ++tokens;
         }
         compile(regex, filename, lineno);
         TSDebug(PLUGIN_NAME, "Added regex rule for /%s/", regex.c_str());
@@ -220,7 +221,7 @@ CountryAcl::add_token(const std::string &str)
 }
 
 void
-CountryAcl::read_regex(const char *fn)
+CountryAcl::read_regex(const char *fn, int &tokens)
 {
   std::ifstream f;
   int lineno = 0;
@@ -234,7 +235,7 @@ CountryAcl::read_regex(const char *fn)
       getline(f, line);
       ++lineno;
       acl = new RegexAcl(new CountryAcl());
-      if (acl->parse_line(fn, line, lineno)) {
+      if (acl->parse_line(fn, line, lineno, tokens)) {
         if (NULL == _regexes) {
           _regexes = acl;
         } else {
@@ -255,6 +256,9 @@ CountryAcl::read_regex(const char *fn)
 bool
 CountryAcl::eval(TSRemapRequestInfo *rri, TSHttpTxn txnp) const
 {
+  bool ret = _allow;
+
+  TSDebug(PLUGIN_NAME, "CountryAcl::eval() called, default ACL is %s", ret ? "allow" : "deny");
   // If there are regex rules, they take priority first. If a regex matches, we will
   // honor it's eval() rule. If no regexes matches, fall back on the default (which is
   // "allow" if nothing else is specified).
@@ -269,16 +273,19 @@ CountryAcl::eval(TSRemapRequestInfo *rri, TSHttpTxn txnp) const
         return acl->eval(rri, txnp);
       }
     } while ((acl = acl->next()));
+    ret = !_allow; // Now we invert the default since no regexes matched
   }
 
   // None of the regexes (if any) matched, so fallback to the remap defaults if there are any.
   int iso = country_id_by_addr(TSHttpTxnClientAddrGet(txnp));
 
-  if ((iso <= 0) || (!_iso_country_codes[iso])) {
-    return !_allow;
+  if ((iso <= 0) || !_iso_country_codes[iso]) {
+    TSDebug(PLUGIN_NAME, "ISO not found in table, returning %d", !ret);
+    return !ret;
   }
 
-  return _allow;
+  TSDebug(PLUGIN_NAME, "ISO was found in table, or -1, returning %d", ret);
+  return ret;
 }
 
 int
@@ -288,11 +295,11 @@ CountryAcl::process_args(int argc, char *argv[])
 
   for (int i = 3; i < argc; ++i) {
     if (!strncmp(argv[i], "allow", 5)) {
-      _allow = true;
+      set_allow(true);
     } else if (!strncmp(argv[i], "deny", 4)) {
-      _allow = false;
+      set_allow(false);
     } else if (!strncmp(argv[i], "regex::", 7)) {
-      read_regex(argv[i] + 7);
+      read_regex(argv[i] + 7, tokens);
     } else if (!strncmp(argv[i], "html::", 6)) {
       read_html(argv[i] + 6);
     } else { // ISO codes assumed for the rest
diff --git a/plugins/experimental/geoip_acl/acl.h b/plugins/experimental/geoip_acl/acl.h
index 5067a75..45bd500 100644
--- a/plugins/experimental/geoip_acl/acl.h
+++ b/plugins/experimental/geoip_acl/acl.h
@@ -51,7 +51,7 @@ public:
   Acl() : _allow(true), _added_tokens(0) {}
   virtual ~Acl() {}
   // These have to be implemented for each ACL type
-  virtual void read_regex(const char *fn) = 0;
+  virtual void read_regex(const char *fn, int &tokens) = 0;
   virtual int process_args(int argc, char *argv[]) = 0;
   virtual bool eval(TSRemapRequestInfo *rri, TSHttpTxn txnp) const = 0;
   virtual void add_token(const std::string &str) = 0;
@@ -83,6 +83,8 @@ protected:
   std::string _html;
   bool _allow;
   int _added_tokens;
+
+  // Class members
   static GeoDBHandle _geoip;
   static GeoDBHandle _geoip6;
 };
@@ -119,7 +121,7 @@ public:
   }
 
   void append(RegexAcl *ra);
-  bool parse_line(const char *filename, const std::string &line, int lineno);
+  bool parse_line(const char *filename, const std::string &line, int lineno, int &tokens);
 
 private:
   bool compile(const std::string &str, const char *filename, int lineno);
@@ -135,7 +137,7 @@ class CountryAcl : public Acl
 {
 public:
   CountryAcl() : _regexes(NULL) { memset(_iso_country_codes, 0, sizeof(_iso_country_codes)); }
-  void read_regex(const char *fn);
+  void read_regex(const char *fn, int &tokens);
   int process_args(int argc, char *argv[]);
   bool eval(TSRemapRequestInfo *rri, TSHttpTxn txnp) const;
   void add_token(const std::string &str);
diff --git a/plugins/experimental/geoip_acl/geoip_acl.cc b/plugins/experimental/geoip_acl/geoip_acl.cc
index 63072b0..1e5788e 100644
--- a/plugins/experimental/geoip_acl/geoip_acl.cc
+++ b/plugins/experimental/geoip_acl/geoip_acl.cc
@@ -105,6 +105,7 @@ TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri)
     Acl *a = static_cast<Acl *>(ih);
 
     if (!a->eval(rri, rh)) {
+      TSDebug(PLUGIN_NAME, "denying request");
       TSHttpTxnSetHttpRetStatus((TSHttpTxn)rh, (TSHttpStatus)403);
       a->send_html((TSHttpTxn)rh);
     }

-- 
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.