fix: napi_string off by one error

[Max-Clark]

Description

While testing, we ran into an intermittent issue with the default JSON encoder, Unexpected token \u0000 in JSON at position 1023. JSON stringified payloads that were length 1024 were being saved with \u0000 as the last character verified by fdbcli.

The issue appears to be an off-by-one error In the toStringParams function. To leave space for the null termination, the napi_get_value_string_utf8 function writes up to bufsize - 1 and sets the byte after the last written to \0 shown here. If a string length 1024 enters the if (!buf_in_use && result->len <= sizeof(sp_buf)) block, only 1023 bytes will be written, and index 1023 will be written as \0. Any string length less than 1024 does not have this problem, as it always has space in the buffer, and any string length greater than 1024 goes to the other block.

The simplest and safest way appeared to be reducing the if block to < sizeof(sp_buf).

As far as I could tell, only the json encoder sends the value by napi_string with the built-in encoders, so that's the only test I added.

Type of change

• [x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

An addition to ks.ts adds coverage to this. npm tests passed with my machine's configuration.

Test Configuration: FoundationDB version 7.1.27 running in docker in host mode Ubuntu 20.04 Node 18.14.0 LTS

Checklist:

• [x] My code follows the style guidelines of this project
• [x] I have performed a self-review of my code
• [x] I have made corresponding changes to the documentation
• [x] My changes generate no new warnings
• [x] I have added tests that prove my fix is effective or that my feature works
• [x] New and existing unit tests pass locally with my changes

Thanks for your work!

[josephg]

Hey - thanks for a great investigation and write-up of this problem! I haven't touched this project in awhile - I've been working on some other stuff. Great sleuthing to figure out the problem.

I like the fix, but it looks like your PR has some random other changes mixed in too (adding an npm package.lock file, updating some dependencies, etc). I'll take a look...

[Max-Clark]

Thanks for checking in! I'm happy to revert the other changes, just let me know.

[josephg]

Oh - thanks for the offer but I've just cherry-picked the patches out of your PR.

[josephg]

Fixed and published as 1.1.4. Thanks for your PR! :D