You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2011/10/11 20:26:52 UTC

svn commit: r1182007 - in /trafficserver/traffic/trunk: CHANGES proxy/ReverseProxy.cc proxy/http/remap/UrlRewrite.cc proxy/http/remap/UrlRewrite.h

Author: zwoop
Date: Tue Oct 11 18:26:52 2011
New Revision: 1182007

URL: http://svn.apache.org/viewvc?rev=1182007&view=rev
Log:
TS-948 Do not reload (or load) a broken remap.config

Modified:
    trafficserver/traffic/trunk/CHANGES
    trafficserver/traffic/trunk/proxy/ReverseProxy.cc
    trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.cc
    trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.h

Modified: trafficserver/traffic/trunk/CHANGES
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/CHANGES?rev=1182007&r1=1182006&r2=1182007&view=diff
==============================================================================
--- trafficserver/traffic/trunk/CHANGES (original)
+++ trafficserver/traffic/trunk/CHANGES Tue Oct 11 18:26:52 2011
@@ -1,5 +1,7 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 3.1.1
+  *) [TS-948] Don't reload or load a broken remap.config.
+
   *) [TS-824] Range requests that result in cache refresh give 200 status
    response with full contents. Review and suggestions for improvements
    by Charlie Gero.

Modified: trafficserver/traffic/trunk/proxy/ReverseProxy.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/ReverseProxy.cc?rev=1182007&r1=1182006&r2=1182007&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/ReverseProxy.cc (original)
+++ trafficserver/traffic/trunk/proxy/ReverseProxy.cc Tue Oct 11 18:26:52 2011
@@ -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

Modified: trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.cc?rev=1182007&r1=1182006&r2=1182007&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.cc (original)
+++ trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.cc Tue Oct 11 18:26:52 2011
@@ -476,7 +476,7 @@ UrlRewrite::UrlRewrite(const char *file_
  : 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 =
@@ -525,15 +525,16 @@ UrlRewrite::UrlRewrite(const char *file_
   ink_strlcat(config_file_path, config_file, sizeof(config_file_path));
   ats_free(config_file);
 
-  if (this->BuildTable() != 0) {
+  if (0 == this->BuildTable()) {
+    _valid = true;
+    pcre_malloc = &ats_malloc;
+    pcre_free = &ats_free;
+
+    if (is_debug_tag_set("url_rewrite"))
+      Print();
+  } else {
     Warning("something failed during BuildTable() -- check your remap plugins!");
   }
-
-  pcre_malloc = &ats_malloc;
-  pcre_free = &ats_free;
-
-  if (is_debug_tag_set("url_rewrite"))
-    Print();
 }
 
 
@@ -548,6 +549,7 @@ UrlRewrite::~UrlRewrite()
   DestroyStore(permanent_redirects);
   DestroyStore(temporary_redirects);
   DestroyStore(forward_mappings_with_recv_port);
+  _valid = false;
 }
 
 /** Sets the reverse proxy flag. */
@@ -1085,7 +1087,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;
   }
 
@@ -1487,7 +1489,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 *));

Modified: trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.h?rev=1182007&r1=1182006&r2=1182007&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.h (original)
+++ trafficserver/traffic/trunk/proxy/http/remap/UrlRewrite.h Tue Oct 11 18:26:52 2011
@@ -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);