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 ec2a657a..d0a1f913 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 @@ -21,6 +21,7 @@ # ********************************************************************* import random +import re import json import gzip import platform @@ -44,7 +45,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' @@ -212,7 +213,7 @@ def get_flow_elements(self): # make up the pipeline so a warning is logged # but otherwise, the system can continue as normal. self._log( - "warn", + "warning", "Share usage element registered " f"to {'too many' if len(self.pipelines) > 0 else 'no'}" " pipelines. Unable to send share usage information.") @@ -299,40 +300,30 @@ def add_to_share_usage(self, data): # --- write variable data # The SessionID used to track a series of requests - xml += f"{data.session_id}" + xml += plain_element("SessionId", data.session_id) # The sequence number of the request in a series of requests. xml += f"{data.sequence}" # The client IP of the request - xml += f"{data.client_ip}" + xml += plain_element("ClientIP", data.client_ip) - # The UTC date/time this entry was written - date = datetime.datetime.now().isoformat() + # The UTC date and time this entry was written, to the second. + date = datetime.datetime.now(datetime.timezone.utc).strftime( + "%Y-%m-%dT%H:%M:%S") xml += f"{date}" - # Write all other evidence data that has been included. + # Write all other evidence data that has been included. The prefix, + # or a key with no prefix, is the element name. An entry whose + # element name is not an XML name is left out, because it would make + # the whole batch unreadable. for category_key, category_value in data.evidence_data.items(): for entry_key, entry_value in category_value.items(): - replaced_string = ReplacedString(entry_value) - # Write start element if len(category_key) > 0: - xml += f"<{category_key} Name=\"{entry_key}\"" - else: - xml += f"<{entry_key}" - # Write any attributes - if replaced_string.replaced: - xml += ' replaced="true"' - if replaced_string.truncated: - xml += ' truncated="true"' - # End the start element - xml += ">" - # Write the value - xml += replaced_string.result - # Write end element - if len(category_key) > 0: - xml += f"" - else: - xml += f"" - + if XML_NAME.match(category_key): + xml += record_element( + category_key, entry_value, entry_key) + elif XML_NAME.match(entry_key): + xml += record_element(entry_key, entry_value) + xml += "" self.share_data.append(xml) @@ -399,49 +390,91 @@ 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)) + +# The characters that are written as entities in element text. +XML_ENTITIES = { + '&': '&', + '"': '"', + "'": ''', + '<': '<', + '>': '>', +} + -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 +def is_valid_xml_char(code): + return any(low <= code <= high for low, high in VALID_XML_RANGES) -# an array describing whether a character value is valid -IS_VALID_XML_CHAR = get_is_valid_char_map() + +# The characters XML 1.0 allows in a name, without the colon, which names a +# namespace. +XML_NAME_START = ( + "A-Z_a-z\u00c0-\u00d6\u00d8-\u00f6\u00f8-\u02ff\u0370-\u037d" + "\u037f-\u1fff\u200c-\u200d\u2070-\u218f\u2c00-\u2fef\u3001-\ud7ff" + "\uf900-\ufdcf\ufdf0-\ufffd\U00010000-\U000effff") +XML_NAME = re.compile( + "[" + XML_NAME_START + "][" + XML_NAME_START + + "\\-.0-9\u00b7\u0300-\u036f\u203f-\u2040]*\\Z") + + +def plain_element(tag, value): + """! + An element of a usage sharing record that is not an Evidence entry, with + its value made to fit the XML by ReplacedString and no attributes, as + the record gives these elements none. + """ + return "<" + tag + ">" + ReplacedString(value).result + "" + + +def record_element(tag, value, name=None): + """! + One element of a usage sharing record. The value, and the Name attribute + when there is one, are made to fit the XML by ReplacedString, and the + element is marked escaped or truncated when either changed. + """ + replaced = ReplacedString(value) + escaped = replaced.escaped + xml = "<" + tag + if name is not None: + replaced_name = ReplacedString(name) + escaped = escaped or replaced_name.escaped + xml += ' Name="' + replaced_name.result + '"' + if escaped: + xml += ' escaped="true"' + if replaced.truncated: + xml += ' truncated="true"' + return xml + ">" + replaced.result + "" 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: """! 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 00000000..93992946 --- /dev/null +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_share_usage_record.py @@ -0,0 +1,172 @@ + +""" +How the usage sharing element writes an Evidence value that was changed, as +the Processing section of the usage sharing element specification says, +including its invalid character and length rules. +""" + +import re +import unittest +import xml.etree.ElementTree as ElementTree + +from fiftyone_pipeline_core.logger import Logger +from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder +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, value, key="header.user-agent", session_id="session", + client_ip=""): + element = ShareUsage(requested_package_size=1000) + self.addCleanup(element.close) + element.constant_xml = "test" + data = ShareUsageData() + data.session_id = session_id + data.client_ip = client_ip + data.sequence = 1 + data.try_add(key, value) + element.add_to_share_usage(data) + return element.share_data[0] + + def test_session_id_is_written_so_the_record_parses(self): + session_id = 'a"): + xml = self.record("a value", key=key) + device = ElementTree.fromstring(xml) + self.assertNotIn("a value", xml, key) + self.assertEqual("session", device.find("SessionId").text, key) + + def test_date_sent_is_utc_to_the_second(self): + device = ElementTree.fromstring(self.record("Mozilla/5.0")) + self.assertRegex( + device.find("DateSent").text, + re.compile(r"\A\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\Z")) + + def test_registered_to_too_many_pipelines_logs_a_warning(self): + logged = [] + + class Recorder(Logger): + def log_internal(self, level, log): + logged.append(level) + + element = ShareUsage(requested_package_size=1000) + self.addCleanup(element.close) + pipelines = [] + for _ in range(2): + builder = PipelineBuilder({"add_javascript_builder": False, + "use_setheader_properties": False}) + builder.add_logger(Recorder("debug")) + pipelines.append(builder.build()) + element.pipelines = pipelines + self.assertEqual([], element.get_flow_elements()) + self.assertEqual(["warning", "warning"], logged) + + 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 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 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 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_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") + 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__": + unittest.main() diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py b/pipeline/fiftyone_pipeline_engines_fiftyone/tests/test_usage.py index 4438b8d3..66e9b4e5 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): """!