trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From iga...@apache.org
Subject svn commit: r1129974 - in /trafficserver/traffic/branches/3.0.x: ./ CHANGES STATUS proxy/http/remap/UrlMapping.cc proxy/http/remap/UrlRewrite.cc
Date Wed, 01 Jun 2011 00:02:50 GMT
Author: igalic
Date: Wed Jun  1 00:02:50 2011
New Revision: 1129974

URL: http://svn.apache.org/viewvc?rev=1129974&view=rev
Log:
Merge changes from r1129268 from trunk
This fixes [TS-798], preventing us to add add broken remap rules when we encounter parse errors
of remap.config.
Author: zwoop
Reviewer: ericb, bcall

Modified:
    trafficserver/traffic/branches/3.0.x/   (props changed)
    trafficserver/traffic/branches/3.0.x/CHANGES
    trafficserver/traffic/branches/3.0.x/STATUS
    trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlMapping.cc
    trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlRewrite.cc

Propchange: trafficserver/traffic/branches/3.0.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Jun  1 00:02:50 2011
@@ -1,3 +1,4 @@
 /incubator/trafficserver/traffic/branches/dev:891823-915885
 /trafficserver/traffic/branches/ts-291:965529-991993
 /trafficserver/traffic/branches/wccp:1021790-1040544
+/trafficserver/traffic/trunk:1129268

Modified: trafficserver/traffic/branches/3.0.x/CHANGES
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/3.0.x/CHANGES?rev=1129974&r1=1129973&r2=1129974&view=diff
==============================================================================
--- trafficserver/traffic/branches/3.0.x/CHANGES (original)
+++ trafficserver/traffic/branches/3.0.x/CHANGES Wed Jun  1 00:02:50 2011
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 
 Changes with Apache Traffic Server 3.0.0
+  *) [TS-798] We add broken remap rules when we encounter parse errors of
+   remap.config.
+
   *) [TS-810] Typo in switch statement + slight improvement.
 
   *) [TS-809] ts.h broken when compiling C plugins.

Modified: trafficserver/traffic/branches/3.0.x/STATUS
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/3.0.x/STATUS?rev=1129974&r1=1129973&r2=1129974&view=diff
==============================================================================
--- trafficserver/traffic/branches/3.0.x/STATUS (original)
+++ trafficserver/traffic/branches/3.0.x/STATUS Wed Jun  1 00:02:50 2011
@@ -39,16 +39,6 @@ A list of all bugs open for the next v3.
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  * remap: If we encounter parsing errors of remap.config, i.e. a broken
-           configuration, we end up adding it to the remap containers
-           anyways. This is not a broken configuration, it's also a
-           configuration that is free()'d back to the heap. This is bad.
-    Trunk patch: http://svn.apache.org/viewvc?rev=1129268&view=rev
-                 http://svn.apache.org/viewvc?rev=1129704&view=rev
-    Jira: https://issues.apache.org/jira/browse/TS-798
-    +1: zwoop
-    +1: ericb
-    +1: bcall
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

Modified: trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlMapping.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlMapping.cc?rev=1129974&r1=1129973&r2=1129974&view=diff
==============================================================================
--- trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlMapping.cc (original)
+++ trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlMapping.cc Wed Jun  1 00:02:50
2011
@@ -48,7 +48,7 @@ url_mapping::add_plugin(remap_plugin_inf
 
   _plugin_list[_plugin_count] = i;
   _instance_data[_plugin_count] = ih;
-  _plugin_count++;
+  ++_plugin_count;
 
   return true;
 }

Modified: trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlRewrite.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlRewrite.cc?rev=1129974&r1=1129973&r2=1129974&view=diff
==============================================================================
--- trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlRewrite.cc (original)
+++ trafficserver/traffic/branches/3.0.x/proxy/http/remap/UrlRewrite.cc Wed Jun  1 00:02:50
2011
@@ -1052,7 +1052,7 @@ UrlRewrite::BuildTable()
   char *fromHost_lower = NULL;
   char *fromHost_lower_ptr = NULL;
   char fromHost_lower_buf[1024];
-  url_mapping *new_mapping;
+  url_mapping *new_mapping = NULL;
   mapping_type maptype;
   referer_info *ri;
   int origLength;
@@ -1099,23 +1099,24 @@ UrlRewrite::BuildTable()
 
     // Strip leading whitespace
     while (*cur_line && isascii(*cur_line) && isspace(*cur_line))
-      cur_line++;
+      ++cur_line;
 
     if ((cur_line_size = strlen((char *) cur_line)) <= 0) {
       cur_line = tokLine(NULL, &tok_state);
-      cln++;
+      ++cln;
       continue;
     }
+
     // Strip trailing whitespace
     cur_line_tmp = cur_line + cur_line_size - 1;
     while (cur_line_tmp != cur_line && isascii(*cur_line_tmp) && isspace(*cur_line_tmp))
{
       *cur_line_tmp = '\0';
-      cur_line_tmp--;
+      --cur_line_tmp;
     }
 
     if ((cur_line_size = strlen((char *) cur_line)) <= 0 || *cur_line == '#' || *cur_line
== '\0') {
       cur_line = tokLine(NULL, &tok_state);
-      cln++;
+      ++cln;
       continue;
     }
 
@@ -1135,11 +1136,8 @@ UrlRewrite::BuildTable()
     // Initial verification for number of arguments
     if (bti.paramc<1 || (bti.paramc < 3 && bti.paramv[0][0] != '.') || bti.paramc>
BUILD_TABLE_MAX_ARGS) {
       snprintf(errBuf, sizeof(errBuf), "%s Malformed line %d in file %s", modulePrefix, cln
+ 1, config_file_path);
-      Debug("url_rewrite", "[BuildTable] %s", errBuf);
-      SignalError(errBuf, alarm_already);
-      cur_line = tokLine(NULL, &tok_state);
-      cln++;
-      continue;
+      errStr = errStrBuf;
+      goto MAP_ERROR;
     }
     // just check all major flags/optional arguments
     bti.remap_optflg = check_remap_option(bti.argv, bti.argc);
@@ -1149,11 +1147,12 @@ UrlRewrite::BuildTable()
     if (bti.paramv[0][0] == '.') {
       if ((errStr = parse_directive(&bti, errStrBuf, sizeof(errStrBuf))) != NULL) {
         snprintf(errBuf, sizeof(errBuf) - 1, "%s Error on line %d - %s", modulePrefix, cln
+ 1, errStr);
-        Debug("url_rewrite", "[BuildTable] %s", errBuf);
-        SignalError(errBuf, alarm_already);
+        errStr = errStrBuf;
+        goto MAP_ERROR;
       }
+      // We skip the rest of the parsing here.
       cur_line = tokLine(NULL, &tok_state);
-      cln++;
+      ++cln;
       continue;
     }
 
@@ -1179,11 +1178,8 @@ UrlRewrite::BuildTable()
       maptype = FORWARD_MAP_REFERER;
     } else {
       snprintf(errBuf, sizeof(errBuf) - 1, "%s Unknown mapping type at line %d", modulePrefix,
cln + 1);
-      Debug("url_rewrite", "[BuildTable] - %s", errBuf);
-      SignalError(errBuf, alarm_already);
-      cur_line = tokLine(NULL, &tok_state);
-      cln++;
-      continue;
+      errStr = errStrBuf;
+      goto MAP_ERROR;
     }
 
     new_mapping = NEW(new url_mapping(cln));  // use line # for rank for now
@@ -1360,40 +1356,6 @@ UrlRewrite::BuildTable()
       Debug("url_rewrite_regex", "Configured regex rule for host [%s]", fromHost_lower);
     }
 
-    add_result = false;
-
-    switch (maptype) {
-    case FORWARD_MAP:
-    case FORWARD_MAP_REFERER:
-      if ((add_result = _addToStore(forward_mappings, new_mapping, reg_map, fromHost_lower,
-                                    is_cur_mapping_regex, num_rules_forward)) == true) {
-        // @todo: is this applicable to regex mapping too?
-        SetHomePageRedirectFlag(new_mapping, new_mapping->toUrl);
-      }
-      break;
-    case REVERSE_MAP:
-      add_result = _addToStore(reverse_mappings, new_mapping, reg_map, fromHost_lower,
-                               is_cur_mapping_regex, num_rules_reverse);
-      new_mapping->homePageRedirect = false;
-      break;
-    case PERMANENT_REDIRECT:
-      add_result = _addToStore(permanent_redirects, new_mapping, reg_map, fromHost_lower,
-                               is_cur_mapping_regex, num_rules_redirect_permanent);
-      break;
-    case TEMPORARY_REDIRECT:
-      add_result = _addToStore(temporary_redirects, new_mapping, reg_map, fromHost_lower,
-                               is_cur_mapping_regex, num_rules_redirect_temporary);
-      break;
-    default:
-      // 'default' required to avoid compiler warning; unsupported map
-      // type would have been dealt with much before this
-      break;
-    };
-
-    if (!add_result) {
-      goto MAP_WARNING;
-    }
-
     // If a TS receives a request on a port which is set to tunnel mode
     // (ie, blind forwarding) and a client connects directly to the TS,
     // then the TS will use its IPv4 address and remap rules given
@@ -1446,20 +1408,14 @@ UrlRewrite::BuildTable()
         // this loads the first plugin
         if (load_remap_plugin(bti.argv, bti.argc, new_mapping, errStrBuf, sizeof(errStrBuf),
0, &plugin_found_at)) {
           Debug("remap_plugin", "Remap plugin load error - %s", errStrBuf[0] ? errStrBuf
: "Unknown error");
-          Debug("url_rewrite", "Remap plugin load error - %s", errStrBuf[0] ? errStrBuf :
"Unknown error");
-          ink_error(errStrBuf);
           errStr = errStrBuf;
           goto MAP_ERROR;
         }
         //this loads any subsequent plugins (if present)
         while (plugin_found_at) {
           jump_to_argc += plugin_found_at;
-          int ret = load_remap_plugin(bti.argv, bti.argc, new_mapping, errStrBuf, sizeof(errStrBuf),
jump_to_argc,
-                                      &plugin_found_at);
-          if (ret) {
+          if (load_remap_plugin(bti.argv, bti.argc, new_mapping, errStrBuf, sizeof(errStrBuf),
jump_to_argc, &plugin_found_at)) {
             Debug("remap_plugin", "Remap plugin load error - %s", errStrBuf[0] ? errStrBuf
: "Unknown error");
-            Debug("url_rewrite", "Remap plugin load error - %s", errStrBuf[0] ? errStrBuf
: "Unknown error");
-            ink_error(errStrBuf);
             errStr = errStrBuf;
             goto MAP_ERROR;
           }
@@ -1467,31 +1423,60 @@ UrlRewrite::BuildTable()
       }
     }
 
+    // Now add the mapping to appropriate container
+    add_result = false;
+    switch (maptype) {
+    case FORWARD_MAP:
+    case FORWARD_MAP_REFERER:
+      if ((add_result = _addToStore(forward_mappings, new_mapping, reg_map, fromHost_lower,
+                                    is_cur_mapping_regex, num_rules_forward)) == true) {
+        // @todo: is this applicable to regex mapping too?
+        SetHomePageRedirectFlag(new_mapping, new_mapping->toUrl);
+      }
+      break;
+    case REVERSE_MAP:
+      add_result = _addToStore(reverse_mappings, new_mapping, reg_map, fromHost_lower,
+                               is_cur_mapping_regex, num_rules_reverse);
+      new_mapping->homePageRedirect = false;
+      break;
+    case PERMANENT_REDIRECT:
+      add_result = _addToStore(permanent_redirects, new_mapping, reg_map, fromHost_lower,
+                               is_cur_mapping_regex, num_rules_redirect_permanent);
+      break;
+    case TEMPORARY_REDIRECT:
+      add_result = _addToStore(temporary_redirects, new_mapping, reg_map, fromHost_lower,
+                               is_cur_mapping_regex, num_rules_redirect_temporary);
+      break;
+    default:
+      // 'default' required to avoid compiler warning; unsupported map
+      // type would have been dealt with much before this
+      break;
+    }
+    if (!add_result)
+      goto MAP_WARNING;
+
     if (unlikely(fromHost_lower_ptr))
       fromHost_lower_ptr = (char *) xfree_null(fromHost_lower_ptr);
 
     cur_line = tokLine(NULL, &tok_state);
-    cln++;
-    continue;
-
-  MAP_WARNING:
-    Error("Could not add rule at line #%d; Continuing with remaining lines", cln + 1);
-    if (new_mapping) {
-      delete new_mapping;
-      new_mapping = NULL;
-    }
+    ++cln;
     continue;
 
+    // Deal with error / warning scenarios
   MAP_ERROR:
     snprintf(errBuf, sizeof(errBuf), "%s %s at line %d", modulePrefix, errStr, cln + 1);
     SignalError(errBuf, alarm_already);
+    ink_error(errBuf);
+
+  MAP_WARNING:
+    Warning("Could not add rule at line #%d; Continuing with remaining lines", cln + 1);
     if (new_mapping) {
       delete new_mapping;
       new_mapping = NULL;
     }
     cur_line = tokLine(NULL, &tok_state);
-    cln++;
-    return 1;
+    ++cln;
+    continue;
   }                             /* end of while(cur_line != NULL) */
 
   clear_xstr_array(bti.paramv, sizeof(bti.paramv) / sizeof(char *));



Mime
View raw message