Repository navigation
Mark usage sharing values with replaced characters as escaped - #54
Merged
Merged
Conversation
The Usage Sharing Element page names the attribute escaped, where the Node element wrote replaced.
… and the 10,000 character limit
…t values at 10,000 characters The Usage Sharing Element page marks a changed value escaped or truncated. A character XML 1.0 does not allow is now written as a backslash, x and the four lower case hexadecimal digits of its code, as the .NET and Go elements do, and the limit is 10,000 characters, cut from the original value.
…idence names Evidence that is not a string must be shared as its text rather than as an empty value. The session id and the client IP come from the request, and an evidence name is written in an attribute, so the XML characters in each must be escaped. A request with no client IP must have no ClientIP element rather than one holding undefined.
Evidence that is not a string is shared as its text, as .NET writes it, where a number became an empty value. The session id, the sequence, the client IP and the Name attribute go through the same escaping as evidence values, because the session id and the client IP come from the request, so a value holding < or & made the whole batch invalid XML. A request with no client IP now has no ClientIP element, where it had one holding undefined, as Java leaves it out. The 10,000 limit's constant names its unit, code points.
The specification's record marks only the bracketed items as optional, and ClientIP and ServerIP have no brackets, so a request with no client IP must have an empty ClientIP element, as .NET writes it, and every record must have a ServerIP element. The ServerIP tests give the server an IPv4 and an IPv6 address, an IPv6 address alone, and loopback addresses alone.
The specification's record marks only the bracketed items as optional, and ClientIP and ServerIP have no brackets. A request with no client IP now has an empty ClientIP element, as .NET writes it, where it had none. Every record now has the ServerIP element, which .NET and Java write and Node did not. Like .NET, it is the first address of this server that is not a loopback address, an IPv4 address before an IPv6 one, or empty where there is none.
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.
Brings the usage sharing record in line with the Processing section of the Usage Sharing Element page and the record decisions in specifications#122 (merged into
rc). Both were read in full onrc.The page says that "escaped - Indicates that the original evidence value included invalid characters, which have been replaced with escaped ones." and "truncated - Indicates that the original evidence value was too long." The record on
rcshows them as<PREFIX [escaped="true"] [truncated="true"] Name="FIELD">VALUE</PREFIX>.What changed
escaped="true", whereshareUsage.jswrotereplaced="true".xand the four lower case hexadecimal digits of its code, for example\x001f, as the .NET and Go elements do. The code wrote U+FFFD, and its list of allowed characters was wrong (it treated tab and line feed as invalid, and every character above U+00FF as invalid). XML 1.0 allows tab, line feed, carriage return, U+0020 to U+D7FF, U+E000 to U+FFFD and U+10000 to U+10FFFF, and a surrogate pair is one allowed character. A lone surrogate is written as\xand its four digits.truncated="true". The constant isSHARE_USAGE_MAX_EVIDENCE_CODE_POINTS, so its name gives the unit.SessionId,Sequence,ClientIPand theNameattribute go through the same escaping as evidence values (addToShareUsage). The session id comes fromquery.session-id, which the browser sends, and the client IP from the request, so a value holding<or&made the whole batch invalid XML.<ClientIP></ClientIP>, where it had<ClientIP>undefined</ClientIP>, becauseShareUsageDatasetclientIpand the writer readclientIP. The record says "Things in [square brackets] are optional", and<ClientIP>IP_ADDRESS</ClientIP>has no brackets. .NET writes the element empty when there is no value (WriteElementString("ClientIP", data.ClientIP)).<ServerIP>, which the record lists without brackets and which .NET (ShareUsageBase.cs,HostAddress) and Java (getHostAddress) write and Node did not. As in .NET,serverAddressgives the first address of this server that is not a loopback address, an IPv4 address before an IPv6 one, or an empty string where there is none. Node reads the addresses withos.networkInterfaces(), where .NET resolves the host name, and the value is worked out once with the rest of the fixed part of the record.ReplacedStringwrites a value that is not a string as its text, andundefinedandnullas nothing. Evidence values already reach it as text, becausetryAddToDatacallsvalue.toString(), so the tests that a number, zero andfalseare shared as their text pass on the old code too, as guards. The change matters forSequence, which is a number and now goes throughReplacedString.Open draft #45 changes
shareUsage.jsand its tests in the evidence handling, and neither it nor #36 touches the lines changed here. The web integration draft #53 adds one test toshareUsage.test.jsaway from the lines changed here.Tests
The invalid character, long value, emoji at the limit, escapes-do-not-count and escaped-and-truncated cases are in
tests/shareUsage.test.js. The test server decoded each chunk of the body on its own and split an emoji in two, so it now decodes as the chunks arrive. New since the last review are the XML characters in the session id, the client IP and an evidence name, an emptyClientIPfor a request with no client IP,ServerIPfor a server with IPv4 and IPv6 addresses, with IPv6 alone and with loopback addresses alone, and a number, zero andfalseas evidence.\xform and the 10,000 limit (9 of 29 fail): https://github.com/51Degrees/51degrees-node/actions/runs/426932336073de4c0b(3 of 48 fail). The session id was written as<SessionId>a<b&c"d</SessionId>, the name asName="x"y&z<", and a request with no client IP had<ClientIP>undefined</ClientIP>. The number, zero andfalsetests pass, as above: https://github.com/51Degrees/51degrees-node/actions/runs/42796755603833b56a(48 passed): https://github.com/51Degrees/51degrees-node/actions/runs/42796772772ClientIPandServerIPtests on833b56a(4 of 51 fail). The record had no<ClientIP></ClientIP>and no<ServerIP>: https://github.com/51Degrees/51degrees-node/actions/runs/42803236764160d1c6(51 passed): https://github.com/51Degrees/51degrees-node/actions/runs/42803254512160d1c6. Configure and the Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 pass, Configure printed "Left out 2 macOS entries", and all 7Setting 'PullRequestSha'lines are160d1c6aff57992cc95ce5379172ccd2486e8735: https://github.com/51Degrees/51degrees-node/actions/runs/42829954329macOS 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.
Not done here
An evidence key with no prefix, such as
foorather thanheader.foo, is written with its name as the element name, and an element name cannot be escaped. That is unchanged here. The elements are also written in a different order from the record, the fixed part first, as .NET writes its fixed part afterClientIPrather than in the record's order. Nothing else on the page differs from the Node code that was checked.