TS-3120: overlapping remap rank when using .include directives

As described in the docs, we use line number for remap rule rank:

  Once these rules are executed we pick the lowest line number as
  the match (which replicates first-match-wins).

However, when we use .include directives to include some other remap
config files, there will be overlapping and conflict with the line
numbers in each other file.

*Examples*

  remap.config
    .include remap1.config
    .include remap2.config

  remap1.config
    map /foo/ https://www.yahoo.com

  remap2.config
    map /foo/bar1 https://www.yahoo.com
    map /foo/bar2 https://www.yahoo.com

When parsing remap1.config, first entry in remap1.config is inserted
with rank 0, second with rank 1. Then parsing remap2.config, the
single entry is inserted with rank 0 again. So the entry in
remap2.config is overlapped with first entry in remap1.config and
takes precedence with second entry. This would confuse customers.

I use count in UrlRewrite::_addToStore for rank. It is used to count
the number of each type of map rules(map, reverse_map,
map_with_referer...). When we insert a new url_mapping, it is stored
in a separate MappingsStore. So there would be no conflict with
each type's count.


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

Branch: refs/heads/master
Commit: 50520e7fe5dea63ace1df4a03cdcbf0029bd820d
Parents: 067df58
Author: Feifei Cai <ff...@yahoo-inc.com>
Authored: Thu Oct 9 08:48:14 2014 -0700
Committer: James Peach <jpe...@apache.org>
Committed: Thu Oct 9 09:17:44 2014 -0700

----------------------------------------------------------------------
 doc/reference/configuration/remap.config.en.rst | 6 +++---
 proxy/http/remap/RemapConfig.cc                 | 2 +-
 proxy/http/remap/UrlMapping.h                   | 1 +
 proxy/http/remap/UrlRewrite.cc                  | 1 +
 4 files changed, 6 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/50520e7f/doc/reference/configuration/remap.config.en.rst
----------------------------------------------------------------------
diff --git a/doc/reference/configuration/remap.config.en.rst 
b/doc/reference/configuration/remap.config.en.rst
index c32fc80..46b5b79 100644
--- a/doc/reference/configuration/remap.config.en.rst
+++ b/doc/reference/configuration/remap.config.en.rst
@@ -113,9 +113,9 @@ Traffic Server recognizes three space-delimited fields: 
``type``,
 Precedence
 ==========
 
-Remap rules are not processed top-down, but based on an internal priority. Once
-these rules are executed we pick the lowest line number as the match (which
-replicates first-match-wins).
+Remap rules are not processed top-down, but based on an internal
+priority. Once these rules are executed we pick the first match
+based on configuration file parse order.
 
 1. ``map_with_recv_port`` and ```regex_map_with_recv_port```
 #. ``map`` and ``regex_map`` and ``reverse_map``

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/50520e7f/proxy/http/remap/RemapConfig.cc
----------------------------------------------------------------------
diff --git a/proxy/http/remap/RemapConfig.cc b/proxy/http/remap/RemapConfig.cc
index 398039f..70987e1 100644
--- a/proxy/http/remap/RemapConfig.cc
+++ b/proxy/http/remap/RemapConfig.cc
@@ -984,7 +984,7 @@ remap_parse_config_bti(const char * path, BUILD_TABLE_INFO 
* bti)
       goto MAP_ERROR;
     }
 
-    new_mapping = new url_mapping(cln);  // use line # for rank for now
+    new_mapping = new url_mapping();
 
     // apply filter rules if we have to
     if ((errStr = process_filter_opt(new_mapping, bti, errStrBuf, 
sizeof(errStrBuf))) != NULL) {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/50520e7f/proxy/http/remap/UrlMapping.h
----------------------------------------------------------------------
diff --git a/proxy/http/remap/UrlMapping.h b/proxy/http/remap/UrlMapping.h
index cd4b1e3..ad9c1de 100644
--- a/proxy/http/remap/UrlMapping.h
+++ b/proxy/http/remap/UrlMapping.h
@@ -117,6 +117,7 @@ public:
   LINK(url_mapping, link); // For use with the main Queue linked list holding 
all the mapping
 
   int getRank() const { return _rank; };
+  void setRank(int rank) { _rank = rank; };
 
 private:
   remap_plugin_info* _plugin_list[MAX_REMAP_PLUGIN_CHAIN];

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/50520e7f/proxy/http/remap/UrlRewrite.cc
----------------------------------------------------------------------
diff --git a/proxy/http/remap/UrlRewrite.cc b/proxy/http/remap/UrlRewrite.cc
index a9897e9..a3f547c 100644
--- a/proxy/http/remap/UrlRewrite.cc
+++ b/proxy/http/remap/UrlRewrite.cc
@@ -582,6 +582,7 @@ UrlRewrite::_addToStore(MappingsStore &store, url_mapping 
*new_mapping, RegexMap
     store.regex_list.enqueue(reg_map);
     retval = true;
   } else {
+    new_mapping->setRank(count); // Use the mapping rules number count for rank
     retval = TableInsert(store.hash_lookup, new_mapping, src_host);
   }
   if (retval) {

Reply via email to