From 4e8a67068a796b1b05b2da20e1f5cff63b9c911e Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 20:45:34 +0000 Subject: [PATCH 1/3] Test that the object name refuses the Window properties the specification lists --- .../test_javascriptbuilder_object_name.py | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_object_name.py b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_object_name.py index 8342461f..c6e2e400 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_object_name.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_object_name.py @@ -49,6 +49,24 @@ from .classes.memorylogger import MemoryLogger from .test_javascriptbuilder import TestEngine +# The names the specification refuses that are not reserved words of the +# language, being eval, arguments, and the properties of the browser's Window +# object that a top level var cannot replace or that navigate the page away. +REFUSED_NAMES = [ + "eval", "arguments", "window", "document", "top", "name", "status", + "history", "navigator", "closed", "location", "frameElement", + "isSecureContext", "crossOriginIsolated", "trustedTypes", "crypto", + "indexedDB", "localStorage", "sessionStorage", "caches", "cookieStore", + "speechSynthesis", "styleMedia", "crashReport", "credentialless", + "customElements", "documentPictureInPicture", "fence", "launchQueue", + "originAgentCluster"] + +# Window properties that a top level var can replace, so the names stay +# allowed, and near misses of refused names. +ALLOWED_NAMES = [ + "self", "frames", "parent", "opener", "length", "windows", "Window", + "cachesX"] + NODE = shutil.which("node") HARNESS = os.path.join(os.path.dirname(__file__), "object_name_harness.js") @@ -205,6 +223,50 @@ def test_invalid_name_from_evidence_is_ignored( if check_text: self.assertNotIn(name, warnings[0]) + @parameterized.expand([[name] for name in REFUSED_NAMES]) + def test_refused_window_name_from_evidence_is_ignored(self, name): + """A requested name the specification refuses is not declared. The + configured name is used, the script parses and runs, and a warning + is logged.""" + + logger = MemoryLogger("warning") + script = _render( + {"minify": False}, + {Constants.EVIDENCE_OBJECT_NAME: name}, + logger) + + self.assert_uses_name(script, "fod") + self.assert_parses(script) + + result = self.run_script(script, "fod") + self.assertIsNone(result["error"]) + self.assertTrue(result["exists"]) + self.assertEqual(["fod"], result["globals"]) + + warnings = [ + entry["message"] for entry in logger.memory_log + if entry["level"] == "warning" + and Constants.EVIDENCE_OBJECT_NAME in entry["message"]] + self.assertEqual(1, len(warnings), str(logger.memory_log)) + + @parameterized.expand([[name] for name in ALLOWED_NAMES]) + def test_allowed_window_name_from_evidence_is_used(self, name): + """A name the specification allows is declared and the script + parses.""" + + logger = MemoryLogger("warning") + script = _render( + {"minify": False}, + {Constants.EVIDENCE_OBJECT_NAME: name}, + logger) + + self.assert_uses_name(script, name) + self.assert_parses(script) + self.assertEqual([], [ + entry for entry in logger.memory_log + if entry["level"] == "warning" + and Constants.EVIDENCE_OBJECT_NAME in entry["message"]]) + def test_invalid_name_from_evidence_uses_configured_name(self): """The fallback is the configured name, not always 'fod'.""" @@ -239,6 +301,19 @@ def test_invalid_configured_name_is_refused(self, _, name): with self.assertRaises(ValueError): JavascriptBuilderElement({"obj_name": name}) + @parameterized.expand([[name] for name in REFUSED_NAMES]) + def test_refused_window_name_is_refused(self, name): + """A name the specification refuses is refused when the builder is + created.""" + + with self.assertRaises(ValueError): + JavascriptBuilderElement({"obj_name": name}) + + @parameterized.expand([[name] for name in ALLOWED_NAMES]) + def test_allowed_window_name_is_accepted(self, name): + element = JavascriptBuilderElement({"obj_name": name}) + self.assertEqual(name, element.settings["_objName"]) + @parameterized.expand([ ["plain", "myFod"], ["underscore", "_fod"], From 811e68328129ab74dae72f84e45a1deb2af5d14c Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 20:45:34 +0000 Subject: [PATCH 2/3] Refuse the Window property names the specification lists as the object name --- .../javascriptbuilder.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py index bdb85c50..5352865e 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py @@ -60,9 +60,11 @@ # Names the object cannot have. These are the reserved words of the language, # including those reserved only in strict mode and the literals null, true -# and false, plus the three global values a top level var cannot replace, so -# the object would silently never be created, and the name of the constructor -# the script itself defines. +# and false, plus the three global values a top level var cannot replace, +# eval, the properties of the browser's Window object that a top level var +# cannot replace or that navigate the page away, so the object would +# silently never be created, and the name of the constructor the script +# itself defines. RESERVED_WORDS = frozenset([ "await", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", @@ -71,7 +73,16 @@ "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "var", "void", "while", "with", "yield", - "Infinity", "NaN", "undefined", "fiftyoneDegreesManager"]) + "Infinity", "NaN", "undefined", + "eval", "arguments", + "window", "document", "top", "name", "status", "history", "navigator", + "closed", "location", "frameElement", "isSecureContext", + "crossOriginIsolated", "trustedTypes", "crypto", "indexedDB", + "localStorage", "sessionStorage", "caches", "cookieStore", + "speechSynthesis", "styleMedia", "crashReport", "credentialless", + "customElements", "documentPictureInPicture", "fence", "launchQueue", + "originAgentCluster", + "fiftyoneDegreesManager"]) def is_valid_object_name(name): From f03e52ddea919e7d02272378ec6d3016d1bb948d Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 09:59:35 +0000 Subject: [PATCH 3/3] Say in the object name comment why eval and arguments are refused --- .../src/fiftyone_pipeline_core/javascriptbuilder.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py index 5352865e..ea6c2c7f 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py @@ -60,11 +60,12 @@ # Names the object cannot have. These are the reserved words of the language, # including those reserved only in strict mode and the literals null, true -# and false, plus the three global values a top level var cannot replace, -# eval, the properties of the browser's Window object that a top level var -# cannot replace or that navigate the page away, so the object would -# silently never be created, and the name of the constructor the script -# itself defines. +# and false. Then eval and arguments, which strict mode code cannot declare, +# and eval would also replace the page's own eval function. Then the three +# global values and the properties of the browser's Window object that a +# top level var cannot replace, so the object would never be created, and +# location, which would navigate the page away. Last, the name of the +# constructor the script itself defines. RESERVED_WORDS = frozenset([ "await", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export",