Skip to content

Share only the evidence usage sharing accepts, as .NET does - #51

Merged
jwrosewell merged 2 commits into
rcfrom
fix/usage-sharing-defaults
Nov 7, 2026
Merged

jwrosewell merged 2 commits into
rcfrom
fix/usage-sharing-defaults

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

This is the fix for #50, which can close once it reaches main. It also fixes the filter point listed under "Also found" in #28, and two faults found on the way.

Problem

  1. EvidenceKeyFilterShareUsage.include() split the key at every . and applied its rules only when that gave exactly two parts. Any other key was shared whatever the configuration said, for example query.id.email, cookie.a.b, session.org.example.attribute, and a header on the block list whose name has a dot.
  2. Java had no list of keys that are never shared. .NET never shares id.email, id.salt, request-query or request-path.
  3. Query string parameters starting 51D_ were not shared unless named, where the accepted evidence section of the specification and .NET share them (#28).
  4. Found on the way: the session cookie was never kept. The filter compared the lowercased cookie name with the session cookie name as given, and the default name is JSESSIONID, so cookie.JSESSIONID never matched and session tracking had no effect.
  5. Found on the way: the default tracker was built with the plain EvidenceKeyFilterShareUsage, not EvidenceKeyFilterShareUsageTracker. The plain filter includes the session id and sequence number, which differ on every request, so every request looked new to the tracker.

Change

  1. The key is split at its first ., so the prefix is everything before it and the field everything after it, as .NET does. A key with no . is still shared.
  2. id.email, id.salt, request-query and request-path are never shared, matched without regard to case as the whole key or after a ., and checked before every other rule, as in .NET's EvidenceKeyFilterShareUsage.
  3. Query string parameters starting 51D_ are shared, and the builder's documentation says so.
  4. The session cookie name is lowercased, as .NET does.
  5. The default tracker uses EvidenceKeyFilterShareUsageTracker.
  6. The Cookie header is blocked as well as cookies (see below).

For review

  1. The Cookie header is no longer shared. The specification says the default block list "SHOULD include the cookies HTTP header", and the filter blocked cookies, as .NET does. HTTP has no header called Cookies, and the request header that carries cookies is Cookie (RFC 6265 section 5.4), so the raw header, with every cookie in it, was shared and undid the cookie rule. The 51d_ cookies are still shared, as their own cookie. entries. .NET has the same gap.
  2. The four never-shared keys and the cookie header name are not in the specification yet. specifications#89 proposes both, so that every language agrees.

Tests

  • EvidenceKeyFilterShareUsageTests is new. It ports the .NET filter tests and adds the cases the issue lists: query.id.email, header.id.email, cookie.a.b, session.org.example.attribute and query.valid.email, together with query.51D_Test, the session cookie in either case, lists in mixed case and the tracker filter.
  • ShareUsageElementTests gains three tests that check what is sent. The first checks what the default settings send. The second uses the default tracker and checks that a repeat differing only in its session id and sequence is not sent again. The third tracks sessions and checks that cookie.JSESSIONID is kept and never sent, and that a new session is shared again.

Checks

  • The tests alone, at b5498fd, failed on Java 8 and 17 for the reasons above. All 9 new tests failed, among them cookie.JSESSIONID not kept, 2 records where 1 was expected for the repeat, query.51D_Test not shared, and id.email and server.request-query shared. Every existing test passed (run 42197039440, first attempt).
  • With the fix, at 0df5ed8, all 29 tests in the two classes passed on both versions, with the one existing disabled test skipped (run 42197039440, second attempt).
  • The full build at 0df5ed8 passed on all 15 Build and Test legs, with the performance comparison left out by the temporary build's settings (run 42197718302).

Release note

Usage sharing now applies its rules to keys with more than one dot, never shares an email address, its salt, the query string or the path, shares query string parameters starting 51D_, keeps the JSESSIONID session cookie for the tracker without sending it, and no longer shares the same request again because its sequence number changed.

Usage sharing should share what the specification's accepted evidence
section allows: headers other than Cookie, cookies, session values and
query string parameters starting 51D_, the parameters the caller names,
and any other evidence. A name containing a dot is matched whole. The
email address, the salt for hashed email identifiers, the query string and
the path are never shared. The session cookie is kept for the tracker
whatever the case of its name, and the default tracker does not share
again a request that differs only in its session id and sequence.

These tests fail on rc.
The usage sharing filter split each key at every dot and applied its
rules only to keys with exactly two parts, so any key with more, such as
query.id.email, cookie.a.b or a session attribute named after a class,
was shared whatever the configuration said. It now splits at the first
dot, so the field is everything after it.

The filter now also
- never shares id.email, id.salt, request-query or request-path, matched
  without regard to case as the whole key or after a dot, as .NET does,
- shares query string parameters starting 51D_, as the specification and
  .NET do,
- does not share the Cookie header, which carries every cookie. It
  blocked "cookies", which no request sends,
- matches the session cookie name without regard to case. The default
  name, JSESSIONID, was compared with the lowercased key, so the session
  cookie was never kept for the tracker.

The default tracker used the plain filter, which includes the session id
and sequence number, so every request looked new to it. It now uses
EvidenceKeyFilterShareUsageTracker, which leaves them out.

The builder's documentation now says that query string parameters
starting 51D_ are shared by default.
@jwrosewell
jwrosewell marked this pull request as ready for review November 7, 2026 00:22
@jwrosewell
jwrosewell merged commit 1aaaf70 into rc Nov 7, 2026
2 checks passed
@jwrosewell
jwrosewell deleted the fix/usage-sharing-defaults branch November 7, 2026 00:22
@jwrosewell jwrosewell mentioned this pull request Nov 12, 2026
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