You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ig...@apache.org on 2011/06/01 02:02:50 UTC
svn commit: r1129974 - in /trafficserver/traffic/branches/3.0.x: ./ CHANGES
STATUS proxy/http/remap/UrlMapping.cc proxy/http/remap/UrlRewrite.cc
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 *));