Share every query value by default, and no cookie but 51D_ ones even when all Evidence is shared

pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsage.java

@@ -33,6 +33,7 @@
 import java.util.Set;
 
 import static fiftyone.pipeline.core.Constants.*;
+import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS;
 
 /**
  * This filter is used by the {@link ShareUsageElement} to decide which of the
@@ -44,12 +45,13 @@
  * 2) A cookie that starts with {@link Constants#FIFTYONE_COOKIE_PREFIX} or is
  *     the session cookie (if configured in the constructor).
  * 3) A session value that starts with {@link Constants#FIFTYONE_COOKIE_PREFIX}.
- * 4) A query string parameter that starts with
+ * 4) A query string parameter, when all query string parameters are shared,
+ *     which is the default. Otherwise one that starts with
  *     {@link Constants#FIFTYONE_COOKIE_PREFIX} or has been configured to be
  *     shared using the constructor parameter.
  * 5) Not a header, cookie, session value or query string parameter.
- * When all Evidence is shared, everything is included. When all query string
- * parameters are shared, every query string parameter is included.
+ * When all Evidence is shared, everything is included but cookies, which
+ * follow rule 2 whatever the configuration.
  * The prefix of a key is everything before its first '.', and the field
  * everything after it. An email address, the salt for hashed email
  * identifiers, the query string, the path and the Cookie header are never
@@ -112,12 +114,14 @@ public class EvidenceKeyFilterShareUsage implements EvidenceKeyFilter {
     };
 
     /**
-     * Constructor.
+     * Constructor. All Evidence is not shared, and all query string
+     * parameters are, as are the defaults.
      * @param blockedHttpHeaders a list of the names of the HTTP headers that
      *                           share usage should not send to 51Degrees
      * @param includedQueryStringParams a list of the names of query string
      *                                  parameters that share usage should send
-     *                                  to 51Degrees
+     *                                  to 51Degrees when not all of them are
+     *                                  shared
      * @param includeSession if true then the session cookie will be included in
      *                       the filter
      * @param sessionCookieName the name of the cookie that contains the session
@@ -134,7 +138,7 @@ public EvidenceKeyFilterShareUsage(
             includeSession,
             sessionCookieName,
             false,
-            false);
+            SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS);
     }
 
     /**
@@ -143,13 +147,15 @@ public EvidenceKeyFilterShareUsage(
      *                           share usage should not send to 51Degrees
      * @param includedQueryStringParams a list of the names of query string
      *                                  parameters that share usage should send
-     *                                  to 51Degrees
+     *                                  to 51Degrees when not all of them are
+     *                                  shared
      * @param includeSession if true then the session cookie will be included in
      *                       the filter
      * @param sessionCookieName the name of the cookie that contains the session
      *                          id
      * @param shareAllEvidence if true then all Evidence is included, apart from
-     *                         the keys that are never shared
+     *                         the keys that are never shared and the cookies
+     *                         other than the 51Degrees ones
      * @param shareAllQueryStringParameters if true then all query string
      *                                      parameters are included
      */
@@ -197,12 +203,22 @@ public boolean include(String key) {
             }
         }
 
+        int separator = lowerKey.indexOf(EVIDENCE_SEPERATOR);
+
+        // Only the 51Degrees cookies are shared, even when all Evidence is.
+        // The session cookie is included only for the tracker to compare.
+        if (separator > 0 &&
+            EVIDENCE_COOKIE_PREFIX.equals(lowerKey.substring(0, separator))) {
+            String field = lowerKey.substring(separator + 1);
+            return field.startsWith(Constants.FIFTYONE_COOKIE_PREFIX) ||
+                (includeSession && field.equals(sessionCookieName));
+        }
+
         if (shareAllEvidence) {
             return true;
         }
 
         boolean result;
-        int separator = lowerKey.indexOf(EVIDENCE_SEPERATOR);
         if (separator > 0) {
             String prefix = lowerKey.substring(0, separator);
             String field = lowerKey.substring(separator + 1);
@@ -212,12 +228,6 @@ public boolean include(String key) {
                     // appear in the list of blocked headers.
                     result = blockedHttpHeaders.contains(field) == false;
                     break;
-                case EVIDENCE_COOKIE_PREFIX:
-                    // Only add cookies that start with the 51Degrees cookie
-                    // prefix.
-                    result = field.startsWith(Constants.FIFTYONE_COOKIE_PREFIX) ||
-                        (includeSession && field.equals(sessionCookieName));
-                    break;
                 case EVIDENCE_SESSION_PREFIX:
                     // Only add session values that start with the 51Degrees cookie
                     // prefix.

pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/Constants.java

@@ -112,6 +112,13 @@ public class Constants {
      */
     static final String SHARE_USAGE_DEFAULT_URL = "https://devices-v4.51degrees.com/new.ashx";
 
+    /**
+     * Every query string and HTTP form parameter is shared by default, not
+     * only the 51Degrees ones and those named.
+     */
+    public static final boolean
+        SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS = true;
+
     /**
      * Constant Added to blocked HTTP Headers so that the cookie header is
      * blocked by default.

pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBase.java

@@ -434,7 +434,7 @@ protected ShareUsageBase(
             Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MAX_DELAY_SECONDS_DEFAULT,
             Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT,
             false,
-            false);
+            Constants.SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS);
     }
 
     /**

pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageBuilderBase.java

@@ -65,7 +65,8 @@ public abstract class ShareUsageBuilderBase<T extends ShareUsageBase> {
     protected double maxRecoveryDelaySeconds = Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MAX_DELAY_SECONDS_DEFAULT;
     protected double recoveryMultiplier = Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT;
     protected boolean shareAllEvidence;
-    protected boolean shareAllQueryStringParameters;
+    protected boolean shareAllQueryStringParameters =
+        Constants.SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS;
 
     /**
      * Constructor
@@ -154,13 +155,14 @@ public ShareUsageBuilderBase<T> setIncludedQueryStringParameter(
      * If set to true, every query string and HTTP form parameter is shared,
      * rather than only those that start with 51D_ or are named in the
      * included parameters. The whole query string and the parameters that are
-     * never shared are still not shared.
+     * never shared are still not shared. Set it to false to share only the
+     * 51D_ parameters and those named.
      * <p>
-     * Default value is false
+     * Default value is true
      * @param shareAll true to share all query string parameters
      * @return this builder
      */
-    @DefaultValue(booleanValue = false)
+    @DefaultValue(booleanValue = true)
     public ShareUsageBuilderBase<T> setShareAllQueryStringParameters(
         boolean shareAll) {
         this.shareAllQueryStringParameters = shareAll;
@@ -177,10 +179,11 @@ public boolean isShareAllQueryStringParameters() {
 
     /**
      * If set to true, all Evidence is shared, which overrides the blocked
-     * HTTP headers and the rules for cookies, session values and query string
+     * HTTP headers and the rules for session values and query string
      * parameters. The keys that are never shared (an email address, the salt
      * for hashed email identifiers, the whole query string, the path and the
-     * Cookie header) are still not shared.
+     * Cookie header) are still not shared, and nor is any cookie whose name
+     * does not start with 51D_.
      * <p>
      * Default value is false
      * @param shareAll true to share all Evidence

pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElement.java

@@ -43,6 +43,7 @@
 import java.util.zip.GZIPOutputStream;
 
 import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_DEFAULT_HTTP_POST_TIMEOUT;
+import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS;
 import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_INITIAL_DELAY_SECONDS_DEFAULT;
 import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MAX_DELAY_SECONDS_DEFAULT;
 import static fiftyone.pipeline.engines.fiftyone.flowelements.Constants.SHARE_USAGE_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT;
@@ -332,7 +333,7 @@ public class ShareUsageElement extends ShareUsageBase {
                 SHARE_USAGE_EXPONENTIAL_BACKOFF_MAX_DELAY_SECONDS_DEFAULT,
                 SHARE_USAGE_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT,
                 false,
-                false);
+                SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS);
     }
 
     /**

pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/data/EvidenceKeyFilterShareUsageTests.java

@@ -56,6 +56,35 @@ private static EvidenceKeyFilterShareUsage defaults() {
             false);
     }
 
+    private static EvidenceKeyFilterShareUsage filter(
+        boolean includeSession,
+        boolean shareAllEvidence,
+        boolean shareAllQueryStringParameters) {
+        return new EvidenceKeyFilterShareUsage(
+            Collections.<String>emptyList(),
+            Collections.<String>emptyList(),
+            includeSession,
+            DEFAULT_SESSION_COOKIE_NAME,
+            shareAllEvidence,
+            shareAllQueryStringParameters);
+    }
+
+    /**
+     * Shares only the 51D_ query string parameters and those named, as when
+     * share all query string parameters is turned off.
+     */
+    private static EvidenceKeyFilterShareUsage namedParametersOnly(
+        List<String> blockedHttpHeaders,
+        List<String> includedQueryStringParams) {
+        return new EvidenceKeyFilterShareUsage(
+            blockedHttpHeaders,
+            includedQueryStringParams,
+            false,
+            DEFAULT_SESSION_COOKIE_NAME,
+            false,
+            false);
+    }
+
     /**
      * By default headers other than Cookie are shared, cookies, session
      * values and query string parameters only when their names start with
@@ -73,6 +102,11 @@ public void webRequestIsNeverIncluded() {
         assertFalse(filter.include("Server.Http-Request"));
     }
 
+    /**
+     * By default every header but Cookie, every query string parameter but
+     * those never shared, and any other evidence are shared, and cookies and
+     * session values only when their names start with 51D_.
+     */
     @Test
     public void defaultRules() {
         EvidenceKeyFilterShareUsage filter = defaults();
@@ -87,22 +121,93 @@ public void defaultRules() {
         assertFalse(filter.include("session.org.example.attribute"));
         assertTrue(filter.include("query.51d_gethighentropyvalues"));
         assertTrue(filter.include("query.51D_Test"));
-        assertFalse(filter.include("query.password"));
+        assertTrue(filter.include("query.password"));
+        assertTrue(filter.include("Query.Utm_Source"));
         assertFalse(filter.include("query.id.email"));
+        assertFalse(filter.include("server.request-query"));
         assertTrue(filter.include("server.client-ip"));
         assertTrue(filter.include("nodot"));
     }
 
+    /**
+     * With share all query string parameters off, only the 51D_ parameters
+     * and those named are shared.
+     */
+    @Test
+    public void shareAllQueryStringParametersOff_OnlyNamedAnd51D() {
+        EvidenceKeyFilterShareUsage filter = namedParametersOnly(
+            Collections.<String>emptyList(),
+            Collections.singletonList("campaign"));
+
+        assertFalse(filter.include("query.password"));
+        assertTrue(filter.include("query.campaign"));
+        assertTrue(filter.include("query.51D_Test"));
+        assertTrue(filter.include("header.user-agent"));
+    }
+
+    /**
+     * Even when all Evidence is shared, a cookie is shared only when its
+     * name starts with 51D_, whatever its case. The session cookie is kept
+     * only by a filter that includes it for the tracker to compare.
+     */
+    @Test
+    public void shareAllEvidence_NoCookieBut51D() {
+        EvidenceKeyFilterShareUsage sent = filter(false, true, true);
+        EvidenceKeyFilterShareUsage kept = filter(true, true, true);
+
+        for (EvidenceKeyFilterShareUsage filter : Arrays.asList(sent, kept)) {
+            assertTrue(filter.include("cookie.51D_Profile"));
+            assertTrue(filter.include("cookie.51d_screenpixelswidth"));
+            assertFalse(filter.include("cookie._ga"));
+            assertFalse(filter.include("Cookie.Other"));
+            assertFalse(filter.include("cookie.a.b"));
+            assertTrue(filter.include("session.org.example.attribute"));
+            assertTrue(filter.include("header.x-anything"));
+            assertTrue(filter.include("query.password"));
+        }
+        assertFalse(sent.include("cookie." + DEFAULT_SESSION_COOKIE_NAME));
+        assertTrue(kept.include("cookie." + DEFAULT_SESSION_COOKIE_NAME));
+    }
+
+    /**
+     * With all Evidence shared, the tracker compares the session cookie only
+     * when track session is on, and no other cookie but the 51D_ ones.
+     */
+    @Test
+    public void trackerWithShareAllEvidence_SessionCookieOnlyWhenTracked() {
+        EvidenceKeyFilterShareUsageTracker tracked =
+            new EvidenceKeyFilterShareUsageTracker(
+                Collections.<String>emptyList(),
+                Collections.<String>emptyList(),
+                true,
+                DEFAULT_SESSION_COOKIE_NAME,
+                true,
+                true);
+        EvidenceKeyFilterShareUsageTracker untracked =
+            new EvidenceKeyFilterShareUsageTracker(
+                Collections.<String>emptyList(),
+                Collections.<String>emptyList(),
+                false,
+                DEFAULT_SESSION_COOKIE_NAME,
+                true,
+                true);
+
+        assertTrue(tracked.include("cookie." + DEFAULT_SESSION_COOKIE_NAME));
+        assertFalse(untracked.include("cookie." + DEFAULT_SESSION_COOKIE_NAME));
+        assertFalse(tracked.include("cookie._ga"));
+        assertFalse(untracked.include("cookie._ga"));
+        assertTrue(untracked.include("cookie.51D_Profile"));
+    }
+
     /**
      * A name containing a dot is matched as a whole against the parameters to
      * share.
      */
     @Test
     public void fieldIsEverythingAfterTheFirstDot() {
-        EvidenceKeyFilterShareUsage filter = filter(
+        EvidenceKeyFilterShareUsage filter = namedParametersOnly(
             Collections.<String>emptyList(),
-            Collections.singletonList("valid.email"),
-            false);
+            Collections.singletonList("valid.email"));
 
         assertTrue(filter.include("query.valid.email"));
         assertFalse(filter.include("query.valid"));
@@ -115,10 +220,9 @@ public void fieldIsEverythingAfterTheFirstDot() {
      */
     @Test
     public void listsIgnoreCase() {
-        EvidenceKeyFilterShareUsage filter = filter(
+        EvidenceKeyFilterShareUsage filter = namedParametersOnly(
             Collections.singletonList("X-Secret"),
-            Collections.singletonList("Campaign"),
-            false);
+            Collections.singletonList("Campaign"));
 
         assertFalse(filter.include("header.x-secret"));
         assertTrue(filter.include("query.campaign"));