Skip to content

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

Draft
jwrosewell wants to merge 3 commits into
rcfrom
fix/usage-sharing-defaults
Draft

jwrosewell wants to merge 3 commits into
rcfrom
fix/usage-sharing-defaults

Conversation

@jwrosewell

Copy link
Copy Markdown
Contributor

Usage sharing now shares every query string and HTTP form value by default, and shares no cookie but the 51D_ ones even when all Evidence is shared, as specifications#155 now says.

Why

James Rosewell decided at about 20:55 UTC on 14 November that the rule the cloud service follows is every language package's default: "Every package shares every query and form value by default, and Share all Evidence no longer shares cookies other than 51D_ ones (the raw Cookie header never)." specifications#155 wrote it into the usage sharing element's page, merged into the specifications' rc at 33de68a0.

What the specification says

usage-sharing-element.md lines 65 to 69 at 30b3741f:

It MUST also return false, whatever the configuration and even when all Evidence is shared, for any key whose prefix is cookie. and whose field does not start with 51D_. The 51D_ cookies carry the values the 51Degrees JavaScript sends, and they are the only cookies shared. A cookie a site sets for itself, such as its session cookie, is never shared.

Lines 76 to 81:

  • Prefix = query., the share all query string parameters option is off, and field does not start with 51D_ and is not in a user-configurable allow list of query string parameters that will be shared.

Share all query string parameters is on by default, so every query string and HTTP form parameter is shared unless a user turns it off.

The options table (line 425) gives share all query string parameters a default of true. Lines 90 to 99 keep the session cookie for the repeat check when track session is on, and say "The session cookie MUST NOT be shared unless its name starts with 51D_, as no other cookie is."

What changed

  • Constants.java gains SHARE_USAGE_DEFAULT_SHARE_ALL_QUERY_STRING_PARAMETERS, true, and the builder (ShareUsageBuilderBase, its field, Javadoc and @DefaultValue), the element's constructors that take no value for it (ShareUsageBase, ShareUsageElement) and the filter's shorter constructor (EvidenceKeyFilterShareUsage) take their default from it.
  • EvidenceKeyFilterShareUsage.include now applies the cookie rule before the share all Evidence check, so with all Evidence shared a cookie is still kept only when its name starts with 51D_, or when it is the session cookie and the filter was made with the session included. The element makes the filter for the record with the session left out (ShareUsageBase.java line 547), so the session cookie is never shared, and the tracker's filter keeps it when track session is on (line 566), so the session is still compared.
  • The Javadoc of the options says what they now do.

Tests

Tests first, 4ce76a71. New in EvidenceKeyFilterShareUsageTests: shareAllQueryStringParametersOff_OnlyNamedAnd51D, shareAllEvidence_NoCookieBut51D (a 51D_ cookie in any case is kept, _ga and another cookie are not, and the session cookie only by a filter made with it) and trackerWithShareAllEvidence_SessionCookieOnlyWhenTracked. defaultRules now expects query.password and Query.Utm_Source shared and server.request-query not. The tests of the allow list, and the element and options tests that took the old defaults, now say which default they test.

Red 43673395639 ran the tests with rc de74c8c and no change, plus a throwaway workflow, Java 17. 9 of 50 failed, exactly the new and changed tests: EvidenceKeyFilterShareUsageTests defaultRules, shareAllEvidence_NoCookieBut51D and trackerWithShareAllEvidence_SessionCookieOnlyWhenTracked, ShareUsageElementTests ShareUsageElement_DefaultSharesOnlyAcceptedEvidence ("password parameter") and ShareUsageElement_WebRequest_SharesWhatNoOtherElementAccepts ("A query string parameter from the web request was not shared by default."), and four ShareUsageOptionsTests. UsageSharingEvidenceTests passed 7 of 7.

Green 43673749176 ran the head plus the same workflow. EvidenceKeyFilterShareUsageTests 15 of 15, ShareUsageElementTests 26 with 1 skipped as on rc, ShareUsageOptionsTests 9 of 9 and UsageSharingEvidenceTests 7 of 7 passed. An earlier green, 43673384594, stopped at a missing static import in ShareUsageElement, which the fix commit now holds.

Full build

Under way, without macOS under the 9 November decision. This section is replaced with the run, its result and the head it built when it ends.

Before you upgrade

  • Every query string and HTTP form parameter is now shared with usage sharing by default, apart from the keys that are never shared. To share only the 51D_ parameters and those named, as before, set setShareAllQueryStringParameters(false) on the builder, or ShareAllQueryStringParameters to false in the options file.
  • With share all Evidence on, a cookie whose name does not start with 51D_ is no longer shared, the session cookie included. Before, share all Evidence shared every cookie.

What it releases

pipeline.engines.fiftyone changes, so it and the packages that depend on it are in Monday's plan under version per package.

Base branch

rc at de74c8c, merged into the branch at fab13be.

…D_ ones

The specification now has share all query string parameters on by
default, and even when all Evidence is shared it shares no cookie whose
name does not start with 51D_. The session cookie is still compared by
the tracker when track session is on. The tests that took the old
defaults now take the new ones, and the tests of the allow list turn
share all query string parameters off.
…when all Evidence is shared

The usage sharing specification now has share all query string parameters
on by default, and returns false for every cookie whose name does not start
with 51D_, whatever the configuration and even when all Evidence is shared.
The builder, the element and the filter take the new default from one
constant, and the cookie rule now runs before the share all Evidence check.
The tracker's filter still keeps the session cookie when track session is
on, so that the session is compared, and the filter for the record never
keeps it, so it is never shared.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant