Repository navigation
Accept no Evidence for usage sharing, and give it the whole web request - #68
Draft
jwrosewell wants to merge 6 commits into
Draft
jwrosewell wants to merge 6 commits into
jwrosewell wants to merge 6 commits into
Conversation
The Pipeline with usage sharing should accept only its other elements' Evidence, the web integration should add the web request as server.http-request, usage sharing should share a header only that request holds under its rules, the tracker should key on what would be shared, a revalidation's ETag should be the one it sends, and the cloud request engine should not send the web request.
The usage sharing element's filter accepted every header except Cookie, and the Pipeline's filter is the union of its elements' filters, so it accepted every header too. The client-side ETag is a hash of the Evidence in that filter, so a revalidation carrying If-None-Match hashed differently and never got a 304, and the Vary header named every header. - The element accepts no Evidence. EvidenceKeyFilterShareUsage stays as the rules for which of the values it finds are shared. - ShareUsageEvidence.find gives it every Evidence value and every value of the web request when the Evidence holds server.http-request, keeping the Evidence value for a key in both. The tracker keys on the same values, so the session tracking hash is of what would be shared. - The web integration records every value it offers, accepted or not, in a read-only WebRequestEvidence map, added as server.http-request on every request. It is a map rather than the HttpServletRequest because the usage sharing element does not depend on the servlet API. - The cloud request engine sent every Evidence value whose prefix is not header, cookie or query, so it would have sent the web request. It leaves that key out.
…ce element With usage sharing accepting no Evidence, the session id and sequence of a client-side callback reach Evidence only if the sequence element accepts them, as its specification lists. The revalidation test now sends them, as a callback does, and the usage sharing element's session cookie test expects the element to accept nothing.
…a sequence sent as text The sequence element accepted no Evidence and relied on usage sharing accepting query.session-id and query.sequence, so without it every callback started a new session. It now accepts both, as the specification lists, and reads a sequence that arrives from the query string as text, which the cast to int failed on.
A pipeline of the sequence, set headers, JSON builder, JavaScript builder and usage sharing elements should accept neither If-None-Match nor a header no element uses, and still accept the keys its elements read.
They track two requests that should count as one repeat, so the class's 50 millisecond interval let a slow runner track both.
This was referenced Nov 8, 2026
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.
Follows the draft specification change 51Degrees/specifications#99, "Usage sharing accepts no Evidence, and reads the web request instead".
The problem
The usage sharing element's filter,
EvidenceKeyFilterShareUsage, accepted every header exceptCookie, and the Pipeline's filter is the union of its elements' filters, so it accepted every header too. The client-side endpoints' ETag is a hash of the Evidence in that filter, so a revalidation carryingIf-None-Matchhashed differently and never got a 304, and the Vary header named every header.What changed
ShareUsageBase.getEvidenceKeyFilterreturns an empty list.EvidenceKeyFilterShareUsageis unchanged as the rules for which values are shared (never-shared keys, the cookie header, the block list,51D_cookies and session values, the query string allow list).ShareUsageEvidence.findgives every Evidence value and, when the Evidence holdsserver.http-request, every value of the web request, keyed without regard to case. A key in both keeps the Evidence value, andserver.http-requestitself is never one of the values. The ignore-data filter and the values shared both read these.ShareUsageTrackerkeys its cache on the found values its filter includes, rather than on the Evidence alone, which no longer holds headers no other element accepts. It keeps its own cache for that, because the base class's cache is keyed byFlowData.generateKey, and changing the base class would releasepipeline.engines.server.http-requeston every request.WebRequestEvidenceServiceCorerecords every value it offers, accepted or not, in a newWebRequestEvidence, and adds it directly rather than through the accepted-Evidence check. That covers headers, cookies, query string and form parameters, session values,server.client-ip,header.protocol, and the Spring service'sserver.contextroot, so usage sharing sees the same values as before.header.<name>,cookie.<name>,query.<name>,session.<name>), not theHttpServletRequest.pipeline.engines.fiftyone, where the usage sharing element lives, does not depend on the servlet API. The values are copied so that they stay the same once the container recycles the request. ItstoStringgives a count rather than the values, so a debug log of Evidence, or an example page that lists all Evidence, does not print every header and cookie.CloudRequestEngineDefault.getFormDatasends every Evidence value whose prefix is notheader,cookieorquery, whether or not the engine accepts it, so it would have sentserver.http-requestto the cloud service ashttp-request. It now leaves that key out. The key is repeated as a private constant there, becausepipeline.cloudrequestenginedoes not depend onpipeline.engines.fiftyone.SequenceElementaccepted no Evidence and relied on usage sharing acceptingquery.session-idandquery.sequence. With usage sharing accepting nothing, every client-side callback would start a new session. It now accepts both, as its specification lists. It also reads a sequence that arrives as text, which the cast tointfailed on, as the red run below shows happens onrctoday.How to verify
Both runs are on
ubuntu-latestwith JDK 17, with the JavaScript templates submodule checked out, runningShareUsage*Tests,EvidenceKeyFilterShareUsageTests,CloudRequestEngineTests,UsageSharingEvidenceTests,ClientsidePropertyServiceTests,PipelineFilterTests,UACHServiceCoreTestsandFiftyOneInterceptorTeststhroughmvn -f pipeline/pom.xml -pl pipeline.engines.fiftyone,pipeline.cloudrequestengine,pipeline.web.packages/pipeline.web,pipeline.web.packages/pipeline.web.mvc -am test.rcat 2299507 plus the three test commits (8977586, a645d1f, 2b1e52a): run 42424560368,TOTALS {'passed': 76, 'failure': 7, 'skipped': 1, 'error': 2}. The skip isShareUsageBuilder_IgnoreData_InvalidFilter, which is@Disabled. The run that printed the red errors' causes in full, without the third test commit, is run 42419662468.TOTALS {'passed': 85, 'skipped': 1}.TOTALS {'passed': 85, 'skipped': 1}. It merges with Share a repeated evidence set at most once in each interval #61 without conflict.The brief's four checks, each red on
rcunless noted.1. The Pipeline does not accept
header.if-none-matchor an arbitrary header.ShareUsageAcceptedEvidenceTestsbuilds a Pipeline of the test engineEmptyEngineand the usage sharing element.2. With the web request supplied, a header no other element accepts is still shared, and the cookie header and a blocked header are not.
ShareUsageElement_WebRequest_SharesWhatNoOtherElementAcceptsalso checks thatUser-Agent, in both places, is shared once with the Evidence value, that a51D_cookie and the client IP from the request are shared, and that the web request itself is not.The web integration side is
addEvidenceFromRequest_AddsTheWebRequest.3. A revalidation's ETag is the one it sends, so it gets a 304.
Revalidation_SameETag_Gets304serves the JSON endpoint from a real Pipeline of the sequence, JSON builder and usage sharing elements, with a request carrying the session id and sequence as a callback does, then sendsIf-None-Matchwith the ETag it got. Onrcthe Pipeline cannot process that request at all.An earlier red run with the first test commit only, before the requests carried a session id and sequence (run 42418221825), failed on the ETag itself:
The revalidation's ETag differs from the one it sends. expected:<113796253> but was:<1812379018>.Callback_SessionAndSequence_CarriedOnfails onrcwith the sameClassCastException, and passes here with the session id kept and the sequence at 3.4. Without a web request, Evidence added by hand is shared as before. The existing
ShareUsageElementTests, which add Evidence by hand, pass unchanged on both sides, apart from the one below. Two new tracker tests show the session tracking hash.ShareUsageTracker_WebRequest_SessionCookieTrackedpasses on both sides, andShareUsageTracker_WebRequest_UnsharedValueIgnored, two requests differing only in a cookie that is not shared, fails onrcwithexpected: <1> but was: <2>.ShareUsageElement_TrackSession_SessionCookieasserted that the element accepted the session cookie. It now asserts that it accepts nothing, and still checks that the session cookie keeps two sessions apart and is never sent, so it is red onrcby design.CloudRequestEngineTests.getFormData_LeavesOutTheWebRequestfails onrcwithThe web request was sent to the cloud service.Every element's accepted Evidence
Under #99's rule that an element accepts only the Evidence that can change its results, I read the filter of every flow element in the repository's main code. Usage sharing was the only one that accepted every header, and the sequence element relied on it. Every other filter is an
EvidenceKeyFilterWhitelist, which matches only the keys listed, so none accepts every key or every header.ShareUsageElementSequenceElementquery.session-id,query.sequence(this pull request)SetHeadersElement,JsonBuilderElementJavaScriptBuilderElementheader.Host,header.protocol,query.fod-js-object-name,query.fod-js-enable-cookies,server.contextrootCloudRequestEngineDefaulthttps://cloud.51degrees.com/api/v4/evidencekeys. On 7 November that list was 138 named keys, with noIf-None-Matchand no wildcardDeviceDetectionCloudEngine,IPIntelligenceCloudEngine,GeoLocationCloudEngine, the hardware profile engines (PropertyKeyedCloudEngineBase)DeviceDetectionHashEngine,IPIntelligenceOnPremiseEngineengine.getKeys())TranslationEngine,CountriesTranslationEngine,CountryCodeTranslationEnginequery.translation,query.accept-language,header.accept-languageParallelElements, the PipelineSimpleFlowElement,SimpleOnPremiseEngine,SimpleClientSideElement,SimpleCloudEnginedate-of-birth,date-of-birth,cookie.date-of-birth, nothingWebPipeline_AcceptsOnlyWhatItsElementsUsebuilds a Pipeline of the sequence, set headers, JSON builder, JavaScript builder and usage sharing elements, and checks that it accepts neitherheader.if-none-matchnorheader.x-testnorserver.http-request, while it still acceptsheader.hostandquery.session-id. Onrcit fails withIf-None-Match is accepted., and here it passes.Other drafts touching the same files
ShareUsageTracker.javaandShareUsageTrackerTests.java.ShareUsageBase.java.git merge-treeof this branch with each of them merges without conflict. Cache the client-side endpoints' answers as the specification and RFC 9110 ask #67 changesClientsidePropertyServiceCore.java, which this does not, and the revalidation test here sends a GET, so it holds under Cache the client-side endpoints' answers as the specification and RFC 9110 ask #67's 412 rule for other methods.The JavaScript file and the session id
A request for
51Degrees.core.jscarries no session id, so the sequence element creates one and writes it into Evidence, where, as an accepted key, it is part of the ETag, and the file gets a new ETag every time. James Rosewell decided on 7 November 2026 that this is correct, because the session id is rightly part of the ETag and the result changes as the sequence increases, so nothing changes there. A callback carries its session id and sequence, so its revalidation gets a 304.For James to decide
pipeline.cloudrequestengine, which withpipeline.developerexamples.cloudengineis not in the 29 packages Monday's plan already releases. Without it, the cloud service would receive every header and cookie of each request as one form field.51D_that only usage sharing accepted no longer reach Evidence, so the JavaScript builder no longer echoes them into its callback URL. Usage sharing still shares them from the web request.The full pull request build, common-ci's
nightly-pull-request.ymlrun as a dry run from a temporary branch, built 60b5550, this branch's head, merged withrc(run 42455459082). All 15 build and test jobs passed on the second attempt, on Ubuntu x64 with Java 8, 11, 17 and 21, Ubuntu arm64 with Java 17, Windows with Java 8, 11, 17 and 21, and macOS arm64 and x64 with Java 11, 17 and 21. The first attempt failed only inMacOS_X64_Java_11, inDataUpdateServiceTests.DataUpdateService_UpdateFromFile_FileUpdated, a race in the data update service that this change does not touch (#70), andMacOS_X64_Java_21was cancelled with it. An earlier full build of 2b1e52a, before the tracker interval commit, passed all 15 (run 42424866504).What remains