Repository navigation
Accept no evidence in usage sharing, and share the web request it finds - #45
Merged
Merged
Conversation
The tests ask for the Cache-Control, Vary and weak ETag headers of the web integration specification's cache header detail on both client-side endpoints, the same ETag for the same evidence in two processes, a 304 for a matching If-None-Match on a GET and a 412 on a POST, as RFC 9110 section 13.1.2 requires, and private, no-cache with no ETag on a response to a request that recorded errors, while a matching If-None-Match still gets its 304.
Node's client-side JSON and JavaScript endpoints are written in each web example, and none sent Cache-Control, Vary or an ETag, or answered If-None-Match, so they had none of the web integration specification's cache header detail. Helpers.writeClientSideResponse writes either endpoint's response with those headers. The ETag is a SHA-256 of the evidence the request carries, read from the request again, so evidence processing adds, such as the new session id the sequence element makes, takes no part, and leaving out the request's precondition and cache headers, so that a revalidation can match. The same evidence therefore gives the same ETag in every process. It is weak, as RFC 9110 section 8.8.3 asks of a validator that can stay the same when the response changes. A matching If-None-Match gets a 304 with the same Cache-Control, Vary and ETag to a GET or HEAD, and a 412 to any other method, as RFC 9110 sections 13.1.2 and 15.4.5 require. A response to a request whose processing recorded errors carries Cache-Control private, no-cache and no ETag, and a matching If-None-Match still gets its 304. The web examples of Device Detection, IP Intelligence, location and the pipeline's custom element example use it for every client-side endpoint.
…sing A browser can stop waiting while the pipeline processes, and the request's socket then has no addresses. Reading the request again for the ETag then threw a TypeError, which stopped the Device Detection cloud web example during the Selenium contract tests.
writeClientSideResponse read the request again for its ETag after processing. A browser can stop waiting while the pipeline processes, and the request's socket then has no addresses, so the read threw a TypeError. In the Selenium contract tests on macOS this stopped the Device Detection cloud web example. addFromRequest now keeps every value it reads, under its evidence key, in requestEvidence, and the ETag is built from that, hashing only the evidence an element accepts, so it no longer touches the request. Evidence added by hand has no such record, so its ETag is built from the flow data's evidence.
The tests check that a pipeline holding the usage sharing element does not accept If-None-Match or an arbitrary header, that the web request is in the evidence as server.http-request, that a header no other element accepts is still shared from it while the Cookie header and a blocked header are not, that evidence added by hand is shared as before, and that the client-side endpoint answers a revalidation with a 304 and leaves evidence only usage sharing reads out of its ETag.
The usage sharing element's evidence key filter accepted every header but Cookie, and the evidence an element accepts is what caching, the ETag and the Vary header are built from, so the client-side ETag took in every header the usage sharing element collected, such as the client's 51D_ cookies and the server's own address. The element now accepts no evidence. It shares what it finds, being every value of the web request and every evidence value, with the evidence value winning for a key in both, under the same rules as before, and the tracker compares the values that would be shared. Evidence keeps a key an element accepts, as before, or one an element keeps without accepting it, given by the new keptEvidenceKeyFilter. The usage sharing element keeps what it would share, so evidence added by hand is shared as it was. addFromRequest also adds the web request as server.http-request, as a frozen object holding every value it read under its evidence key, whether or not an element accepts it. get returns it and getAll leaves it out, because it is the request rather than a value to process. writeClientSideResponse hashes only the evidence an element accepts.
jwrosewell
force-pushed
the
fix/usage-sharing-accepts-no-evidence
branch
from
November 7, 2026 22:31
84472f8 to
86f6877
Compare
…ement's keys The tests check that the cloud request engine neither sends server.http-request to the cloud service nor lets it change its cache key, and that the sequence element accepts query.session-id and query.sequence itself, so that alone in a pipeline it keeps a session going.
The sequence element read and wrote query.session-id and query.sequence but accepted no evidence, so the flow data kept them only because the JavaScript builder or the usage sharing element accepted them. Alone in a pipeline, the session id it made was dropped. It now accepts both, as its specification lists them. The cloud request engine and the on-premise engines read getAll(), which leaves server.http-request out, so neither sends it to the cloud service or to native code, and it takes no part in the cloud request engine's cache key. The tests in the commit before check the cloud request engine.
… read The JavaScript builder accepted every header and the set headers element, having no filter of its own, accepted every key, so the default pipeline accepted If-None-Match and any other header. The tests ask that it does not, and that it still accepts the host, the protocol and the query values the JavaScript builder reads, and the User-Agent an engine reads.
…headers Under the rule that an element accepts only the evidence that can change its results, two elements of the default pipeline were wide. The JavaScript builder accepted every header, though only the host and the protocol change the script, and the set headers element had no filter of its own, so it accepted every key, though it reads only the other elements' results. With either in the pipeline, If-None-Match and every other header were accepted evidence. The JavaScript builder now accepts every query value, which the script sends back as its parameters, and header.host and header.protocol. The set headers element accepts none. The usage sharing element keeps the headers it shares, so what it shares is unchanged.
… what they read, and keep the evidence the tests read with elements that say so
… header is never shared
…path and session values
This was referenced Nov 9, 2026
Merged
…ts-no-evidence # Conflicts: # pipeline/fiftyone.pipeline.cloudrequestengine/cloudRequestEngine.js
An on-premise engine reads the keys it accepts from the data file when it initialises, which waits for the download when updateOnStart is set and there is no file yet. With an element now accepting nothing by default, evidence added before the file arrived was dropped, so processing failed with 'The evidence required to determine this property was not supplied'. Both engines now keep every key until initEngine replaces the filter with the keys the file lists, as the cloud request engine does for the keys the service lists.
…ts-no-evidence # Conflicts: # pipeline/fiftyone.pipeline.cloudrequestengine/tests/cloudRequestEngine.test.js
…ts-no-evidence # Conflicts: # pipeline/fiftyone.pipeline.core/evidence.js # pipeline/fiftyone.pipeline.core/javascriptbuilder.js # pipeline/fiftyone.pipeline.core/types/evidence.d.ts
The builder now accepts only what it reads, which the specification's Accepted Evidence list for the element gives as header.host, header.protocol and the query string. The test's point is that a key is found whatever its letters, so it uses the host rather than the user agent, which the builder does not read.
…ts-no-evidence # Conflicts: # pipeline/fiftyone.pipeline.core/evidence.js
jwrosewell
marked this pull request as ready for review
November 11, 2026 19:34
This was referenced Nov 11, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Node's usage sharing element accepted, as its own evidence, every key it might
share. Accepted evidence is what the Pipeline advertises, and the ETag and the
Vary header of the client-side endpoints and the engines' cache keys are built
from it, so adding usage sharing to a pipeline changed the ETag of every
response and widened every cache key, for evidence that cannot change a single
result.
The Flow Data keeps a key only when some element accepts it, so usage sharing
accepting everything was also the only reason a header no other element reads
reached the Flow Data at all. The two could not be separated without another
route to the request, and that route is
server.http-request.The Cookie header was blocked only while
shareAllwas off, so turningshareAllon shared every cookie the browser sent, including the ones thecookie rule exists to leave out.
.NET, Java, Python, PHP, Go and Rust have all taken this change, so Node is the
last language still accepting what it shares.
What the specification says
usage-sharing-element.md, Accepted Evidence,
at the fixed commit d72eb074.
Shared Evidence
at the same commit adds the Cookie header to the keys that are never shared.
The same section says the session cookie is read from the same two places when
track session is on, that it is not shared unless all evidence is shared, and
that the hash the tracker compares "is of the values that would be shared, as
the element accepts no Evidence of its own for the caching logic to read".
advertize-accepted-evidence.md
at the same commit is the rule the element default follows, that an element
accepts only the Evidence it reads.
sequence-element.md, Accepted Evidence
gives the sequence element's keys as
query.session-idandquery.sequence,which is what it now accepts.
Change
The web request, as evidence
evidence.js
addFromRequestbuilds the whole web request as a frozen object and adds itunder
server.http-request, with every value it read under the key thatvalue has as evidence, plus the whole request line as
server.request-method,server.request-pathandserver.request-query.It also now reads the session's values as
session.evidence, which thepage lists among the parts of the request.
request.methodandrequest.urlbyte for byte with no decoding, with themethod empty where the request carries none and the query empty where the
target has no question mark, which is what
web-integration.md, Populating Evidence
asks for, so that an absent key always says the request line was not
supplied rather than that the request had no query.
requestLinePartsreplaces
requestLinePath, which wrote an empty path as/. .NET'sWebRequestEvidenceService.SplitRequestTargetsplits the same way, at thefirst question mark with the mark dropped from the query. Both
server.request-pathandserver.request-queryare in the usage sharingelement's
NEVER_SHARED_KEYS, so neither is ever shared.addkeepsserver.http-requestapart from the evidence store rather thanfiltering it, because it is the request rather than a value to process, and
getgives it back.getAllleaves it out, so nothing that walks theevidence meets it. Both comparisons are made on the folded key, so the rule
rcbrought in, that evidence keys ignore letter case, holds for this keytoo.
addalso keeps a key that an element's newkeptEvidenceKeyFiltermatches,which is how the flow data still holds what usage sharing shares without
usage sharing accepting it.
requestEvidencefield is gone, replaced by the same values underserver.http-request.flowElement.js
everything, which is the element default the specification asks for.
keptEvidenceKeyFilteris the evidence an element reads without it changinga result. The flow data keeps it, and nothing else reads the field, so it
never reaches the accepted evidence, the ETag, the Vary header or a cache
key.
This changes what an existing Flow Element of your own is given. On
rcan element that set no
evidenceKeyFiltergot the baseEvidenceKeyFilter,whose
filterEvidenceKeyreturns true, so it accepted every key. After thischange it accepts none, and a Flow Element written outside this repository
that relied on the old default stops being given any evidence, with no error
and no warning. Anyone upgrading has to give such an element the filter naming
the keys it reads. This belongs in the release note for the version that
carries it.
helpers.js
server.http-requestrather than theremoved
requestEvidence, and still hashes only what an element accepts, soevidence kept for usage sharing takes no part.
Usage sharing
shareUsage.js
evidenceKeyFilteris an emptyBasicListEvidenceKeyFilter, so the elementaccepts nothing. The filter it had becomes
keptEvidenceKeyFilter, so theflow data still keeps what would be shared and, when sessions are tracked,
the session cookie.
dataEvidenceKeyFilter, which decides what is sent, is still built withincludeSession: false, so the session cookie is read for the tracker andnot shared, as the page asks.
getFoundEvidenceis the two places the page names. It reads every value ofserver.http-requestand then every evidence value, so a key in both takesthe evidence value. The two are merged on the folded key, so a key whose
letters differ between the view and the Evidence is one value under the
Evidence spelling, which is the page's "A key found in both places is shared
once".
getDataFromEvidenceand the tracker's key both readgetFoundEvidence, soa header no other element accepts is shared, and the tracker compares the
values that would be shared.
shareUsageEvidenceKeyFilter.js
header.cookiemoves from the headers blocked whileshareAllis off intoNEVER_SHARED_KEYS, so it is never shared whatever the settings.server.http-requestitself is never shared, only the values read from it.Other elements
The cloud request engine keeps every key until the cloud service's keys arrive,
and then narrows to them, so it no longer accepts everything for the life of a
process that never reaches the service. The set headers element accepts
nothing, because it reads the other elements' results. The sequence element
accepts
query.session-idandquery.sequence, which is the list its pagegives. The JavaScript builder accepts
header.hostandheader.protocolrather than every header.
The two on-premise engines read the keys they accept from the data file when
the engine is initialised, which waits for the download when
updateOnStartis set and there is no file yet. With an element accepting nothing by default,
evidence added before the file arrived was dropped, and the first full build on
this draft failed on
update on start-up with no data file > downloads the data file and then processes with itwithThe evidence required to determine this property was not supplied. BothdeviceDetectionOnPremise.js
and
ipIntelligenceOnPremise.js
now keep every key until
initEnginereplaces the filter with the keys thefile lists, which is what the cloud request engine does for the keys the
service lists.
One test of
rc's own moved with the behaviourevidenceKeyCase.test.js, which51degrees-node#71
added to
rc, has a test that the JavaScript Builder finds a key whatever itsletters, and it used
HEADER.User-Agentas its example because the old filteraccepted every header. The builder no longer accepts the user agent, which it
does not read, so the test now uses
HEADER.Host. The point of the test, beingletter case, is unchanged.
What else this branch carries
This branch holds
51degrees-node#46,
"Accept only the evidence each element reads, and none by default", by a merge
commit, and has since it was opened. That change is the element default above
and the per element filters, and it is the same rule
51degrees-python#63
and 51degrees-php#41
merged. Usage sharing cannot accept nothing until the default is nothing, so
the two cannot be separated without undoing the merge and rebuilding both. If
this draft merges, #46 has nothing left to add and the coordinator should close
it as superseded.
The conflicts with rc
This draft was CONFLICTING when it was picked up, and
rchas moved six timessince. Every resolution is below.
cloudRequestEngine.js.rcand this branch each added an import,LicenseKeysonrcandEvidenceKeyFilterhere. Both are kept.cloudRequestEngine.test.js. Each side appended a test at the end of thefile, this branch's
evidence set before the keys arrive is keptandrc'sevidence never supplies resource, license or valuesfrom#61. Both are kept,
in that order.
evidence.js,types/evidence.d.tsandjavascriptbuilder.js, against#71.
addandgetnow fold the key to lower case once and compare the folded key against
server.http-requestbeforerc's lookup by folded key, so both ruleshold.
JSEvidenceKeyFilterkeepsrc'sString(key)guard and thisbranch's narrower list. The two doc comments are joined.
shareUsage.jsmerged with no conflict against#36, which
rctookat 13:30. Both features are in the merged file, the back-off and the empty
accepted evidence, and the generated
types/shareUsage.d.tscarries both.rcat5228a9f9,
conflicted in
evidence.jsalone, inget, against#72.
getnowanswers
server.http-requestfrom the fieldaddset aside, and otherwiseuses
rc's lookup, which reads the accepted store alone so that a key setaside is not returned. Both rules hold.
rcat 8093efac,the move of
cxxto the release commit 7c4ac436 from#55, merged with no
conflict. It changes the submodule and a generated wrapper, neither of
which this draft touches.
The same change in the other languages
All six are merged. All six use the key
server.http-request, and .NET and PHPalso put the request's method on the view, as this does. .NET's
EvidenceKeyFilterShareUsagekeeps the session cookie out of what is sharedand leaves it to the tracker's own filter, which is what
dataEvidenceKeyFilterandtrackerEvidenceKeyFilterdo here.Tests
shareUsageAcceptsNoEvidence.test.js
sends real requests to a local server and reads what was shared. It checks that
a pipeline with usage sharing does not accept arbitrary headers, that the web
request is in the evidence with every part the page names, that the method is
shared while the path and the Cookie header are not even when all evidence is
shared, that a header no other element accepts is still shared from the web
request, that evidence wins over the request for the same key, that a key in
both places whose letters differ is shared once, that the request line is on
the view with an empty query where the target has none, that evidence added by
hand is shared as before, and that evidence only usage sharing reads takes no
part in the ETag while a revalidation still gets a 304.
acceptedEvidence.test.js,
elementDefaultEvidence.test.js
and
sequenceElementAccepts.test.js
cover the element default and the per element filters.
httpRequestEvidence.test.js
checks the web request is neither sent to the cloud service nor part of its
cache key.
updateOnStartMissingFile.test.jsin each on-premise package gainskeeps every key until the data file arrives, which names the rule theregression above broke, so it cannot break again without a test saying so.
Red, the first round. Run 43068117316,
on
tmp/node-45-red2, which isrcat 5228a9f with this branch's test filesand nothing else from it. All six suites failed, 18 of the 29 tests, being six
of the nine in
shareUsageAcceptsNoEvidence.test.js, two of the nine inshareUsageEvidenceKeyFilter.test.js, one of the two inhttpRequestEvidence.test.js, all three inacceptedEvidence.test.js, allfour in
elementDefaultEvidence.test.jsand both insequenceElementAccepts.test.js. The 11 that passed assert whatrcalreadydoes, chiefly the seven
shareUsageEvidenceKeyFilter.test.jsalready had.Red, the request line and the folded key.
Run 43077365630,
on
tmp/node-45-red4, which is this branch with the three new assertions andwithout the code that answers them. Three of the 11 failed, being
the web request has every part the specification names,the request line is on the view with an empty query where there is noneanda key in both places whose letters differ is shared once.Green. Run 43077435257,
on
tmp/node-45-green10, which is this head with one temporary workflow added.The six suites above and
evidenceKeyCase.test.js,evidence.test.jsandcloudRequestEngine.test.jspass, 71 of 71 in 9 suites.Full build. Run 43077468895
on the head, 834dec1, every job a success. All seven
Setting 'PullRequestSha'lines read834dec1, Configure printed "Left out 2 macOS
entries", and each of the six build jobs ran 1,221 tests passed with 2 skipped,
then 132 passed with 4 skipped.
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.
What remains
inside this branch and should be closed as superseded if this merges. That is
the coordinator's call, not mine.
stacked on Accept only the evidence each element reads, and none by default #46's branch and needs rebasing onto
rcafter this merges.query.key, wherejavascript-builder.md, Accepted Evidence
names four, being
header.host,header.protocol,query.fod-js-object-nameandquery.fod-js-enable-cookies, and .NET'sJavaScriptBuilderElementhas exactly those four in its whitelist. It isleft as it is here, and the reason is worth writing down for whoever takes
it. Step 7 of the same page says the script's parameters are "all Evidence
values starting with
query.", and .NET reads them straight from the flowdata's evidence, which in .NET holds every value added whether or not an
element accepts it. Node's
Evidence.getanswers from the accepted storealone, so narrowing the builder's filter without also changing where the
parameters are read from loses the query string. Both changes together were
tried on this branch and reverted in
527b660, after
run
43069531045
failed 17 of 664 tests across the form body, session id, sequence, letter
case and cloud suites. Making Node match needs the question of what
getanswers for a key set aside settled first, which is a change to
#72's model rather
than to this draft.
CloudRequestEngine.keysAndPropertiesFetchedreadsthis.evidenceKeyFilter.length, which no evidence key filter in Node has, soit is always false. That is true on
rcas well and is untouched here.