stream: disallow writing string chunk with 'buffer' encoding

[Renegade334]

In non-object mode, Writable.prototype.write() validates the encoding as either 'buffer' or a valid string encoding.

Under default conditions, passing the encoding 'buffer' in combination with a string chunk results indirectly in an ERR_UNKNOWN_ENCODING error, because those are passed to Buffer.from() which considers the 'buffer' encoding to be invalid.

However, when decodeStrings is disabled, this indirect validation does not occur, and the string chunk is sent to the write() implementation with the encoding 'buffer', which is obviously nonsense.

semver... something?

Refs: #33075

[nodejs-github-bot]

Review requested:

• @nodejs/streams

[Renegade334]

This was a test for the behaviour reported in #12152, where calling .write('string', 'buffer') on a StreamBase-backed stream would crash the process. I don't believe there's any way to hit this if this change lands, but not confidently enough to revert back to a CHECK (cf. #12753) without a second opinion – maybe Team Stream has an insight?

(cc @addaleax)

[Renegade334]

I'm fairly confident that this can't now be hit (

node/lib/internal/net.js

Lines 67 to 73 in 843dc5f

	if (enc !== 'buffer')
	chunk = Buffer.from(chunk, enc);
	this._handle.bytesWritten += chunk.length;
	const ctx = {};
	writeBuffer(fd, chunk, 0, chunk.length, null, undefined, ctx);

was the offending call) so have restored the CHECK.

[Renegade334]

These tests were using the wrong constructor option name for setting the _write implementation. It doesn't matter if the error tests fail as expected, but it does make the assertion message less clear if an error test doesn't fail, as it instead complains about a missing _write implementation rather than reporting the missing expected error.

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.06%. Comparing base (c24e552) to head (b420c0f).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
src/stream_base.cc	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #63062   +/-   ##
=======================================
  Coverage   90.05%   90.06%           
=======================================
  Files         714      714           
  Lines      225247   225248    +1     
  Branches    42578    42580    +2     
=======================================
+ Hits       202842   202860   +18     
+ Misses      14181    14180    -1     
+ Partials     8224     8208   -16     

Files with missing lines	Coverage Δ
lib/internal/streams/writable.js	96.38% <100.00%> (-0.25%)	⬇️
src/stream_base.cc	80.03% <0.00%> (-0.26%)	⬇️

... and 25 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.