From eb2a2e5e2fd9ebe81de1323e6645d1b503983a32 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 17:50:54 +0000 Subject: [PATCH 1/6] Test the usage sharing record attributes --- .../tests/test_share_usage_record.py | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py new file mode 100644 index 0000000..269c56b --- /dev/null +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py @@ -0,0 +1,48 @@ + +""" +How the usage sharing element writes an Evidence value that was changed, as +the Processing section of the usage sharing element specification says. +""" + +import unittest + +from fiftyone_pipeline_engines_fiftyone.share_usage import ( + SHARE_USAGE_MAX_EVIDENCE_LENGTH, ShareUsage, ShareUsageData) + + +class ShareUsageRecordTests(unittest.TestCase): + + def record(self, key, value): + element = ShareUsage(requested_package_size=1000) + self.addCleanup(element.close) + element.constant_xml = "test" + data = ShareUsageData() + data.session_id = "session" + data.sequence = 1 + data.try_add(key, value) + element.add_to_share_usage(data) + return element.share_data[0] + + def testUnchangedValueHasNoAttributes(self): + xml = self.record("header.user-agent", "Mozilla/5.0") + self.assertIn('
Mozilla/5.0
', xml) + + def testInvalidCharacterIsMarkedEscaped(self): + xml = self.record("header.user-agent", "a\x01b") + self.assertIn(' escaped="true"', xml) + self.assertNotIn("replaced", xml) + + def testLongValueIsMarkedTruncated(self): + xml = self.record( + "header.user-agent", "a" * (SHARE_USAGE_MAX_EVIDENCE_LENGTH + 1)) + self.assertIn(' truncated="true"', xml) + self.assertNotIn("escaped", xml) + + def testChangedValueIsWrittenWithoutCdataOrBadSchema(self): + xml = self.record("header.user-agent", "a\x01b") + self.assertNotIn("CDATA", xml) + self.assertNotIn("BadSchema", xml) + + +if __name__ == "__main__": + unittest.main() From b34c488b3535a8abbc1019ef8c72ceda8e70a54a Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 17:52:47 +0000 Subject: [PATCH 2/6] Mark a value with invalid characters as escaped in the usage sharing record --- .../src/fiftyone_pipeline_engines_fiftyone/share_usage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py b/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py index 210dfb0..2315b8b 100644 --- a/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py @@ -285,7 +285,7 @@ def add_to_share_usage(self, data): xml += f"<{entry_key}" # Write any attributes if replaced_string.replaced: - xml += ' replaced="true"' + xml += ' escaped="true"' if replaced_string.truncated: xml += ' truncated="true"' # End the start element From 46579e11060c1807874a92323890d23cc8e934d3 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 17:55:20 +0000 Subject: [PATCH 3/6] Expect escaped in the usage sharing invalid character test --- pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py index 4f1bea4..f68efad 100644 --- a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py @@ -317,7 +317,7 @@ def test_invalid_char(self): self.assertEqual(len(app.received), 1) self.assertNotIn("ƌ", app.received[0]) self.assertIn(replacementChar, app.received[0]) - self.assertIn(f"
1{replacementChar}2
", app.received[0]) + self.assertIn(f"
1{replacementChar}2
", app.received[0]) def test_truncated_value(self): """! From 6dcef57e9ea09dcaea9ff33dd5711ba53f3fc526 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 20:31:46 +0000 Subject: [PATCH 4/6] Test the usage sharing record escapes and limit --- .../tests/test_share_usage_record.py | 80 +++++++++++++++---- .../tests/test_usage.py | 13 ++- 2 files changed, 71 insertions(+), 22 deletions(-) diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py index 269c56b..5a12a11 100644 --- a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py @@ -1,7 +1,8 @@ """ How the usage sharing element writes an Evidence value that was changed, as -the Processing section of the usage sharing element specification says. +the Processing section of the usage sharing element specification says, with +the invalid character and length rules of specifications pull request 122. """ import unittest @@ -9,10 +10,12 @@ from fiftyone_pipeline_engines_fiftyone.share_usage import ( SHARE_USAGE_MAX_EVIDENCE_LENGTH, ShareUsage, ShareUsageData) +BS = chr(92) + class ShareUsageRecordTests(unittest.TestCase): - def record(self, key, value): + def record(self, value, key="header.user-agent"): element = ShareUsage(requested_package_size=1000) self.addCleanup(element.close) element.constant_xml = "test" @@ -23,25 +26,74 @@ def record(self, key, value): element.add_to_share_usage(data) return element.share_data[0] - def testUnchangedValueHasNoAttributes(self): - xml = self.record("header.user-agent", "Mozilla/5.0") + def test_limit_is_ten_thousand(self): + self.assertEqual(10000, SHARE_USAGE_MAX_EVIDENCE_LENGTH) + + def test_unchanged_value_has_no_attributes(self): + xml = self.record("Mozilla/5.0") self.assertIn('
Mozilla/5.0
', xml) - def testInvalidCharacterIsMarkedEscaped(self): - xml = self.record("header.user-agent", "a\x01b") - self.assertIn(' escaped="true"', xml) - self.assertNotIn("replaced", xml) + def test_invalid_character_is_written_as_hex_and_marked_escaped(self): + xml = self.record("a" + chr(0x1f) + "b") + self.assertIn( + '
a' + BS + 'x001fb
', + xml) - def testLongValueIsMarkedTruncated(self): - xml = self.record( - "header.user-agent", "a" * (SHARE_USAGE_MAX_EVIDENCE_LENGTH + 1)) - self.assertIn(' truncated="true"', xml) + def test_other_invalid_characters(self): + for code, text in ((0x0, "0000"), (0x8, "0008"), (0xb, "000b"), + (0xc, "000c"), (0xe, "000e"), (0xd800, "d800"), + (0xdfff, "dfff"), (0xfffe, "fffe"), + (0xffff, "ffff")): + xml = self.record("x" + chr(code) + "y") + self.assertIn("x" + BS + "x" + text + "y", xml, hex(code)) + self.assertIn(' escaped="true"', xml, hex(code)) + + def test_characters_xml_allows_are_kept(self): + value = "t" + chr(9) + chr(10) + chr(13) + chr(0x18c) + chr(0xfffd) + xml = self.record(value) + self.assertIn(">" + value + "<", xml) + self.assertNotIn("escaped", xml) + + def test_markup_characters_are_still_entities(self): + xml = self.record("a&bd\"e'") + self.assertIn(">a&b<c>d"e'<", xml) self.assertNotIn("escaped", xml) - def testChangedValueIsWrittenWithoutCdataOrBadSchema(self): - xml = self.record("header.user-agent", "a\x01b") + def test_value_of_the_limit_is_not_truncated(self): + xml = self.record("a" * 10000) + self.assertIn(">" + "a" * 10000 + "<", xml) + self.assertNotIn("truncated", xml) + + def test_longer_value_is_cut_and_marked_truncated(self): + xml = self.record("a" * 10001) + self.assertIn( + ' truncated="true">' + "a" * 10000 + "", xml) + + def test_length_is_counted_before_markup_characters_are_written(self): + xml = self.record("&" * 10000) + self.assertNotIn("truncated", xml) + + def test_character_outside_the_basic_plane_is_never_split(self): + emoji = chr(0x1f600) + xml = self.record("a" * 9999 + emoji + "tail") + self.assertIn( + ' truncated="true">' + "a" * 9999 + emoji + "", xml) + xml = self.record("a" * 9998 + emoji + emoji + "z") + self.assertIn( + ' truncated="true">' + "a" * 9998 + emoji + emoji + "", + xml) + self.assertNotIn("escaped", xml) + + def test_invalid_character_in_a_long_value_sets_both(self): + xml = self.record(chr(1) + "a" * 10000) + self.assertIn(' escaped="true"', xml) + self.assertIn(' truncated="true"', xml) + + def test_changed_value_is_written_without_cdata_or_bad_schema(self): + xml = self.record("a" + chr(1) + "b") self.assertNotIn("CDATA", xml) self.assertNotIn("BadSchema", xml) + self.assertNotIn("replaced", xml) if __name__ == "__main__": diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py index f68efad..5afaaee 100644 --- a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py @@ -304,20 +304,17 @@ def test_invalid_char(self): requested_package_size=1, endpoint=testEndpoint) - replacementChar = chr(int("0xFFFD", 16)) - pipeline = (PipelineBuilder())\ .add(usage)\ .build() flowdata = pipeline.create_flowdata() - flowdata.evidence.add("header.user-agent", "1ƌ2") + flowdata.evidence.add("header.user-agent", "1" + chr(0x1f) + "2") flowdata.process() self.waitForUsageThreads() self.assertEqual(len(app.received), 1) - self.assertNotIn("ƌ", app.received[0]) - self.assertIn(replacementChar, app.received[0]) - self.assertIn(f"
1{replacementChar}2
", app.received[0]) + self.assertNotIn(chr(0x1f), app.received[0]) + self.assertIn('
1\\x001f2
', app.received[0]) def test_truncated_value(self): """! @@ -328,7 +325,7 @@ def test_truncated_value(self): requested_package_size=1, endpoint=testEndpoint) - ua = "X" * 1000 + ua = "X" * 10001 pipeline = (PipelineBuilder())\ .add(usage)\ @@ -341,7 +338,7 @@ def test_truncated_value(self): self.assertEqual(len(app.received), 1) self.assertIn("truncated=\"true\"", app.received[0]) self.assertNotIn(ua, app.received[0]) - self.assertIn(f'
{"X" * 512}
', app.received[0]) + self.assertIn(f'
{"X" * 10000}
', app.received[0]) def test_send_more_than_once(self): """! From ef68190c7e7abc290a4d6a9fce904911c3694b3d Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 20:32:43 +0000 Subject: [PATCH 5/6] Write invalid characters as backslash x and four hex digits, and cut 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. --- .../share_usage.py | 66 ++++++++++--------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py b/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py index 2315b8b..3650c27 100644 --- a/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/src/fiftyone_pipeline_engines_fiftyone/share_usage.py @@ -37,7 +37,7 @@ # The maximum length of a piece of evidence's value which can be # added to the usage data being sent. -SHARE_USAGE_MAX_EVIDENCE_LENGTH = 512 +SHARE_USAGE_MAX_EVIDENCE_LENGTH = 10000 SHARE_USAGE_VERSION = '1.1' @@ -284,7 +284,7 @@ def add_to_share_usage(self, data): else: xml += f"<{entry_key}" # Write any attributes - if replaced_string.replaced: + if replaced_string.escaped: xml += ' escaped="true"' if replaced_string.truncated: xml += ' truncated="true"' @@ -361,49 +361,51 @@ def get_data_from_evidence(self, flow_data): return data -# a set of valid XML character values (ignoring valid controls x09, x0a, x0d, x85) -VALID_XML_CHARS = [*range(int("0x20", 16), int("0x7F", 16))] -VALID_XML_CHARS.extend(range(int("0x40", 16), int("0x100", 16))) +# The characters that XML 1.0 section 2.2 allows, as ranges of code points. +VALID_XML_RANGES = ( + (0x9, 0xA), (0xD, 0xD), (0x20, 0xD7FF), (0xE000, 0xFFFD), + (0x10000, 0x10FFFF)) -def get_is_valid_char_map(): - maxChar = int('0x100', 16) - isValidChar = {} - for c in range(maxChar): - isValidChar[c] = c in VALID_XML_CHARS - return isValidChar +# The characters that are written as entities in element text. +XML_ENTITIES = { + '&': '&', + '"': '"', + "'": ''', + '<': '<', + '>': '>', +} -# an array describing whether a character value is valid -IS_VALID_XML_CHAR = get_is_valid_char_map() + +def is_valid_xml_char(code): + return any(low <= code <= high for low, high in VALID_XML_RANGES) class ReplacedString: """! - replace characters that cause problems in XML with the "Replacement character" + Makes an Evidence value fit in the usage sharing XML. The value is cut to + SHARE_USAGE_MAX_EVIDENCE_LENGTH characters (code points, so a character is + never split), a character XML 1.0 does not allow is written as a backslash, + "x" and its code in four lower-case hexadecimal digits, and the markup + characters are written as entities. `escaped` and `truncated` say what + changed. """ def __init__(self, text): self.result = "" - self.replaced = False + self.escaped = False self.truncated = False if text: - escapedText = text\ - .replace('&', '&')\ - .replace('"', '"')\ - .replace('\'', ''')\ - .replace('<', '<')\ - .replace('>', '>') - - length = min(len(escapedText), SHARE_USAGE_MAX_EVIDENCE_LENGTH) - self.truncated = len(escapedText) > SHARE_USAGE_MAX_EVIDENCE_LENGTH - self.result = "".join(map(self.map_char, list(escapedText[0:length]))) - + if not isinstance(text, str): + text = str(text) + self.truncated = len(text) > SHARE_USAGE_MAX_EVIDENCE_LENGTH + self.result = "".join( + map(self.map_char, text[0:SHARE_USAGE_MAX_EVIDENCE_LENGTH])) def map_char(self, char): - charV = ord(char) - if charV < len(IS_VALID_XML_CHAR) and IS_VALID_XML_CHAR[charV]: - return char - else: - self.replaced = True - return chr(int("0xFFFD", 16)) + code = ord(char) + if not is_valid_xml_char(code): + self.escaped = True + return "\\x%04x" % code + return XML_ENTITIES.get(char, char) class ShareUsageData: """! From b6ebdd603c04f66c5c823504ef30197103e62d51 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 20:43:19 +0000 Subject: [PATCH 6/6] Test that escapes do not count towards the usage sharing length limit --- .../tests/test_share_usage_record.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py index 5a12a11..30b2b0b 100644 --- a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py @@ -73,6 +73,12 @@ def test_length_is_counted_before_markup_characters_are_written(self): xml = self.record("&" * 10000) self.assertNotIn("truncated", xml) + def test_escapes_do_not_count_towards_the_limit(self): + xml = self.record(chr(1) + "a" * 9999) + self.assertIn(' escaped="true"', xml) + self.assertNotIn("truncated", xml) + self.assertIn(chr(92) + "x0001" + "a" * 9999 + "<", xml) + def test_character_outside_the_basic_plane_is_never_split(self): emoji = chr(0x1f600) xml = self.record("a" * 9999 + emoji + "tail")