RDKB-66901 : harden session handling and request parsing

[pavankumar464]

Reason for change: Webui Wi-Fi page 500 Internal Error
Test Procedure: Test Webui actions
Risks: Low
Priority: P1

[pavankumar464]

• revalidate active sessions and prevent stale proxy persistence
• handle failed session creation and startup safely
• expire DUKSID on logout and preserve secure-cookie behavior
• parse standalone DUKSID cookie fields only
• retain equals signs in GET, POST, and file metadata values
• add prefix-runtime regression tests and synchronize parser fixtures

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings affect session runtime behavior, cookie parsing, stale-session cleanup, and test correctness.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Hardens JST session handling and request parsing to address WebUI Wi-Fi page failures.

Changes:

• Adds session lifecycle and secure-cookie handling.
• Preserves = characters in request values.
• Expands regression tests and regenerates parser fixtures.

File summaries

File	Description
tests/parser/jst_prefix.js	Updated parser prefix fixture.
tests/parser/jst_parser_template_block_string.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_template_block_content.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_skip_whitespace.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_single_quotes.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_line_feeds.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_unknown.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_runtime.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_once.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_not_if_in_line_comment.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_not_if_in_content.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_not_if_in_block_comment.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_nested.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_malformed_1.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_code_before.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_include_code_after.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_comment_tag.jst.parsed	Regenerated parser output.
tests/parser/jst_parser_backslash.jst.parsed	Regenerated parser output.
tests/parser_test.cpp	Adds session and parsing regression tests.
jsts/jst_prefix.js	Updates runtime session and request handling.

Review details

Suppressed comments (7)

jsts/jst_prefix.js:120

• Resetting _val_input on the first call makes the malformed-request guard one-shot. A template that calls session_start() again after the first false result can then create a session from the same invalid request; keep the failure state for the rest of the request.

  if($_val_input == 1)
  {
    $_val_input = 0;
    return false;

jsts/jst_prefix.js:241

• Calling print here writes unexpected post data directly to stdout before _jst_finish emits HTTP headers, producing an invalid CGI response for malformed input. Buffer the diagnostic with echo or route it through the normal error-response path instead.

    {
      print("unexpected post data");
      $_val_input = 1;

jsts/jst_prefix.js:115

• When an already-started session becomes stale, the native start() returns false but source/jst_session.c leaves session_identifier set when utime() fails. This branch clears only the JavaScript variables, so session_status() remains true and the stale native ID can be reused. Clear the native session state as well, preferably in the native failure path before returning false.

    if(ccsp_session.start())
      return true;

    $_jst_session = null;
    $_SESSION = {};
    return false;

tests/parser_test.cpp:661

• The native session_start implementation does not inspect HTTPS/REQUEST_SCHEME or otherwise bind a session ID to a request scheme, so a valid file-backed cookie is accepted when the request changes scheme. This expectation cannot pass until scheme binding is implemented in the native session layer.

  EXPECT_FALSE(duk_get_boolean(ctx, -1));

tests/parser_test.cpp:747

• After the session file is removed, native session_start() returns false from utime() but leaves session_identifier non-NULL; getStatus() then still returns true. The final assertion in this test fails and the prefix can report an inactive session as active. Clear the native identifier/status on stale-file failure.

  duk_get_prop_string(ctx, -1, "getStatus");
  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
  EXPECT_FALSE(duk_get_boolean(ctx, -1));

tests/parser_test.cpp:887

• session_create() in source/jst_session.c fills all 32 token characters from getrandom() and never forces the first token character to encode the request scheme. charAt(8) === '1' is therefore nondeterministic and usually false, even if secure detection is added. Make scheme encoding part of native ID generation or assert only the cookie attribute.

  EXPECT_TRUE(evaluateJavaScriptBoolean(ctx, "session_id().charAt(8) === '1'"));

tests/parser_test.cpp:234

• Returning false for every evaluation error makes EXPECT_FALSE(evaluateJavaScriptBoolean(...)) pass when the JavaScript throws, masking the missing isSecure() API and other runtime failures. Record the evaluation error with ADD_FAILURE() (or return a separate success status) before returning the boolean result.

  if (duk_peval_string(ctx, source) != DUK_EXEC_SUCCESS)
  {
    duk_pop(ctx);
    return false;
  }

• Files reviewed: 20/20 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.