FIX: Write only a safe session id and a positive sequence into the client script

[jwrosewell]

What is wrong

The JavaScript builder writes the session id into the script inside double
quotes and the sequence as a number, both exactly as the evidence gives
them.

• A session id holding a double quote or a backslash breaks the script, so
  the page gets nothing. The session id can come from the page request as
  query.session-id evidence, and the Sequence Element keeps a session id
  that is already there rather than replacing it.
• A sequence that is not a positive number is written as it is, so 0 or a
  negative number reaches the script. The Sequence Element adds one to
  whatever number it finds, so a request carrying -1 reaches the builder
  as 0, and the largest integer wraps round to the smallest.
• Where there is no Sequence Element, the script should still parse, with a
  sequence of 1 and an empty session id.

What changed

JavaScriptBuilderElement:

• a session id is written only when it matches ^[A-Za-z0-9-]{1,64}$, and
  anything else, including a value of the wrong type, is written as an
  empty string;
• a sequence is written only when it is a positive 32 bit integer, and
  anything else is written as 1;
• a sequence given as text is now read when it is made of digits, which is
  what the .NET builder does. Before this change, text was ignored and the
  sequence was always 1.

The rule is in the specification, in the draft
51Degrees/specifications#31 (steps 3 and 5 of
pipeline-specification/pipeline-elements/javascript-builder.md).

Tests

SessionIdAndSequenceTests, which needs no browser. Each rendered script
is parsed with the GraalVM JavaScript engine that this module already
depends on, without running it. The tests are:

• SessionId_Valid_Rendered, SessionId_LongestValid_Rendered
• SessionId_Invalid_RenderedEmpty for a"b, a\b, </script>, a 65
  character id, a non-ASCII id and an empty one
• SessionId_InvalidWithSequenceElement_RenderedEmpty, the same values
  with the Sequence Element in the pipeline
• SessionId_FromSequenceElement_Rendered
• NoSequenceElement_NoEvidence_DefaultsRendered
• Sequence_InvalidText_RenderedOne for abc, -1, 0, 99999999999,
  an empty value and 2147483648
• Sequence_InvalidNumber_RenderedOne for -1, 0, the smallest integer
  and a value too large for an integer
• Sequence_Valid_Rendered for 5 and the largest integer, as a number
  and as text
• Sequence_InvalidWithSequenceElement_RenderedOne for -1 and the
  largest integer, which the Sequence Element turns into 0 and the
  smallest integer

Before the source change, 17 of the 32 cases fail, including:

SessionId_Invalid_RenderedEmpty:152->render:276->assertParses:287 the script does not parse: SyntaxError: Unnamed:9:26 Missing close quote
SessionId_Invalid_RenderedEmpty:153 expected: <> but was: <a\b>
SessionId_Invalid_RenderedEmpty:153 expected: <> but was: <</script>>
Sequence_InvalidNumber_RenderedOne:206 expected: <1> but was: <-1>
Sequence_InvalidNumber_RenderedOne:206 expected: <1> but was: <-2147483648>
Sequence_InvalidWithSequenceElement_RenderedOne:238 expected: <1> but was: <0>
Sequence_Valid_Rendered:222 expected: <5> but was: <1>

With the change, 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: 32, Failures: 0, Errors: 0, Skipped: 0 - in SessionIdAndSequenceTests
Tests run: 50, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Things found on the way that are not changed here

• The JSON builder refuses to run unless the evidence holds a sequence that
  is already an Integer, so in practice this module cannot run without the
  Sequence Element. The tests that need a pipeline without that element use
  a small stand in for the JSON builder so the values reach the builder as
  given.
• The Sequence Element casts the sequence evidence straight to int, so a
  request carrying sequence=abc makes the pipeline throw rather than
  starting again at 1. Nothing reaches the script in that case.
• The builder's evidence key filter does not name query.session-id or
  query.sequence, so in a web application neither is collected from the
  request unless another element asks for them. The usage sharing filter
  does ask for them.

Overlap with other work

• The object name draft FIX: Use only a valid JavaScript identifier as the client side object name #144 changes the same file, in the method that
  builds the script rather than in the session id and sequence, so the two
  do not touch the same lines.

The same change in the other ports

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

The merges between this draft, #143 and #144 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
35187920129
ran on 34ec39b, the head of this branch, and concluded success with all 15
jobs green. In the Ubuntu_Java_21 job the Maven totals are 657 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/35187920129

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 657 tests run with 0 failures, 0 errors and 0 skipped. Java 8 runs 642, 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.