FIX: Use only a valid JavaScript identifier as the client side object name

[jwrosewell]

Problem

An object name that is not a valid JavaScript identifier was written into the script as given, which broke the script or changed what it does.

The name of the client side object (fod by default) comes from two places.

1. The builder, through JavaScriptBuilderElementBuilder.setObjectName. This already refused a name that did not match the identifier pattern, but it accepted reserved words such as var.
2. The page request, through the query.fod-js-object-name evidence. JavaScriptBuilderElement.buildJavaScript used any non-empty value with no check at all, and the template writes the name into the var declaration, the session storage key, the window["<name>Evidence"] lookup and the warning shown when the script is loaded twice.

Change

• JavaScriptBuilderElement.isValidObjectName accepts a name only when it matches ^[A-Za-z_$][A-Za-z0-9_$]*$ and is not on the refused list. That list holds the reserved words of the language, plus two further groups.
  • Infinity, NaN and undefined are refused because a top level var cannot replace them, so the object would silently never be created.
  • fiftyoneDegreesManager is refused because it is the constructor the script defines and calls to create the object. The other languages use the same list.
• A name from the query.fod-js-object-name evidence that is not valid, including an empty value, is ignored. The configured name is used instead and a warning is logged. The warning does not repeat the requested value.
• setObjectName uses the same check, so a name on the refused list is now refused with PipelineConfigurationException as well.
• The element's public constructor applies the same check, so a name that does not come through the builder is refused too. A null name means fod, because no name was configured, and an empty name is refused.

The rendered output for a valid name is unchanged.

Tests

JavaScriptObjectNameTests needs no browser. It renders the script against a mocked flow data, parses it, and then runs it with the GraalVM JavaScript engine the module already depends on, against a minimal window, document and sessionStorage.

• ObjectName_FromBuilder_UsedThroughout and ObjectName_FromEvidence_UsedThroughout use myFod. They check the script declares var myFod and never var fod, that the storage key and the evidence lookup use myFod, that the script parses, and that once run window.myFod exists, complete, onChange and refresh are functions and myFod.device.ismobile is true.
• ObjectName_InvalidFromEvidence_DefaultUsed covers a;b//, 9bad, x"y, the empty string, class and fiftyoneDegreesManager. Each must give a working fod object, a script that parses, one warning, and none of the requested text in the code. The line that holds the script's own request parameters is left out of that last check, because it carries the page's query back to the endpoint as url encoded JSON strings, which is data and not code.
• ObjectName_InvalidFromEvidence_ConfiguredNameUsed checks the fallback is the configured name and not always fod.
• ObjectName_InvalidFromBuilder_Refused and ObjectName_InvalidFromConstructor_Refused check the configuration error, including for fiftyoneDegreesManager.

Verification

Run with mvn -o -pl pipeline.javascriptbuilder -am test -Dtest=JavaScriptObjectNameTests,JavaScriptBuilderElementBuilderTests,CookieTests -Dsurefire.failIfNoSpecifiedTests=false. JavaScriptBuilderTests was not run because it starts Chrome.

• With both commits on Java 21, 38 tests ran with 0 failures (4 in CookieTests, 14 in JavaScriptBuilderElementBuilderTests, 20 in JavaScriptObjectNameTests).
• With both commits on Java 8 (Temurin 1.8.0_504), the same 38 tests ran with 0 failures, which confirms the GraalVM engine runs on the Java 8 legs.
• With the two source files put back to main, the first version of JavaScriptObjectNameTests (17 tests) gave 11 failures. All four constructor cases, the reserved word builder case, all five invalid evidence cases and the configured fallback case failed. For the empty value, main already fell back to fod, so that case failed only on the new warning.
• With the source put back to the first commit, the three new fiftyoneDegreesManager cases were the only failures, 3 out of 20.

Outstanding

• The same check is needed in the other languages, so that every builder gives the same output.
• JavaScriptBuilderTests.JavaScriptBuilderElement_VerifyObjName still runs in a browser and still passes a valid name, so it is unaffected.

Alignment with the other languages, third commit

An empty configured name was treated as no name at all and quietly became fod. The agreed rule is that a name which is not configured at all, meaning absent or null, means fod, and that a configured name which is not valid, including an empty one, is refused when the element is built. The third commit brings this repository to that rule.

• The element's constructor refuses an empty name and takes a null name as fod.
• JavaScriptBuilderElementBuilder starts with fod rather than with an empty name, and setObjectName(null) leaves that default in place.
• ObjectName_InvalidFromConstructor_Refused now covers the empty name, and ObjectName_NotConfigured_DefaultUsed checks that a name that was never set, a name set to null and a null name given to the constructor all give a working fod object.

With the third commit's source changes removed and its tests in place, 2 of the 22 tests fail:

ObjectName_InvalidFromConstructor_Refused:214 Expected fiftyone.pipeline.core.exceptions.PipelineConfigurationException to be thrown, but nothing was thrown.
ObjectName_NotConfigured_DefaultUsed:238 PipelineConfiguration

With the third commit, the module's tests pass, leaving out JavaScriptBuilderTests because it starts a browser:

Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 - in CookieTests
Tests run: 14, Failures: 0, Errors: 0, Skipped: 0 - in JavaScriptBuilderElementBuilderTests
Tests run: 22, Failures: 0, Errors: 0, Skipped: 0 - in JavaScriptObjectNameTests
Tests run: 40, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

The session id and the sequence are handled in a separate draft, #145, which changes the same file in different methods.

The same change in the other ports

The Node port of this change is
pipeline-node#206.

The merges between this draft, #143 and #145 were run locally in both
directions and all six are clean. The three way merge was built and tested
as well, giving Tests run: 74, Failures: 0, Errors: 0, Skipped: 0 and
BUILD SUCCESS for pipeline.javascriptbuilder, leaving out the test that
starts a browser. So the three can land in any order without a merge.

CI on this branch, 17 September 2026

The "Pull Requests" workflow was dispatched on this branch with dryrun=true,
which builds the branch and merges nothing. Run
35187912996
ran on 1f60be0, the head of this branch, and concluded success with all 15
jobs green. In the Ubuntu_Java_21 job the Maven totals are 647 tests run, 0
failures, 0 errors and 6 skipped, with no error lines in the log.

This branch is three commits behind main and merges into it cleanly.
The nightly checks out the pull request, merges main into it and builds
that before it merges anything, so the combination is tested there.

[jwrosewell]

Proven in CI

Full build and test run on this branch: https://github.com/51Degrees/pipeline-java/actions/runs/35187912996

A draft pull request only runs the link lint, so the Pull Requests workflow was dispatched against this branch as a dry run to get real evidence. Nothing was merged, and the log shows why: with no pull request based on this branch the run builds the branch on its own as pull request 0, and the checkout and completion steps both print "Not running for a PR".

All 15 jobs finished green, being 11 build and test jobs (Java 8, 11, 17 and 21 on Ubuntu and Windows, and Java 11, 17 and 21 on macOS), the pull request lookup, the configure step, the performance comparison and the completion step.

Reading the job logs rather than the conclusions, each job reports 23 Maven module summaries, and on Java 11 and later they add up to 647 tests run with 0 failures, 0 errors and 0 skipped. Java 8 runs 632, the difference being the tests that need a later Java. The log contains no "BUILD FAILURE", no "FAILURES!", no "ERRORS!" and no summary line with a non-zero failure or error count.

The pull request has not been marked ready and no review has been requested.