buffer: make buflen in integer range

[kylo5aby]

make buflen within the integer range when buffer.toString() and writing a string to buffer(for example buffer.write(str))

Fixes: #51817

[kvakil]

Suggested change

	if (max_length > static_cast<size_t>(v8::String::kMaxLength))
	ThrowErrStringTooLong(env->isolate());
	if (max_length > static_cast<size_t>(v8::String::kMaxLength)) {
	ThrowErrStringTooLong(env->isolate());
	return;
	}

You're missing a return here; the Throw* functions don't actually return, you have to do it manually.

[kvakil]

This test probably won't work in test/parallel, since 2 GiB is a lot to allocate (maybe it'll work on Linux)? It might work to put it in test/sequential instead.

[kylo5aby]

thanks @kvakil

[joyeecheung]

For tests that require a lot of resource, we typically use test/pummel. You could also catch ERR_MEMORY_ALLOCATION_FAILED in the buffer creation and common.skip() in the test if the memory could not be allocated.

[kylo5aby]

thanks your suggestion @joyeecheung , I have update it, add a method in StringBytes to ensure that the capacity of buffer remains within the integer range when operating with string

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57289/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57531/

[kylo5aby]

this PR has been approved but hasn't been landed for a long time, could you please take a look? @legendecas

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57967/

[kylo5aby]

I find some test checks didn't pass, I believe that's because in the test file, the buffer with size > INT32_MAX on x32 platform which will throw range error, I have updated and skip the test file on x32 platform, could you take a look? @legendecas

[legendecas]

This needs a rebase.

[kylo5aby]

This needs a rebase.

Resolved. Thanks!

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/58481/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59447/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59540/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59560/

[jakecastelli]

This will need a rebase again 😞

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.34%. Comparing base (0b3ae01) to head (db80e6e).
Report is 654 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #51821      +/-   ##
==========================================
+ Coverage   87.32%   87.34%   +0.01%     
==========================================
  Files         649      650       +1     
  Lines      182570   182847     +277     
  Branches    35026    35071      +45     
==========================================
+ Hits       159432   159701     +269     
- Misses      16406    16411       +5     
- Partials     6732     6735       +3     

Files with missing lines	Coverage Δ
src/string_bytes.cc	66.76% <100.00%> (+1.25%)	⬆️

... and 61 files with indirect coverage changes

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63423/