Null-terminate PooledBuffer and Utf8StringArray UTF-8 strings by r-Larch · Pull Request #510 · microsoft/node-api-dotnet

René Larch (r-Larch) · 2026-10-21T20:14:25Z

Fixes #508.

(Rebased onto main after #482 was merged. The duplicate Utf8StringArray commit and its test are dropped.)

PooledBuffer.FromStringUtf8 / FromSpanUtf8: write the NUL terminator explicitly. On .NET the buffer comes from ArrayPool<byte>.Shared, which is not zeroed, so napi_throw_*_error, node_embedding_runtime_config_add_module and node_embedding_last_error_message_set read stale bytes past the string.
An empty string is encoded as just the terminator instead of PooledBuffer.Empty (null pointer), so JSError.ThrowError("") now actually throws. null still maps to a null pointer (used for the optional code argument).
EmbeddingRuntimeConfigAddModule: dispose the module-name buffer. Node.js copies the name into a std::string (EmbeddedRuntime::AddModule), so this is safe.

Tests

All fail on main and pass with the fix:

PooledBufferTests: dirties the shared pool, then checks that both PooledBuffer helpers produce NUL-terminated UTF-8 for "", ASCII and non-ASCII strings, and that null still pins to a null pointer.
NodejsEmbeddingTests.ThrowErrorFromCallback("x" / ""): throws from a .NET callback with a dirty pool and checks the JS e.message. Before: x followed by 15 garbage bytes, and "" did not throw at all.
New helper TestUtils.FillArrayPoolWithGarbage() (no-op on .NET Framework).

Local run (Windows x64, .NET SDK 10.0.402): net10.0 157/159, net9.0 126/127, net8.0 126/127. The only failure, napi-dotnet/overloads, fails the same way on main. I could not run net472 locally (the test-case builds fail with MSB4062 on this machine, also on main).

🤖 Generated with Claude Code

azure-pipelines · 2026-10-21T20:14:35Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

🟢 Approval recommended

The focused fixes have regression coverage, and static review identified no blocking issues.

No open issues 🎉

What changed in this PR

Fixes pooled UTF-8 string termination for native Node.js calls, addressing #508 and #481.

Changes:

Explicitly writes NUL terminators and distinguishes empty strings from null.
Disposes the module-name buffer after registration.
Adds regression tests using dirty pooled buffers.

File	Description
test/Utf8BufferTests.cs	Tests UTF-8 termination and null handling.
test/TestUtils.cs	Adds a helper to dirty pooled buffers.
test/NodejsEmbeddingTests.cs	Tests callback error messages, including empty strings.
src/NodeApi/Runtime/Utf8StringArray.cs	Explicitly terminates each encoded string.
src/NodeApi/Runtime/PooledBuffer.cs	Terminates UTF-8 buffers and preserves null semantics.
src/NodeApi/Runtime/NodejsRuntime.Embedding.cs	Disposes the module-name buffer.

_{Review effort: Balanced}

René Larch (r-Larch) · 2026-10-21T20:18:52Z

René Larch (René Larch (@r-Larch)) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree

Vladimir Morozov (vmoroz) · 2026-10-23T23:08:52Z

René Larch (@r-Larch) , the PR validation fails with the formatting issues.
Please run the following command on your machine to fix them automatically:

dotnet format --no-restore --severity info --verbosity detailed

Vladimir Morozov (vmoroz) · 2026-10-28T20:25:39Z

René Larch (@r-Larch) , I had merged the #482 . You may need to rebase your PR.

PooledBuffer.FromStringUtf8 and FromSpanUtf8 reserved a byte for the terminator but never wrote it. On .NET the array comes from ArrayPool<byte>.Shared, which is not zeroed, so native functions that take a null-terminated const char* without a length read stale bytes past the string (or past the array): napi_throw_error / napi_throw_type_error / napi_throw_range_error / node_api_throw_syntax_error (code and msg), node_embedding_runtime_config_add_module and node_embedding_last_error_message_set. For example JSError.ThrowError("x") produced a JS error message of "x" followed by garbage, and a linked module could be registered under a corrupted name. - Write the terminator explicitly in both helpers. - Encode an empty string as just the terminator instead of returning PooledBuffer.Empty, which pinned to a null pointer (ThrowError("") passed msg = NULL, so napi_throw_error failed and nothing was thrown). A null string still maps to a null pointer. - Return the module-name buffer to the pool in EmbeddingRuntimeConfigAddModule (Node.js copies the name). Fixes microsoft#508. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

René Larch (r-Larch) · 2026-10-29T02:54:41Z

Vladimir Morozov (@vmoroz) thanks for the review! I've rebased onto main after #482. My Utf8StringArray commit and its duplicate test are dropped, so this PR is now a single commit that only touches PooledBuffer. I also fixed the formatting issue: CA1822 on two test methods (I added the same #pragma that NodejsEmbeddingTests.cs uses). dotnet format --verify-no-changes is now clean. #511 and #512 are restacked on top of it.

Copilot

🟢 Approval recommended

No active review issues remain for the UTF-8 buffer termination and temporary-buffer disposal changes.

No open issues 🎉

_{Review effort: Balanced}

Vladimir Morozov (vmoroz) · 2026-10-30T23:43:24Z

René Larch (@r-Larch) , thank you for the contribution! The PR is passed all tests and is merged.

Copilot AI balanced review requested due to automatic review settings October 21, 2026 20:14

This was referenced Oct 21, 2026

Throw the named error type from JSError.Throw*Error(string) #511

Open

Initialize linked modules within the current JS value scope #512

Open

Copilot started reviewing on behalf of René Larch (r-Larch) October 21, 2026 20:15

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Vladimir Morozov (vmoroz) approved these changes Oct 23, 2026

View reviewed changes

Vladimir Morozov (vmoroz) self-requested a review October 27, 2026 21:14

René Larch (r-Larch) force-pushed the fix/pooledbuffer-nul-terminator branch from 19f7529 to 21ae41c Compare October 29, 2026 02:53

Copilot AI balanced review requested due to automatic review settings October 29, 2026 02:53

Copilot started reviewing on behalf of René Larch (r-Larch) October 29, 2026 02:54

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Vladimir Morozov (vmoroz) approved these changes Oct 29, 2026

View reviewed changes

Vladimir Morozov (vmoroz) merged commit 0483aa0 into microsoft:main Oct 30, 2026
29 of 30 checks passed

Navigation Menu

Repository navigation

Null-terminate PooledBuffer and Utf8StringArray UTF-8 strings - #510

Null-terminate PooledBuffer and Utf8StringArray UTF-8 strings#510
Vladimir Morozov (vmoroz) merged 1 commit into
microsoft:mainfrom
r-Larch:fix/pooledbuffer-nul-terminator

René Larch (r-Larch) commented Oct 21, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

René Larch (r-Larch) commented Oct 21, 2026

Uh oh!

Vladimir Morozov (vmoroz) commented Oct 23, 2026

Uh oh!

Vladimir Morozov (vmoroz) commented Oct 28, 2026

Uh oh!

René Larch (r-Larch) commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Vladimir Morozov (vmoroz) commented Oct 30, 2026 •

edited

Loading

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

René Larch (r-Larch) commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests

Uh oh!

azure-pipelines Bot commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

René Larch (r-Larch) commented Oct 21, 2026

Uh oh!

Vladimir Morozov (vmoroz) commented Oct 23, 2026

Uh oh!

Vladimir Morozov (vmoroz) commented Oct 28, 2026

Uh oh!

René Larch (r-Larch) commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Vladimir Morozov (vmoroz) commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

René Larch (r-Larch) commented Oct 21, 2026 •

edited

Loading

Vladimir Morozov (vmoroz) commented Oct 30, 2026 •

edited

Loading