Skip to content

Mark a usage sharing value with invalid characters as escaped, as the specification says - #71

Draft
jwrosewell wants to merge 6 commits into
rcfrom
fix/usage-sharing-record
Draft

jwrosewell wants to merge 6 commits into
rcfrom
fix/usage-sharing-record

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Problem

For the list of gaps between the pipeline libraries and the specification (the Java list pipeline/Gaps-vs-spec.md and the .NET issues #23 to #29).

The usage sharing record differs from the usage sharing element specification, Processing section in three ways.

  1. A value with invalid characters is marked replaced="true". The specification defines the attribute as escaped: "escaped - Indicates that the original evidence value included invalid characters, which have been replaced with escaped ones."
  2. An invalid character is replaced by U+FFFD, and characters above U+00FF are treated as invalid though XML allows them. The decision in specifications #122 is that "A character that XML 1.0 does not allow is replaced by the text \x followed by the code of the character as four lower-case hexadecimal digits, for example \x001f, and the element for that Evidence entry is marked escaped="true"."
  3. A value is cut at 512 characters, counted after the markup characters were written as entities. The same decision says "A value longer than 10,000 characters is cut to 10,000 characters and the element for that Evidence entry is marked truncated="true"." The limit counts code points and never splits a character.

Python writes no CDATA and no BadSchema element, which matches the specification, and a test asserts both stay absent. This does not follow specifications#65 or python#8.

What changed

All changes are in pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py.

  • The attribute is escaped="true".
  • ReplacedString checks each character against the XML 1.0 Char production (tab, line feed, carriage return, U+0020 to U+D7FF, U+E000 to U+FFFD and U+10000 to U+10FFFF). A character outside it is written as a backslash, x and four lower-case hexadecimal digits, for example \x001f, \xd800 or \xfffe. Tab, line feed, carriage return and characters such as U+018C are now kept as they are.
  • SHARE_USAGE_MAX_EVIDENCE_LENGTH is 10,000. The value is cut before the markup characters are written, so & counts as one character. The code works on str, so the cut is on code points and a character outside the Basic Multilingual Plane is never split. A value that is not a str is converted with str() first. A test puts an emoji at the boundary.
  • The tables of valid characters that only covered U+0000 to U+00FF are removed.

Not changed, for a decision

I read the whole Processing section against the code and found differences that were not part of this request. I left them alone.

  1. DateSent is written as local time with fractions of a second (datetime.now().isoformat()). The specification says "in UTC and formatted as: yyyy-mm-ddThh:nn:ss".
  2. FlowElement holds the class name only (for example SequenceElement). The specification says "the fully qualified class name".
  3. ServerIP is not written.
  4. The element order differs from the specification's snippet (constants first, then SessionId, Sequence, ClientIP, DateSent).
  5. The element logs at level warn when it is registered to too many or no pipelines, and Logger.log accepts only debug, info, warning, error and critical, so that call raises ValueError.

Overlap with other drafts

#53 changes share_usage.py near the top of the file and in add_to_share_usage, and #61 changes it at other lines. My edits are the length constant, one attribute line in add_to_share_usage and the ReplacedString block at the end of the file. The constant may need a small merge with #53's imports and constants hunks. The release draft #67 was left alone.

How it was verified

New tests are in pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py. Two existing tests in test_usage.py (test_invalid_char and test_truncated_value) now expect the new form and limit.

  • Red, tests added to the head with the attribute rename: run 42693856018, 10 usage sharing tests fail. An earlier red for the attribute alone is run 42668603188.
  • Green, head b6ebdd60: run 42695689144, usage sharing 47 passed, engines 9 passed, core 354 passed.
  • Full build of head b6ebdd603c04f66c5c823504ef30197103e62d51: run 42695707525. All 11 non-macOS jobs passed, and every Setting 'PullRequestSha' line read (Configure, Ubuntu_Python_3.12 and Windows_Python_3.14) equals the head. The run was cancelled while its 5 macOS jobs were queued.
    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.

The last release into main merged on 2026-11-04.

…values at 10,000 characters

A character XML 1.0 does not allow is written as its code in four lower-case
hexadecimal digits after a backslash and x, and the element is marked escaped.
A value is cut to 10,000 characters (code points) before the markup characters
are written, and the element is marked truncated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant