[jwrosewell]

Evidence keys ignore letter case

The problem

Evidence keys are meant to ignore letter case, so header.User-Agent and
header.user-agent are one key. Node does not do that. Evidence.add stores
the key exactly as it was given and Evidence.get looks it up exactly as it was
given, in
pipeline/fiftyone.pipeline.core/evidence.js,
so the two spellings become two separate entries in evidenceStore and a
caller that adds one spelling and reads the other gets undefined.

That matters in real use because the letters a key arrives in are not under our
control. HTTP header names are case-insensitive, so a value added from a request
or by a framework can reach the pipeline in any letters, whilst our own elements
read a fixed lower-case key. The element then sees nothing, and the pipeline
behaves as though the evidence was never supplied.

BasicListEvidenceKeyFilter.filterEvidenceKey already compared with
toLowerCase() on both sides, at
basicListEvidenceKeyFilter.js line 51,
so a key in any letters is kept or dropped correctly by it. The JavaScript
Builder's filter, at
javascriptbuilder.js line 180,
did not, and is changed here, as Tests below describes. The links are at
c32a2905, the rc this draft started from.

What the specification says

From
pipeline-specification/features/evidence.md,
at commit 67ead6986f3118c68f0714325800b92d48ddc71f, line 10:

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

What changed and why

The change follows the shape Python and PHP already chose, because this is all
about alignment across the languages. Python's is
51degrees-python 95
and PHP's is
51degrees-php 63, and both
keep a side map from the key in lower case to the key as it was first added.
Node now does the same.

In pipeline/fiftyone.pipeline.core/evidence.js:

• The constructor gains this.keys, the map from a key in lower case to the key
  the value is held under in evidenceStore.
• add folds the key to lower case and, where that folded key is already known,
  reuses the spelling the key was first added with before doing anything else.
  So a second add under other letters replaces the first value rather than
  making a second entry, and evidenceStore keeps one entry per key. The map is
  only written in the branch that actually stores the value, so a key the
  element filters reject leaves no entry behind and a later add of the same key
  is still judged on its own.
• get folds the key to lower case, looks the stored spelling up in the map and
  returns that value, returning undefined where the key is not there, which is
  what it returned before for a key that was never added.

types/evidence.d.ts is updated to match the source, with the new keys field
and the two reworded doc comments, because that file is generated from the
JSDoc and a stale declaration would describe behaviour the code no longer has.

getAll is deliberately left alone. It still returns evidenceStore, which now
holds one entry per key under the letters it was first added with, so a caller
that enumerates evidence sees each key once.

Tests

The tests are in their own commit so a red run is possible from it, before the
fix exists. They are in a new file,
pipeline/fiftyone.pipeline.core/tests/evidenceKeyCase.test.js, and they cover:

• A key added in one case is read in any case, including the exact spelling.
• Keys differing only in case are one key, so getAll holds one entry and the
  later value wins.
• A key keeps the letters it was first added with.
• An element reads evidence added in other letters. This builds a default
  pipeline, which includes the Sequence Element, adds QUERY.SESSION-ID and
  Query.Sequence, and checks the Sequence Element read the sequence and
  advanced it, which it can only do through a case-insensitive get.
• A key that was never added is still undefined, which guards the exit path
  of get that the change rewrote.
• The JavaScript Builder keeps a key added in other letters. Its filter at
  javascriptbuilder.js line 180 looked for query. and header. in the
  letters given, so on a pipeline where it is the only element
  Query.Client-IP and HEADER.User-Agent were thrown away before get
  could be asked for them. Whether a key is kept now ignores letter case as
  well, as the basic list filter and the usage sharing filter already did.

Runs

• Red, run 42980261404,
  on the tests-only commit f9d74d6. 4 of the 5 tests in
  evidenceKeyCase.test.js fail, and the one that passes is the undefined
  case, which passes before and after and is there to prove the change did
  not break the absent-key path.
• Red for the JavaScript Builder, run 42994042673
  on branch tmp/node-evcase-red2, the test commit 30b4af9 over the store
  fix. The new test fails and the other 5 pass.
• Green, run 42994059163
  on branch tmp/node-evcase-green2, the head 4f31fff, with
  evidenceKeyCase.test.js and javascriptbuilder.test.js, 45 of 45 pass.
• rc is merged in at 1e077a7 to bring the current cxx submodule.
• Full build on the head 4f31fff, success, run 42996638568. Configure and the Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 pass, 1003 of 1005 tests with 2 skipped in each, Configure printed "Left out 2 macOS entries", and all 7 Setting 'PullRequestSha' lines are 4f31fff2570d8234c91c5254e92c1d097adc897c.

4f31fff also makes the table from each key in lower case to its stored
spelling with no prototype, so __proto__ or constructor is an ordinary
entry in it. The evidence store itself keeps its prototype, as it did,
because getAll() hands it to callers who may call its methods, so a value
added under __proto__ is still not stored. That was so before this
change.

What remains

• node#72, which
  keeps the evidence no element accepts for the Cloud Request Engine, also
  changes Evidence.add, so whichever of the two merges second takes the
  other in first, and keys Send the cloud every Evidence value apart from the reserved names #72's set aside store through the same lower-case
  table, so that two spellings of one key no element accepts are one entry
  and are sent once.
• The same rule in Python and PHP is in their own drafts, linked above.

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.