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 cb96e9c8..59bdf9bd 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,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, 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", @@ -71,7 +74,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): 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"],