src: handle null backing store in ArrayBufferViewContents::Read

[mertcanaltin]

added fallback stack_storage for null scenario

Fixes: #62342

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.69%. Comparing base (0998c37) to head (7d46389).
⚠️ Report is 53 commits behind head on main.

Files with missing lines	Patch %	Lines
src/util-inl.h	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62343      +/-   ##
==========================================
- Coverage   91.60%   89.69%   -1.91%     
==========================================
  Files         337      676     +339     
  Lines      140741   206695   +65954     
  Branches    21797    39579   +17782     
==========================================
+ Hits       128923   185398   +56475     
- Misses      11594    13438    +1844     
- Partials      224     7859    +7635     

Files with missing lines	Coverage Δ
src/util-inl.h	82.35% <66.66%> (ø)

... and 459 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[panva]

With ArrayBufferViewContents being used broadly throughout node's codebase I wonder if we can be sure this is entirely without side effects. I cannot assess that.

That being said, it does fix the particular node:crypto issue reported.

[mertcanaltin]

With ArrayBufferViewContents being used broadly throughout node's codebase I wonder if we can be sure this is entirely without side effects. I cannot assess that.

That being said, it does fix the particular node:crypto issue reported.

I went through all 19 files that use ArrayBufferViewContents, every usage passes data() alongside length(), and none dereferences the pointer when length is 0. Currently data_ can already be null in this scenario, this fix just ensures it's a valid pointer instead, so it's strictly safer than the current behavior.

[panva]

I went through all 19 files that use ArrayBufferViewContents, every usage passes data() alongside length(), and none dereferences the pointer when length is 0. Currently data_ can already be null in this scenario, this fix just ensures it's a valid pointer instead, so it's strictly safer than the current behavior.

Very well, can you update the commit message to src: handle null backing store in ArrayBufferViewContents::Read?

[mertcanaltin]

I went through all 19 files that use ArrayBufferViewContents, every usage passes data() alongside length(), and none dereferences the pointer when length is 0. Currently data_ can already be null in this scenario, this fix just ensures it's a valid pointer instead, so it's strictly safer than the current behavior.

Very well, can you update the commit message to src: handle null backing store in ArrayBufferViewContents::Read?

sure, edited thanks

[mcollina]

lgtm

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in bdf75a6