Skip to content

THRIFT-6409: Preserve nested empty-list values in Node.js TJSONProtocol - #4054

Merged
Jens-G merged 1 commit into
apache:masterfrom
dajiaohuang:THRIFT-6409
Oct 27, 2026
Merged

Jens-G merged 1 commit into
apache:masterfrom
dajiaohuang:THRIFT-6409

Conversation

@dajiaohuang

Copy link
Copy Markdown
Contributor

Fixes THRIFT-6409.

readListEnd previously popped only one stack entry. When a nested list was empty, the child value and empty list frame remained above the parent; subsequent sibling values could be read from the wrong stack frame. Restore the stack to the parent collection before advancing it.

Adds synthetic Node.js TJSONProtocol regressions for nested lists and map values containing empty nested lists, and registers the test in lib/nodejs/test/testAll.sh.

Verification:

  • node lib/nodejs/test/json_nested_empty_lists.test.js — 4 assertions passed.
  • ESLint on the new test — passed.
  • Prettier check on the new test — passed.
  • git diff --check — passed.
  • The full Node.js testAll.sh suite was not run: this Windows host has no Thrift compiler or make available to generate and run the broader protocol tests.
  • Full-file ESLint/Prettier checks on the existing json_protocol.js report pre-existing legacy ES5/style and line-ending issues across unchanged code; no repository-wide formatting was applied.

No CI status was queried.

Generated-by: Pi 1.4.0 (openai-codex/gpt-6-luna)

Client: js

Generated-by: Pi 1.4.0 (openai-codex/gpt-6-luna)
@dajiaohuang
dajiaohuang requested a review from emmenlau as a code owner October 26, 2026 22:05
@mergeable mergeable Bot added the nodejs label Oct 26, 2026
@Jens-G

Jens-G commented Oct 27, 2026

Copy link
Copy Markdown
Member

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

@Jens-G
Jens-G merged commit 8079734 into apache:master Oct 27, 2026
109 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants