[jwrosewell]

Rank Evidence prefixes in the order evidence.md lists them, and send one name once whatever its letter case

Problem

When one name is under more than one prefix, the Cloud Request Engine
should send it once, with the value of the prefix that ranks highest. The
engine ranks server, fiftyone and location alphabetically, where the
specification lists them in another order, and it sends one name given in
two letter cases twice.

getSelectedEvidence sorts the Evidence that is not query, header or cookie
Evidence by the whole key, in reverse and in its exact letter case, with
Object.keys(selectedEvidence).sort().reverse() at
line 677 of cloudRequestEngine.js on rc 5228a9f9,
and getContent adds that group first, at
line 544,
so the key added last wins. Four things follow.

1. fiftyone wins over location, which wins over server, and any other
   prefix that sorts before them, such as apple, wins over all three.
2. Location.b wins over fiftyone.b, because a capital letter sorts
   before a small one.
3. hasKeyPrefix at
   line 702
   compares the prefix in its exact letter case, so Header.d is not header
   Evidence and cookie.d wins over it, and Query.n loses to header.n.
4. addQueryData looks for a name already added with
   hasOwn(queryData, suffix) at
   line 622,
   in its exact letter case, so header.User-Agent and query.user-agent
   go to the cloud as two fields, User-Agent and user-agent, rather than
   one field with the query value.

What the specification says

evidence.md at d72eb074.

Keys MUST be case-insensitive and will usually be split into
<prefix>.<field>.

The prefix indicates where the value has come from. Currently, defined
prefixes are:

• query – Used for Evidence specifically set by the application, or
  values supplied in a query string or the body of a post request. The
  suffix is the parameter name.
• header – Evidence came from an HTTP header. The suffix is the header
  name.
• cookie – Evidence came from a cookie. The suffix is the cookie name.
• server – Evidence came from a server that received a request but did
  not fit into any other category. For example, the IP address of the
  server.
• fiftyone – used internally by 51Degrees.
• location – Used in some cases to supply geo-location information.

The prefixes above are in order from highest to lowest precedence. I.e. if
the same Evidence value is available with two different prefixes (For
example, header.user-agent and query.user-agent), then the entry whose
prefix is earlier in the list above MUST be used.

If any other prefixes are present then their order of precedence is
alphabetical.

The Processing section of
cloud-request-engine.md at d72eb074
applies that order to the request.

Where there are conflicts, the precedence order defined in Evidence MUST
be used to determine which value to send to the remote server.

So the order is query, header, cookie, server, fiftyone, location, then
every other prefix alphabetically, and the prefix's letter case does not
change its place. Because keys are case-insensitive, header.User-Agent
and query.user-agent hold the same field once the prefix is removed, so
the order decides which one value is sent.

What the other languages do

• Go ranks prefixes this way already, in ComparePrefixes at
  lines 234 to 273 of pipeline/core/evidence.go on rc be6088ae,
  comparing the prefixes in lower case.
• 51degrees-dotnet#136,
  51degrees-java#107,
  51degrees-python#99
  and 51degrees-php#70
  make the same changes in those languages, ranking the prefixes and
  sending one name once whatever its letter case.

What changed

pipeline/fiftyone.pipeline.cloudrequestengine/cloudRequestEngine.js, and
the description of getContent in types/cloudRequestEngine.d.ts. Line
numbers in items 1 to 6 are at 9c3a115, the head before the two review
fixes, and item 7 is at the head, 616351a.

1. getSelectedEvidence sorts the other Evidence with a new
   compareOtherKeys, lowest precedence first, at
   lines 689 to 695.
2. compareOtherKeys at
   lines 801 to 845
   ranks the prefix by its place in OTHER_PREFIX_PRECEDENCE,
   ['server', 'fiftyone', 'location'], puts any other prefix after them
   ordered by the prefix itself, and breaks a tie between two keys with the
   same prefix on the whole key. keyPrefix gives the text before the first
   dot, or the whole key where there is none, in lower case, which is how
   addQueryData already splits a key.
3. hasKeyPrefix at
   line 715
   compares the prefix ignoring letter case. A key still needs the dot to
   have the prefix, as before.
4. The check that leaves out the conflict warning for a query key, at
   line 639,
   compares the prefix in lower case, so Query.n replaces header.n
   without a warning, as query.n does.
5. addQueryData at
   lines 623 to 666
   looks for a name already added ignoring letter case. A later, higher
   precedence value replaces it, name and value, so the name is sent once
   and spelled as the winning key spells it. The conflict warning matches
   keys and names ignoring letter case as well. This is what
   51degrees-dotnet#136,
   51degrees-java#107
   and 51degrees-python#99
   do.
6. getContent's description and the comment over the first group say the
   order. The description no longer also says "'query.' evidence should
   take precedence over all other evidence", which repeated its first
   sentence, and keeps the sentence about the conflict warning
   (lines 527 to 545 at 616351a).
   types/cloudRequestEngine.d.ts carries the same description, so it now
   matches the source.
7. addQueryData finds a name already added through a Map from the name
   in lower case to the spelling added
   (lines 611 to 682 at 616351a).
   The map is built from queryData once when the function starts, at
   line 614, read once for each value at line 640, and set for each value
   added at line 680, on both the new name path and the replace path. It
   was a walk of every name already added for each value, so the work
   grew with the square of the number of values. .NET gets the same single
   read from its case-insensitive dictionary. A Map is used rather
   than a plain object, so a name such as constructor cannot find a
   member of the object prototype.

Nothing else calls getSelectedEvidence, addQueryData, hasKeyPrefix
or the new functions. getData calls getContent at
line 425
on every data request, so the order applies to every request. No
signature changed.

Checks made

• Every exit path. getSelectedEvidence has one exit for each group,
  and only the other group's sort changed. compareOtherKeys returns on a
  different rank, then on a different prefix, then on the whole key, and
  returns 0 only for two keys equal in lower case. hasKeyPrefix has one
  exit. addQueryData has three paths for each key, as before. A reserved
  name is skipped. A name not yet added, in any letter case, is added under
  its own spelling. A name already added in any letter case is replaced,
  name and value, after a warning where the key is not query Evidence and
  another key holds the same name.
• A key with no dot. keyPrefix takes the whole key as the prefix, as
  addQueryData takes it as both prefix and name, and hasKeyPrefix still
  needs the dot, so such a key stays in the other group, as before.

Tests

tests/cloudEvidencePrecedence.test.js, six tests.

• each prefix wins over every later one gives one name for each of the 28
  pairs of query, header, cookie, server, fiftyone, location, apple and
  zebra, the higher prefix added first so the order added cannot decide it,
  and checks the higher prefix's value is sent for every pair.
• a prefix ranks the same in any letter case checks fiftyone.b wins
  over Location.b, SERVER.c over Location.c and Header.d over
  cookie.d.
• a query key in any letter case replaces another without a warning
  checks Query.n wins over header.n and logs nothing.
• other prefixes rank by the prefix alone checks ab.n wins over
  ab-c.n, although the key ab-c.n sorts first because a hyphen sorts
  before a dot.
• one name in two letter cases is sent once, from the higher prefix
  checks header.User-Agent and query.user-agent go as one field,
  user-agent, with the query value, and server.X-Thing and
  fiftyone.x-thing as one field, X-Thing, with the server value.
• the names already added are read once, not once for each value
  (lines 140 to 169 at 616351a)
  calls addQueryData with forty header values and one query value that
  replaces one of them in other letters, on a queryData whose ownKeys
  trap counts every read of its names, and checks the names were read at
  most once, that forty names were added and that the query value won.

Commits.

• 7164737, the first four tests.
• 0697802, the prefix order.
• d323e23, an rc merge, taking rc at 1d7e024
  (#61,
  #71 and
  #33), with no
  conflicts. Read and store evidence keys without regard to letter case #71 makes the Evidence store ignore letter case and keep a
  key's first spelling, which leaves every key these tests add distinct.
• 5a3c552, an rc merge, taking rc at 1cab8de
  (#74,
  #70 and
  #72), with no
  conflicts. Send the cloud every Evidence value apart from the reserved names #72 makes getContent read the Evidence no element accepts
  as well, a few lines above the lines this draft changes.
• d321fc1, the letter case test.
• 9c3a115, one name sent once whatever its letter case.
• a31f68a, an rc merge, taking rc at f76d8b0
  (#62,
  #55 and
  #45), with no
  conflicts. Accept no evidence in usage sharing, and share the web request it finds #45 changes cloudRequestEngine.js away from the lines this
  draft changes.
• e5f9b9e, the names read once test.
• 616351a, the Map lookup, and the repeated sentence cut from
  getContent's description.

Runs, each with a temporary workflow on Node 22 running
cloudEvidencePrecedence.test.js, cloudRequestEngine.test.js,
cloudRequestEngineCredentials.test.js and ESLint on the changed files.

• Red for the prefix order,
  run 43031388695
  on 7164737. All four new tests fail. The pairs test lists
  n-server-fiftyone sent fiftyone, n-server-location sent location,
  n-server-apple sent apple, n-fiftyone-apple sent apple and
  n-location-apple sent apple, the letter case test gets location for
  fiftyone, the query test gets header for query, and the prefix test
  gets ab-c for ab. The other two files pass, 36 tests.
• Green for the prefix order,
  run 43031387763
  on 0697802. The four new tests pass, the same 36 pass, and ESLint passes.
• Red for the letter case,
  run 43055821037
  on d321fc1. The new test fails, receiving User-Agent, X-Thing,
  user-agent and x-thing where it expects X-Thing and user-agent.
  The other four pass, and the other two files pass, 37 tests.
• Green for the letter case,
  run 43055821144
  on 9c3a115. All five pass, the same 37 pass, and ESLint passes.
• Full build,
  run 43056427316
  on 9c3a115, the head before the review fixes, passed. Configure, the 6 build and test jobs (Ubuntu,
  Ubuntu ARM and Windows, Node 22 and 24) and Complete succeeded, and the
  2 Compare Performance jobs were skipped, as the full build turns them
  off. Every one of the 7 Setting 'PullRequestSha' lines reads 9c3a115,
  and Configure printed "Left out 2 macOS entries". Earlier full builds
  were on earlier heads,
  run 43032151883
  on 0697802, cancelled when rc took Never send resource, license, licence or values from Evidence to the cloud #61 and Read and store evidence keys without regard to letter case #71,
  run 43034401232
  on d323e23 and
  run 43042464614
  on 5a3c552, both of which passed.

Runs for the two review fixes, each a temporary workflow on Ubuntu with
Node 22 running cloudEvidencePrecedence.test.js,
cloudAllEvidence.test.js and cloudRequestEngine.test.js, in a workflow
file named for its own branch. ESLint runs in the full build.

• Red for the names read once,
  run 43111959654
  on branch tmp/node-75-red4, the test commit e5f9b9e. The new test fails
  with "Expected: <= 1, Received: 41", one read for each of the 41 values,
  and the other 37 of 38 pass.
• Green for the names read once,
  run 43111979404
  on branch tmp/node-75-green4, the head 616351a. All 38 pass.
• An earlier red and green, on tmp/node-75-red3 and tmp/node-75-green3,
  and a full build on tmp/pr-matrix-75-f, were created during the GitHub
  Actions outage of 11 November and never started, so they are not cited.
• Full build on the head 616351a,
  run 43111863074,
  passed. Configure, the 6 build and test jobs (Ubuntu, Ubuntu ARM and
  Windows, Node 22 and 24) and Complete succeeded, and the 2 Compare
  Performance jobs were skipped, as the full build turns them off. Every
  one of the 7 Setting 'PullRequestSha' lines reads
  616351a, and Configure printed "Left out
  2 macOS entries". Each build and test job ran eslint . --ext .js, gave
  1,227 passed and 2 skipped in the unit tests and 132 passed and 4 skipped
  in the integration tests, and ran cloudEvidencePrecedence.test.js.

What remains

• Review by someone other than the author.

macOS was left out of this draft's full build under the 9 November decision.
The rc head is built with macOS before it merges into main.

[jwrosewell]

Closing and reopening at once to start the checks again, because the runs queued during GitHub's Actions outage this evening never started.