[trafficserver] branch 8.1.x updated: Removes the abort() from header_rewrite, and try to deal with errors

[zwoop]

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

zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/8.1.x by this push:
     new 4566854  Removes the abort() from header_rewrite, and try to deal with 
errors
4566854 is described below

commit 4566854918bcebf051e299180b6f46d3af42bcb2
Author: Leif Hedstrom <zw...@apache.org>
AuthorDate: Thu May 23 16:31:55 2019 -0600

    Removes the abort() from header_rewrite, and try to deal with errors
    
    Bryan suggested using exceptions here, rather than just producing errors
    in the log files. As ugly as it is, it does seem to work ... But I blame
    him for all future issues! Long term, I think it'd be better to eliminate
    the exceptions here, and fix all the various layers of returning and
    perculating errors all the way back up through the stack.
    
    (cherry picked from commit 244ecc65e237e9b219357aacbee0c5f961765756)
---
 plugins/header_rewrite/header_rewrite.cc | 25 ++++++++++++++++---------
 plugins/header_rewrite/matcher.h         |  7 +++++--
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/plugins/header_rewrite/header_rewrite.cc 
b/plugins/header_rewrite/header_rewrite.cc
index 60caecd..99b1961 100644
--- a/plugins/header_rewrite/header_rewrite.cc
+++ b/plugins/header_rewrite/header_rewrite.cc
@@ -17,6 +17,7 @@
 */
 #include <fstream>
 #include <string>
+#include <stdexcept>
 
 #include "ts/ts.h"
 #include "ts/remap.h"
@@ -220,16 +221,22 @@ RulesConfig::parse_config(const std::string &fname, 
TSHttpHookID default_hook)
       }
     }
 
-    if (p.is_cond()) {
-      if (!rule->add_condition(p, filename.c_str(), lineno)) {
-        delete rule;
-        return false;
-      }
-    } else {
-      if (!rule->add_operator(p, filename.c_str(), lineno)) {
-        delete rule;
-        return false;
+    // This is pretty ugly, but it turns out, some conditions / operators can 
fail (regexes), which didn't use to be the case.
+    // Long term, maybe we need to percolate all this up through 
add_condition() / add_operator() rather than this big ugly try.
+    try {
+      if (p.is_cond()) {
+        if (!rule->add_condition(p, filename.c_str(), lineno)) {
+          throw std::runtime_error("add_condition() failed");
+        }
+      } else {
+        if (!rule->add_operator(p, filename.c_str(), lineno)) {
+          throw std::runtime_error("add_operator() failed");
+        }
       }
+    } catch (std::runtime_error &e) {
+      TSError("[%s] header_rewrite configuration exception: %s", PLUGIN_NAME, 
e.what());
+      delete rule;
+      return false;
     }
   }
 
diff --git a/plugins/header_rewrite/matcher.h b/plugins/header_rewrite/matcher.h
index 0e285e5..88a2399 100644
--- a/plugins/header_rewrite/matcher.h
+++ b/plugins/header_rewrite/matcher.h
@@ -23,6 +23,7 @@
 
 #include <string>
 #include <sstream>
+#include <stdexcept>
 
 #include "ts/ts.h"
 
@@ -94,9 +95,11 @@ public:
       std::stringstream ss;
       ss << _data;
       TSError("[%s] Invalid regex: failed to precompile: %s", PLUGIN_NAME, 
ss.str().c_str());
-      abort();
+      TSDebug(PLUGIN_NAME, "Invalid regex: failed to precompile: %s", 
ss.str().c_str());
+      throw std::runtime_error("Malformed regex");
+    } else {
+      TSDebug(PLUGIN_NAME, "Regex precompiled successfully");
     }
-    TSDebug(PLUGIN_NAME, "Regex precompiled successfully");
   }
 
   void