Share only the evidence usage sharing accepts unless asked for all

pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/evidence.py

@@ -98,7 +98,8 @@ def filter(self) -> None:
         if self.evidence_filtered:
             return
 
-        for key in self.evidence:
+        # Loop over a copy of the keys, as keys are removed on the way.
+        for key in list(self.evidence):
             if not any([
                 flow_element.filter_evidence_key(key)
                 for flow_element

pipeline/fiftyone_pipeline_core/tests/test_core.py

@@ -58,6 +58,24 @@ def testEvidenceKeyFilter(self):
         nullEvidence = testPipeline.flowdata.evidence.get("header.other-evidence")
         self.assertTrue(nullEvidence == None)
 
+    # Test that evidence no element accepts is removed without an error.
+    # The JavaScript and set header elements accept every key, so they are
+    # left out.
+    def testEvidenceKeyFilterRemovesKeys(self):
+
+        pipeline = (PipelineBuilder({
+                "add_javascript_builder": False,
+                "use_setheader_properties": False}))\
+            .add(ExampleFlowElement1())\
+            .build()
+        flowdata = pipeline.create_flowdata()
+        flowdata.evidence.add("header.user-agent", "test")
+        flowdata.evidence.add("header.accept-encoding", "gzip")
+        flowdata.evidence.add("header.host", "example.com")
+        flowdata.process()
+        self.assertEqual(
+            flowdata.evidence.get_all(), {"header.user-agent": "test"})
+
     # Test Getter methods
     def testGet(self):
 

pipeline/fiftyone_pipeline_engines_fiftyone/examples/usagesharing/51d.json

@@ -9,8 +9,10 @@
               "interval": 1200,
               "requested_package_size": 50,
               "cookie": "",
+              "track_session": false,
               "query_whitelist": [],
-              "header_blacklist": []
+              "header_blacklist": [],
+              "share_all": false
           }
       }
       ]

pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py

@@ -31,7 +31,8 @@
 import requests
 from fiftyone_pipeline_engines.engine import Engine
 
-from .share_usage_evidencekeyfilter import ShareUsageEvidenceKeyFilter
+from .share_usage_evidencekeyfilter import ShareUsageEvidenceKeyFilter, \
+    ShareUsageTrackerEvidenceKeyFilter
 from .share_usage_tracker import ShareUsageTracker
 
 # The maximum length of a piece of evidence's value which can be
@@ -54,7 +55,9 @@ def __init__(
         header_blacklist = [],
         share_percentage = 1,
         endpoint = "https://devices-v4.51degrees.com/new.ashx",
-        request_timeout=REQUEST_TIMEOUT):
+        request_timeout=REQUEST_TIMEOUT,
+        share_all = False,
+        track_session = False):
         """!
         Constructor for ShareUsage element
 
@@ -70,22 +73,32 @@ def __init__(
         reduce the overhead of sending HTTP messages.
         For example, the 51Degrees cloud service uses a value of 2500.
         @type cookie: string
-        @param cookie: If a cookie is being used to identify user 
-        sessions, it can be specified here in order to reduce the 
-        sharing of duplicated data.
+        @param cookie: The name of the cookie that holds the session id.
+        It has no effect unless track_session is True. The cookie itself
+        is not shared unless share_all is True.
+        @type track_session: bool
+        @param track_session: If True, the cookie named by cookie is used
+        to tell user sessions apart, so evidence already shared is shared
+        again when it comes from a new session. False by default. It
+        needs cookie to be set, and has no effect when share_all is True,
+        because every cookie is then shared and used to find repeats.
         @type query_whitelist: list
-        @param query_whitelist: By default query string and HTTP form 
+        @param query_whitelist: By default query string and HTTP form
         parameters are not shared unless prefixed with '51D_'.
-        If you need to share query string parameters, a list can be 
-        specified here.
-        @type query_blacklist: list
-        @param query_blacklist: By default, all HTTP headers 
-        (except a few, such as Cookies) are shared. 
-        Individual headers can be excluded from sharing by adding them 
+        If you need to share query string parameters, a list can be
+        specified here. None shares every parameter.
+        @type header_blacklist: list
+        @param header_blacklist: By default, all HTTP headers
+        except Cookie are shared.
+        Individual headers can be excluded from sharing by adding them
         to this list.
         @type share_percentage : float
         @param share_percentage: approximate proportion of requests to
         be shared. 1 = 100%, 0.5 = 50%, etc..
+        @type share_all: bool
+        @param share_all: If True, all evidence is shared apart from the
+        email address, the salt used for hashed email identifiers, and the
+        request's query string and path. False by default.
         """
 
         super(ShareUsage, self).__init__()
@@ -102,6 +115,25 @@ def __init__(
         self.share_percentage = share_percentage
         self.endpoint = endpoint
         self.request_timeout = request_timeout
+        self.share_all = share_all
+        self.track_session = track_session
+
+        # The session id and sequence number are kept in evidence, as the
+        # usage data carries them separately.
+        included = None if query_whitelist is None else \
+            list(query_whitelist) + ["session-id", "sequence"]
+
+        # When sessions are tracked, the pipeline keeps the session cookie
+        # for the tracker. The data sent leaves it out unless all evidence
+        # is shared.
+        self.evidence_key_filter = ShareUsageEvidenceKeyFilter(
+            cookie, included, header_blacklist, track_session, share_all)
+        self.data_evidence_key_filter = ShareUsageEvidenceKeyFilter(
+            cookie, included, header_blacklist, False, share_all)
+        self.tracker_evidence_key_filter = \
+            ShareUsageTrackerEvidenceKeyFilter(
+                cookie, included, header_blacklist, track_session,
+                share_all)
 
         # Add the share usage tracker which detects when
         # to send up sharing data
@@ -188,11 +220,7 @@ def get_evidence_key_filter(self):
 
         """
 
-        return ShareUsageEvidenceKeyFilter(
-            cookie = self.cookie,
-            query_whitelist = self.query_whitelist,
-            header_blacklist = self.header_blacklist
-        )
+        return self.evidence_key_filter
 
     def share_send_usage(self):
         """!
@@ -291,7 +319,7 @@ def process_internal(self, flow_data):
         """
 
         if random.uniform(0, 1) <= self.share_percentage:
-            cachekey = self.get_evidence_key_filter().filter_evidence(flow_data.evidence.get_all())
+            cachekey = self.tracker_evidence_key_filter.filter_evidence(flow_data.evidence.get_all())
             cachekey = json.dumps(cachekey)
             share = self.tracker.track(cachekey) 
             if share:
@@ -327,7 +355,7 @@ def get_data_from_evidence(self, flow_data):
                         f"The value '{value}' could not be parsed to an integer.")
             else:
                 # Check if we can send this piece of evidence
-                if self.get_evidence_key_filter().filter_evidence_key(key):
+                if self.data_evidence_key_filter.filter_evidence_key(key):
                     data.try_add(key, value)
 
         return data
@@ -394,7 +422,7 @@ def try_add(self, key, value):
         category = ""
         field = key
 
-        first_separator = key.index(".")
+        first_separator = key.find(".")
         if first_separator > 0:
             category = key[0:first_separator]
             field = key[first_separator + 1:len(key)]

pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage_evidencekeyfilter.py

@@ -22,41 +22,64 @@
 
 from fiftyone_pipeline_core.basiclist_evidence_keyfilter import BasicListEvidenceKeyFilter
 
+# Cookies, session values and query string parameters whose names start with
+# this are the ones 51Degrees sets, and are shared.
+FIFTYONE_PREFIX = "51d_"
+
+# The Cookie header carries every cookie, including the ones the cookie rule
+# leaves out, so it is not shared unless all evidence is.
+BLOCKED_HEADERS = ("cookie",)
+
+# Evidence that is never shared, whatever the settings, matched as the whole
+# key or after a '.'. 'id.email' and 'id.salt' are a raw email address and the
+# salt used to make hashed email identifiers. 'request-query' is the whole query
+# string, which would undo the rules for its parameters. 'request-path' would
+# make each page a visitor opens look like a new session to the tracker.
+NEVER_SHARED_KEYS = ("id.email", "id.salt", "request-query", "request-path")
+
 class ShareUsageEvidenceKeyFilter(BasicListEvidenceKeyFilter):
     """
-    The ShareUsageEvidenceKeyFilter filters out all evidence 
-    not needed by the 51Degrees ShareUsage service.
-    It allows for a specific whitelist of query strings,
-    a blacklist of headers and a specific cookie used for 
-    session information
+    The ShareUsageEvidenceKeyFilter decides which evidence the 51Degrees
+    ShareUsage service is sent. Headers are shared unless they are blocked,
+    cookies and session values only when their names start with 51D_, and
+    query string and form parameters only when their names start with 51D_ or
+    are on the list to share. Any other evidence is shared.
     """
 
-    def __init__(self, cookie = None, query_whitelist = [], header_blacklist = []):
+    def __init__(
+            self,
+            cookie = None,
+            query_whitelist = (),
+            header_blacklist = (),
+            include_session = False,
+            share_all = False):
         """!
         Constructor for ShareUsageEvidenceKeyFilter
         @type cookie: string
-        @param cookie: which cookie is used to track evidence
+        @param cookie: the name of the cookie that identifies the session
         @type query_whitelist: list
-        @param query_whitelist: list of query string whitelist evidence to keep
-        @type query_blacklist: list
-        @param query_blacklist: list of header evidence to exclude
-        @type 
-        @param
+        @param query_whitelist: query string and form parameters to share as
+        well as those starting 51D_, or None to share every parameter
+        @type header_blacklist: list
+        @param header_blacklist: headers not to share
+        @type include_session: bool
+        @param include_session: whether the session cookie is included
+        @type share_all: bool
+        @param share_all: whether all evidence is shared, apart from the
+        evidence that is never shared
         """
-        self.query_whitelist = query_whitelist
+        self.cookie = cookie.lower() if cookie else None
 
-        self.header_blacklist = header_blacklist
+        self.query_whitelist = None if query_whitelist is None else \
+            [parameter.lower() for parameter in query_whitelist]
 
-        self.cookie = cookie
+        self.header_blacklist = \
+            [header.lower() for header in header_blacklist or ()]
 
-        if cookie is None and \
-            (query_whitelist is None or len(query_whitelist) == 0) and \
-            (header_blacklist is None or len(header_blacklist) == 0):
-            self.share_all = True
-        else:
-            self.share_all = False
+        self.include_session = include_session
+
+        self.share_all = share_all
 
-
     def filter_evidence_key(self, key):
 
         """!
@@ -69,42 +92,58 @@ def filter_evidence_key(self, key):
         @return: Is this key in the filter's keys list?
 
         """
-        #  get prefix and key of evidence
-        key_parts = key.lower().split(".")
-
-        prefix = key_parts[0]
-        suffix = key_parts[1]
-
-        result = self.share_all
-
-        if not self.share_all:
-            if prefix == "header":
-                # Add the header to the list if the header name does not
-                # appear in the list of blocked headers
-                result = suffix not in self.header_blacklist
-            elif prefix == "cookie":
-                # Only add cookies that start with the 51Degrees cookie
-                # prefix
-                result = suffix.startswith("51d_") or \
-                    (self.include_session and suffix == self.cookie)
-            elif prefix == "session":
-                # Only session values that start with the 51Degrees 
-                # cookie prefix
-                result = suffix.startswith("51d_")
-            elif prefix == "query":
-                # If no query string parameter filter was specified 
-                # then share all of them.
-                # Otherwise, only include query string parameters that 
-                # start with 51d_ or that have been specified in 
-                # the constructor.
-                result = self.query_whitelist is None or \
-                    len(self.query_whitelist) == 0 or \
-                    suffix.startswith("51d_") or \
-                    suffix in self.query_whitelist
-            else:
-                # Add anything that is not a cookie, header, session
-                # variable or query parameter
-                result = True
-
-        return result
-
+        key = key.lower()
+
+        if any(key == never_shared or key.endswith("." + never_shared)
+                for never_shared in NEVER_SHARED_KEYS):
+            return False
+
+        if self.share_all:
+            return True
+
+        # The prefix is everything before the first dot and the field
+        # everything after it, so a field can itself contain dots.
+        prefix, separator, field = key.partition(".")
+
+        if not separator:
+            return True
+        elif prefix == "header":
+            # Add the header to the list if the header name does not
+            # appear in the list of blocked headers
+            return field not in self.header_blacklist and \
+                field not in BLOCKED_HEADERS
+        elif prefix == "cookie":
+            # Only add cookies that start with the 51Degrees cookie
+            # prefix
+            return field.startswith(FIFTYONE_PREFIX) or \
+                (self.include_session and field == self.cookie)
+        elif prefix == "session":
+            # Only session values that start with the 51Degrees
+            # cookie prefix
+            return field.startswith(FIFTYONE_PREFIX)
+        elif prefix == "query":
+            # If no query string parameter list was given then share all
+            # of them. Otherwise, only include query string parameters
+            # that start with 51d_ or that are in the list.
+            return self.query_whitelist is None or \
+                field.startswith(FIFTYONE_PREFIX) or \
+                field in self.query_whitelist
+        else:
+            # Add anything that is not a cookie, header, session
+            # variable or query parameter
+            return True
+
+class ShareUsageTrackerEvidenceKeyFilter(ShareUsageEvidenceKeyFilter):
+    """
+    The evidence the ShareUsageTracker compares to decide whether a request
+    repeats one already shared. The session id and sequence number are left
+    out, because they make every request different.
+    """
+
+    def filter_evidence_key(self, key):
+
+        if key.lower() in ("query.session-id", "query.sequence"):
+            return False
+
+        return super(ShareUsageTrackerEvidenceKeyFilter, self)\
+            .filter_evidence_key(key)