RDKB-65597 harden session parsing/validation and read_file bounds with regressio…

[pavankumar464]

…n tests

[Copilot]

Pull request overview

This PR aims to harden session cookie parsing/validation and improve read_file() robustness, with new regression tests exercising those edge cases.

Changes:

• Add regression tests for read_file() directory input handling and session cookie parsing behaviors.
• Tighten session ID parsing to avoid strtok() mutation and validate prefix/length/charset more strictly.
• Add error handling around fseek()/ftell() in read_file().

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
tests/parser_test.cpp	Adds new regression tests for read_file() and session cookie parsing behaviors.
tests/CMakeLists.txt	Links jst_session.c into the test binary so session tests can execute.
source/jst_session.c	Hardens session ID extraction/validation from HTTP_COOKIE and avoids modifying cookie memory.
source/jst_internal.c	Adds error checks for file positioning calls in read_file().

Comments suppressed due to low confidence (1)

source/jst_internal.c:171

• read_file() returns early on failure without initializing *bufout/*lenout. Callers (and the new regression test) rely on these being left as NULL/0, but that currently depends on the caller pre-initializing them. Initialize outputs at the start of the function to make the API safer and deterministic on error paths.

int read_file(const char *filename, char** bufout, size_t* lenout)
{
  FILE* pf;
  size_t size;
  long tell_result;
  size_t rc;
  char* buf;

  CosaPhpExtLog( "read_file %s\n", filename );

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[github-actions]

📋 PR Format Reminder

• Title: RDKB-65597 harden session parsing/validation and read_file bounds with regressio… — expected TICKET-123 : description
  (Multiple tickets OK: RDKCOM-5492 RDKBDEV-3336 : ... | Include US ticket + subtask for user-stories)
• Description missing:
  • Reason for change
  • Test Procedure
  • Risks (Low / Medium / High)
  • Priority (P0 / P1 / P2)

Expected:

TICKET-123 : brief description

Reason for change: why
Test Procedure: how to verify
Risks: Low / Medium / High
Priority: P0 / P1 / P2

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

source/jst_internal.c:184

• read_file() logs a NULL-safe filename but then still passes filename directly to fopen() and to %s in error logs. If a caller passes NULL, this is undefined behavior / may crash, and it also violates the new intention of handling NULL safely. Add an early if (!filename) return 0; after initializing *bufout/*lenout (and before fopen).

  CosaPhpExtLog("read_file %s\n", filename ? filename : "(null)");

  errno = 0;
  pf = fopen(filename, "r");
  if(!pf)

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

tests/parser_test.cpp:248

• This test uses a hard-coded session id and writes a fixed file path under /tmp. If the test suite is run in parallel (multiple processes) or if another run leaves the same file behind, this can cause collisions and flaky behavior. Generate a per-process unique session id while keeping the required 40-char format/prefix.

  duk_get_prop_string(ctx, -1, "create");
  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
  EXPECT_TRUE(duk_get_boolean(ctx, -1));

tests/parser_test.cpp:296

• This test writes a fixed-named session file under /tmp using a hard-coded session id. To avoid collisions/flakes when tests run concurrently (or when stale files exist), generate a per-process unique id while keeping the invalid prefix and total length.

  duk_get_global_string(ctx, "ccsp_post");
  duk_get_prop_string(ctx, -1, "getFiles");
  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.