diff --git a/pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/JavaScriptBuilderElement.java b/pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/JavaScriptBuilderElement.java index f8d828dc..782c9fa7 100644 --- a/pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/JavaScriptBuilderElement.java +++ b/pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/JavaScriptBuilderElement.java @@ -89,9 +89,11 @@ public class JavaScriptBuilderElement * Words the pattern accepts that cannot be the name of the object. These * are the reserved words of the language, including those reserved only * in strict mode, plus the three global values a top level var cannot - * replace, where the object would silently never be created. The last - * entry is the constructor the script itself defines and calls to create - * the object, so that name would clash with it. + * replace, eval, and the properties of the browser's Window object that a + * top level var cannot replace or that navigate the page away, where the + * object would never be created. The last entry is the constructor the + * script itself defines and calls to create the object, so that name + * would clash with it. */ private static final Set RESERVED_OBJECT_NAMES = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( @@ -103,6 +105,15 @@ public class JavaScriptBuilderElement "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "var", "void", "while", "with", "yield", "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"))); /** diff --git a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptObjectNameTests.java b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptObjectNameTests.java index 4c49c3fa..712bc687 100644 --- a/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptObjectNameTests.java +++ b/pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptObjectNameTests.java @@ -223,6 +223,110 @@ public void ObjectName_InvalidFromConstructor_Refused(String name) { "")); } + /** + * 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. + */ + static final String[] 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. + */ + static final String[] ALLOWED_WINDOW_NAMES = { + "self", "frames", "parent", "opener", "length" + }; + + static Stream refusedNames() { + return Stream.of(REFUSED_NAMES); + } + + static Stream allowedWindowNames() { + return Stream.of(ALLOWED_WINDOW_NAMES); + } + + @ParameterizedTest + @MethodSource("refusedNames") + public void ObjectName_RefusedNameFromEvidence_DefaultUsed(String name) { + JavaScriptBuilderElement element = + new JavaScriptBuilderElementBuilder(loggerFactory).build(); + evidence.put(EVIDENCE_OBJECT_NAME, name); + + String script = render(element); + + assertWorkingObject(script, Constants.DEFAULT_OBJECT_NAME); + assertFalse(Pattern.compile("var " + Pattern.quote(name) + + " = new fiftyoneDegreesManager\\(\\);") + .matcher(withoutParameters(script)).find(), + "the script should not declare '" + name + "'"); + assertEquals(1, count(script, "new fiftyoneDegreesManager()")); + List warnings = warnings(); + assertEquals(1, warnings.size(), String.join("\n", warnings)); + } + + @ParameterizedTest + @MethodSource("refusedNames") + public void ObjectName_RefusedNameFromBuilder_Refused(String name) { + assertThrows( + PipelineConfigurationException.class, + () -> new JavaScriptBuilderElementBuilder(loggerFactory) + .setObjectName(name) + .build()); + } + + @ParameterizedTest + @MethodSource("refusedNames") + public void ObjectName_RefusedNameFromConstructor_Refused(String name) { + assertThrows( + PipelineConfigurationException.class, + () -> new JavaScriptBuilderElement( + loggerFactory.getLogger("test"), + null, + "", + name, + false, + "", + "")); + } + + @ParameterizedTest + @MethodSource("refusedNames") + public void ObjectName_IsValidObjectName_RefusedName(String name) { + assertFalse(JavaScriptBuilderElement.isValidObjectName(name)); + } + + @ParameterizedTest + @MethodSource("allowedWindowNames") + public void ObjectName_WindowPropertyVarCanReplace_Allowed(String name) { + assertTrue(JavaScriptBuilderElement.isValidObjectName(name)); + JavaScriptBuilderElement element = + new JavaScriptBuilderElementBuilder(loggerFactory) + .setObjectName(name) + .build(); + + String script = render(element); + + assertTrue(script.contains("var " + name + " = new"), + "the script should declare '" + name + "'"); + assertEquals(0, warnings().size()); + } + + @ParameterizedTest + @ValueSource(strings = {"windows", "Window", "cachesX"}) + public void ObjectName_NearMissOfRefusedName_Allowed(String name) { + assertTrue(JavaScriptBuilderElement.isValidObjectName(name)); + } + /** * No name configured means the default name, whether the name was never * set or was set to null.