trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bri...@apache.org
Subject git commit: Backport TS-948
Date Sat, 25 Feb 2012 19:41:55 GMT
Updated Branches:
  refs/heads/3.0.x eac7c6be8 -> 2f9ccb0e2


Backport TS-948


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/2f9ccb0e
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/2f9ccb0e
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/2f9ccb0e

Branch: refs/heads/3.0.x
Commit: 2f9ccb0e21cdcf8392ee15f909f2b12d8b35d991
Parents: eac7c6b
Author: Brian Geffon <briang@apache.org>
Authored: Sat Feb 25 11:41:43 2012 -0800
Committer: Brian Geffon <briang@apache.org>
Committed: Sat Feb 25 11:41:43 2012 -0800

----------------------------------------------------------------------
 CHANGES                             |    2 +
 STATUS                              |    8 ---
 proxy/ReverseProxy.cc               |   20 ++++++-
 proxy/http/remap/RemapPluginInfo.cc |    2 +-
 proxy/http/remap/RemapPluginInfo.h  |    2 +-
 proxy/http/remap/UrlRewrite.cc      |   81 +++++++++++++----------------
 proxy/http/remap/UrlRewrite.h       |    4 +-
 7 files changed, 60 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2f9ccb0e/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 8cce3f2..33bb15d 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 3.0.4
 
+  *) [TS-948] do not reload bad remap.config
+
   *) [TS-845] make proxy.config.cluster.ethernet_interface default to
    loopback interface: lo on linux and lo0 on bsd derivatives
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2f9ccb0e/STATUS
----------------------------------------------------------------------
diff --git a/STATUS b/STATUS
index 5cb6254..cfc44b5 100644
--- a/STATUS
+++ b/STATUS
@@ -41,14 +41,6 @@ A list of all bugs open for the next v3.0.4 release can be found at
 
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
 
-  *) do not reload bad remap.config
-   Trunk patch: https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;a=commit;h=31500d106e472a8713c1c3976b161f2dea56ba38
-     https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;a=commit;h=50be413a667ffc10ea32e2d419165584aea008eb
-     https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;a=commit;h=a90a976eff61546340fc1d49d4faffeaa49a3cf2
-     https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;a=commit;h=c2ddca4fac3d5cbacae27cddba87fe07c8651ec0
-   Jira: https://issues.apache.org/jira/browse/TS-948
-   +1: zym, zwoop, igalic
-
   *) Traffic Cop: segment fault when TRACE_LOG_COP is enabled
    Trunk patch: http://svn.apache.org/viewvc?view=rev&rev=1225724
    Jira: https://issues.apache.org/jira/browse/TS-1065

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2f9ccb0e/proxy/ReverseProxy.cc
----------------------------------------------------------------------
diff --git a/proxy/ReverseProxy.cc b/proxy/ReverseProxy.cc
index df3d23f..e9631c3 100644
--- a/proxy/ReverseProxy.cc
+++ b/proxy/ReverseProxy.cc
@@ -78,6 +78,13 @@ init_reverse_proxy()
   reconfig_mutex = new_ProxyMutex();
   rewrite_table = NEW(new UrlRewrite("proxy.config.url_remap.filename"));
 
+  if (!rewrite_table->is_valid()) {
+    Warning("Can not load the remap table, exiting out!");
+    // TODO: For now, I _exit() out of here, because otherwise we'll keep generating
+    // core files (if enabled) when starting up with a bad remap.config file.
+    _exit(-1);
+  }
+
   REVERSE_RegisterConfigUpdateFunc("proxy.config.url_remap.filename", url_rewrite_CB, (void
*) FILE_CHANGED);
   REVERSE_RegisterConfigUpdateFunc("proxy.config.proxy_name", url_rewrite_CB, (void *) TSNAME_CHANGED);
   REVERSE_RegisterConfigUpdateFunc("proxy.config.reverse_proxy.enabled", url_rewrite_CB,
(void *) REVERSE_CHANGED);
@@ -183,10 +190,17 @@ reloadUrlRewrite()
   UrlRewrite *newTable;
 
   Debug("url_rewrite", "remap.config updated, reloading...");
-  eventProcessor.schedule_in(new UR_FreerContinuation(rewrite_table), URL_REWRITE_TIMEOUT,
ET_TASK);
   newTable = new UrlRewrite("proxy.config.url_remap.filename");
-  Debug("url_rewrite", "remap.config done reloading!");
-  ink_atomic_swap_ptr(&rewrite_table, newTable);
+  if (newTable->is_valid()) {
+    eventProcessor.schedule_in(new UR_FreerContinuation(rewrite_table), URL_REWRITE_TIMEOUT,
ET_TASK);
+    Debug("url_rewrite", "remap.config done reloading!");
+    ink_atomic_swap_ptr(&rewrite_table, newTable);
+  } else {
+    static const char* msg = "failed to reload remap.config, not replacing!";
+    delete newTable;
+    Debug("url_rewrite", msg);
+    Warning(msg);
+  }
 }
 
 int

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2f9ccb0e/proxy/http/remap/RemapPluginInfo.cc
----------------------------------------------------------------------
diff --git a/proxy/http/remap/RemapPluginInfo.cc b/proxy/http/remap/RemapPluginInfo.cc
index 99fedfd..0ce5c6f 100644
--- a/proxy/http/remap/RemapPluginInfo.cc
+++ b/proxy/http/remap/RemapPluginInfo.cc
@@ -24,7 +24,7 @@
 #include "RemapPluginInfo.h"
 
 remap_plugin_info::remap_plugin_info(char *_path)
-  :  next(0), path(NULL), path_size(0), dlh(NULL), fp_tsremap_init(NULL), fp_tsremap_done(NULL),
fptsremap_new_instance(NULL),
+  :  next(0), path(NULL), path_size(0), dlh(NULL), fp_tsremap_init(NULL), fp_tsremap_done(NULL),
fp_tsremap_new_instance(NULL),
      fp_tsremap_delete_instance(NULL), fp_tsremap_do_remap(NULL), fp_tsremap_os_response(NULL)

 {
   // coverity did not see xfree

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2f9ccb0e/proxy/http/remap/RemapPluginInfo.h
----------------------------------------------------------------------
diff --git a/proxy/http/remap/RemapPluginInfo.h b/proxy/http/remap/RemapPluginInfo.h
index 8a01eff..65ae63d 100644
--- a/proxy/http/remap/RemapPluginInfo.h
+++ b/proxy/http/remap/RemapPluginInfo.h
@@ -66,7 +66,7 @@ public:
   void *dlh;                    /* "handle" for the dynamic library */
   _tsremap_init *fp_tsremap_init;
   _tsremap_done *fp_tsremap_done;
-  _tsremap_new_instance *fptsremap_new_instance;
+  _tsremap_new_instance *fp_tsremap_new_instance;
   _tsremap_delete_instance *fp_tsremap_delete_instance;
   _tsremap_do_remap *fp_tsremap_do_remap;
   _tsremap_os_response *fp_tsremap_os_response;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2f9ccb0e/proxy/http/remap/UrlRewrite.cc
----------------------------------------------------------------------
diff --git a/proxy/http/remap/UrlRewrite.cc b/proxy/http/remap/UrlRewrite.cc
index 24a40e2..bb87e48 100644
--- a/proxy/http/remap/UrlRewrite.cc
+++ b/proxy/http/remap/UrlRewrite.cc
@@ -23,7 +23,6 @@
 
 #include "UrlRewrite.h"
 #include "Main.h"
-#include "Error.h"
 #include "P_EventSystem.h"
 #include "StatSystem.h"
 #include "P_Cache.h"
@@ -479,7 +478,7 @@ UrlRewrite::UrlRewrite(const char *file_var_in)
  : nohost_rules(0), reverse_proxy(0), backdoor_enabled(0),
    mgmt_autoconf_port(0), default_to_pac(0), default_to_pac_port(0), file_var(NULL), ts_name(NULL),
    http_default_redirect_url(NULL), num_rules_forward(0), num_rules_reverse(0), num_rules_redirect_permanent(0),
-   num_rules_redirect_temporary(0), num_rules_forward_with_recv_port(0)
+   num_rules_redirect_temporary(0), num_rules_forward_with_recv_port(0), _valid(false)
 {
 
   forward_mappings.hash_lookup = reverse_mappings.hash_lookup =
@@ -528,15 +527,16 @@ UrlRewrite::UrlRewrite(const char *file_var_in)
   strncat(config_file_path, config_file, sizeof(config_file_path) - strlen(config_file_path)
- 1);
   xfree(config_file);
 
-  if (this->BuildTable() != 0) {
+  if (0 == this->BuildTable()) {
+    _valid = true;
+    pcre_malloc = &ink_malloc;
+    pcre_free = &xfree;
+
+    if (is_debug_tag_set("url_rewrite"))
+     Print();
+  } else {
     Warning("something failed during BuildTable() -- check your remap plugins!");
   }
-
-  pcre_malloc = &ink_malloc;
-  pcre_free = &ink_free;
-
-  if (is_debug_tag_set("url_rewrite"))
-    Print();
 }
 
 
@@ -551,6 +551,7 @@ UrlRewrite::~UrlRewrite()
   DestroyStore(permanent_redirects);
   DestroyStore(temporary_redirects);
   DestroyStore(forward_mappings_with_recv_port);
+  _valid = false;
 }
 
 /** Sets the reverse proxy flag. */
@@ -1088,7 +1089,7 @@ UrlRewrite::BuildTable()
   memset(&bti, 0, sizeof(bti));
 
   if ((file_buf = readIntoBuffer(config_file_path, modulePrefix, NULL)) == NULL) {
-    ink_error("Can't load remapping configuration file - %s", config_file_path);
+	Warning("Can't load remapping configuration file - %s", config_file_path);
     return 1;
   }
 
@@ -1494,7 +1495,7 @@ UrlRewrite::BuildTable()
     Warning("Could not add rule at line #%d; Aborting!", cln + 1);
     snprintf(errBuf, sizeof(errBuf), "%s %s at line %d", modulePrefix, errStr, cln + 1);
     SignalError(errBuf, alarm_already);
-    ink_fatal(1, errBuf);
+    return 2;
   }                             /* end of while(cur_line != NULL) */
 
   clear_xstr_array(bti.paramv, sizeof(bti.paramv) / sizeof(char *));
@@ -1509,8 +1510,9 @@ UrlRewrite::BuildTable()
     if (TableInsert(forward_mappings.hash_lookup, new_mapping, "")) {
       num_rules_forward++;
     } else {
-      Error("Could not insert backdoor mapping into store");
+      Warning("Could not insert backdoor mapping into store");
       delete new_mapping;
+      return 3;
     }
   }
   // Add the default mapping to the manager PAC file
@@ -1520,8 +1522,9 @@ UrlRewrite::BuildTable()
     if (TableInsert(forward_mappings.hash_lookup, new_mapping, "")) {
       num_rules_forward++;
     } else {
-      Error("Could not insert pac mapping into store");
+      Warning("Could not insert pac mapping into store");
       delete new_mapping;
+      return 3;
     }
   }
   // Destroy unused tables
@@ -1575,7 +1578,7 @@ UrlRewrite::TableInsert(InkHashTable *h_table, url_mapping *mapping,
const char
     // There is already a path index for this host
     if (ht_contents == NULL) {
       // why should this happen?
-      Error("Found entry cannot be null!");
+      Warning("Found entry cannot be null!");
       return false;
     }
   } else {
@@ -1583,7 +1586,7 @@ UrlRewrite::TableInsert(InkHashTable *h_table, url_mapping *mapping,
const char
     ink_hash_table_insert(h_table, src_host, ht_contents);
   }
   if (!ht_contents->Insert(mapping)) {
-    Error("Could not insert new mapping");
+    Warning("Could not insert new mapping");
     return false;
   }
   return true;
@@ -1673,7 +1676,7 @@ UrlRewrite::load_remap_plugin(char *argv[], int argc, url_mapping *mp,
char *err
     }
     pi->fp_tsremap_init = (remap_plugin_info::_tsremap_init *) dlsym(pi->dlh, TSREMAP_FUNCNAME_INIT);
     pi->fp_tsremap_done = (remap_plugin_info::_tsremap_done *) dlsym(pi->dlh, TSREMAP_FUNCNAME_DONE);
-    pi->fptsremap_new_instance = (remap_plugin_info::_tsremap_new_instance *) dlsym(pi->dlh,
TSREMAP_FUNCNAME_NEW_INSTANCE);
+    pi->fp_tsremap_new_instance = (remap_plugin_info::_tsremap_new_instance *) dlsym(pi->dlh,
TSREMAP_FUNCNAME_NEW_INSTANCE);
     pi->fp_tsremap_delete_instance = (remap_plugin_info::_tsremap_delete_instance *) dlsym(pi->dlh,
TSREMAP_FUNCNAME_DELETE_INSTANCE);
     pi->fp_tsremap_do_remap = (remap_plugin_info::_tsremap_do_remap *) dlsym(pi->dlh,
TSREMAP_FUNCNAME_DO_REMAP);
     pi->fp_tsremap_os_response = (remap_plugin_info::_tsremap_os_response *) dlsym(pi->dlh,
TSREMAP_FUNCNAME_OS_RESPONSE);
@@ -1681,7 +1684,7 @@ UrlRewrite::load_remap_plugin(char *argv[], int argc, url_mapping *mp,
char *err
     if (!pi->fp_tsremap_init) {
       snprintf(errbuf, errbufsize, "Can't find \"%s\" function in remap plugin \"%s\"", TSREMAP_FUNCNAME_INIT,
c);
       retcode = -10;
-    } else if (!pi->fptsremap_new_instance) {
+    } else if (!pi->fp_tsremap_new_instance) {
       snprintf(errbuf, errbufsize, "Can't find \"%s\" function in remap plugin \"%s\"",
                    TSREMAP_FUNCNAME_NEW_INSTANCE, c);
       retcode = -11;
@@ -1701,27 +1704,27 @@ UrlRewrite::load_remap_plugin(char *argv[], int argc, url_mapping
*mp, char *err
     ri.tsremap_version = TSREMAP_VERSION;
 
     if (pi->fp_tsremap_init(&ri, tmpbuf, sizeof(tmpbuf) - 1) != TS_SUCCESS) {
-      Error("Failed to initialize plugin %s (non-zero retval) ... bailing out", pi->path);
-      exit(-1);                 //see my comment re: exit() about 60 lines down
+      Warning("Failed to initialize plugin %s (non-zero retval) ... bailing out", pi->path);
+      return -5;
     }
     Debug("remap_plugin", "Remap plugin \"%s\" - initialization completed", c);
   }
 
   if (!pi->dlh) {
     snprintf(errbuf, errbufsize, "Can't load plugin \"%s\"", c);
-    return -5;
+    return -6;
   }
 
   if ((err = mp->fromURL.string_get(NULL)) == NULL) {
     snprintf(errbuf, errbufsize, "Can't load fromURL from URL class");
-    return -6;
+    return -7;
   }
   parv[parc++] = xstrdup(err);
   xfree(err);
 
   if ((err = mp->toUrl.string_get(NULL)) == NULL) {
     snprintf(errbuf, errbufsize, "Can't load toURL from URL class");
-    return -6;
+    return -7;
   }
   parv[parc++] = xstrdup(err);
   xfree(err);
@@ -1757,7 +1760,7 @@ UrlRewrite::load_remap_plugin(char *argv[], int argc, url_mapping *mp,
char *err
   void* ih;
 
   Debug("remap_plugin", "creating new plugin instance");
-  TSReturnCode res = pi->fptsremap_new_instance(parc, parv, &ih, tmpbuf, sizeof(tmpbuf)
- 1);
+  TSReturnCode res = pi->fp_tsremap_new_instance(parc, parv, &ih, tmpbuf, sizeof(tmpbuf)
- 1);
 
   Debug("remap_plugin", "done creating new plugin instance");
 
@@ -1765,22 +1768,10 @@ UrlRewrite::load_remap_plugin(char *argv[], int argc, url_mapping
*mp, char *err
   xfree(parv[1]);               // toURL
 
   if (res != TS_SUCCESS) {
-    // TODO: This is such serious failure, no reason to try to delete the instance.
-    // mp->delete_instance(pi);
     snprintf(errbuf, errbufsize, "Can't create new remap instance for plugin \"%s\" - %s",
c,
                  tmpbuf[0] ? tmpbuf : "Unknown plugin error");
-    Error("Failed to create new instance for plugin %s (non-zero retval)... bailing out",
pi->path);
- 	 	 /**
-		 * fail here, otherwise we *will* fail later
-		 * and that's some jacked backtrace inside CreateTableLookup [see bug 2316658]
-		 * at least this one will be obvious
-		 * We *really* don't want to continue when a plugin failed to init. We can't
-		 * guarantee we are remapping what the user thought we were going to remap.
-		 * using something nice like exit() would be more ideal, but this should be
-		 * caught in development, anyway.
-     **/
-    exit(-1);
-    return -6;
+    Warning("Failed to create new instance for plugin %s (not a TS_SUCCESS return)", pi->path);
+    return -8;
   }
 
   mp->add_plugin(pi, ih);
@@ -1876,7 +1867,7 @@ UrlRewrite::_expandSubstitutions(int *matches_info, const RegexMapping
*reg_map,
   return cur_buf_size;
 
  lOverFlow:
-  Error("Overflow while expanding substitutions");
+  Warning("Overflow while expanding substitutions");
   return 0;
 }
 
@@ -1958,7 +1949,7 @@ UrlRewrite::_regexMappingLookup(RegexMappingList &regex_mappings,
URL *request_u
       Debug("url_rewrite_regex", "Request URL host [%.*s] did NOT match regex in mapping
of rank %d",
             request_host_len, request_host, reg_map_rank);
     } else {
-      Error("pcre_exec() failed with error code %d", match_result);
+      Warning("pcre_exec() failed with error code %d", match_result);
       break;
     }
   }
@@ -2012,23 +2003,23 @@ UrlRewrite::_processRegexMappingConfig(const char *from_host_lower,
url_mapping
   // as this one will be NULL-terminated (required by pcre_compile)
   reg_map->re = pcre_compile(from_host_lower, 0, &str, &str_index, NULL);
   if (reg_map->re == NULL) {
-    Error("pcre_compile failed! Regex has error starting at %s", from_host_lower + str_index);
+	Warning("pcre_compile failed! Regex has error starting at %s", from_host_lower + str_index);
     goto lFail;
   }
 
   reg_map->re_extra = pcre_study(reg_map->re, 0, &str);
   if ((reg_map->re_extra == NULL) && (str != NULL)) {
-    Error("pcre_study failed with message [%s]", str);
+    Warning("pcre_study failed with message [%s]", str);
     goto lFail;
   }
 
   int n_captures;
   if (pcre_fullinfo(reg_map->re, reg_map->re_extra, PCRE_INFO_CAPTURECOUNT, &n_captures)
!= 0) {
-    Error("pcre_fullinfo failed!");
+    Warning("pcre_fullinfo failed!");
     goto lFail;
   }
   if (n_captures >= MAX_REGEX_SUBS) { // off by one for $0 (implicit capture)
-    Error("Regex has %d capturing subpatterns (including entire regex); Max allowed: %d",
+    Warning("Regex has %d capturing subpatterns (including entire regex); Max allowed: %d",
           n_captures + 1, MAX_REGEX_SUBS);
     goto lFail;
   }
@@ -2037,13 +2028,13 @@ UrlRewrite::_processRegexMappingConfig(const char *from_host_lower,
url_mapping
   for (int i = 0; i < (to_host_len - 1); ++i) {
     if (to_host[i] == '$') {
       if (substitution_count > MAX_REGEX_SUBS) {
-        Error("Cannot have more than %d substitutions in mapping with host [%s]",
+        Warning("Cannot have more than %d substitutions in mapping with host [%s]",
               MAX_REGEX_SUBS, from_host_lower);
         goto lFail;
       }
       substitution_id = to_host[i + 1] - '0';
       if ((substitution_id < 0) || (substitution_id > n_captures)) {
-        Error("Substitution id [%c] has no corresponding capture pattern in regex [%s]",
+        Warning("Substitution id [%c] has no corresponding capture pattern in regex [%s]",
               to_host[i + 1], from_host_lower);
         goto lFail;
       }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2f9ccb0e/proxy/http/remap/UrlRewrite.h
----------------------------------------------------------------------
diff --git a/proxy/http/remap/UrlRewrite.h b/proxy/http/remap/UrlRewrite.h
index 1b10364..4ff1a38 100644
--- a/proxy/http/remap/UrlRewrite.h
+++ b/proxy/http/remap/UrlRewrite.h
@@ -71,12 +71,13 @@ class UrlRewrite
 {
 public:
   UrlRewrite(const char *file_var_in);
-   ~UrlRewrite();
+  ~UrlRewrite();
   int BuildTable();
   mapping_type Remap_redirect(HTTPHdr * request_header, URL *redirect_url, char **orig_url);
   bool ReverseMap(HTTPHdr *response_header);
   void SetReverseFlag(int flag);
   void Print();
+  bool is_valid() const { return _valid; };
 //  private:
 
   static const int MAX_REGEX_SUBS = 10;
@@ -190,6 +191,7 @@ public:
   int num_rules_forward_with_recv_port;
 
 private:
+  bool _valid;
   void _doRemap(UrlMappingContainer &mapping_container, URL *request_url);
   bool _mappingLookup(MappingsStore &mappings, URL *request_url, int request_port, const
char *request_host,
                       int request_host_len, UrlMappingContainer &mapping_container);


Mime
View raw message