Skip to content

Commit 1e80253

Browse files
cjihrigcjihrig
committed
buffer: throw when filling with empty buffers
Prior to this commit, Node would enter an infinite loop when attempting to fill a non-zero length buffer with a zero length buffer. This commit introduces a thrown exception in this scenario. PR-URL: #18129 Fixes: #18128 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
1 parent db9c556 commit 1e80253

3 files changed

Lines changed: 22 additions & 7 deletions

File tree

‎doc/api/buffer.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,10 @@ changes:
519519
pr-url: https://github.com/nodejs/node/pull/17427
520520
description: Specifying an invalid string for `fill` triggers a thrown
521521
exception.
522+
- version: REPLACEME
523+
pr-url: https://github.com/nodejs/node/pull/18129
524+
description: Attempting to fill a non-zero length buffer with a zero length
525+
buffer triggers a thrown exception.
522526
-->
523527

524528
* `size` {integer} The desired length of the new `Buffer`.
@@ -1231,6 +1235,10 @@ changes:
12311235
pr-url: https://github.com/nodejs/node/pull/17427
12321236
description: Specifying an invalid string for `value` triggers a thrown
12331237
exception.
1238+
- version: REPLACEME
1239+
pr-url: https://github.com/nodejs/node/pull/18129
1240+
description: Attempting to fill a non-zero length buffer with a zero length
1241+
buffer triggers a thrown exception.
12341242
-->
12351243

12361244
* `value` {string|Buffer|integer} The value to fill `buf` with.

‎src/node_buffer.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -642,20 +642,20 @@ void Fill(const FunctionCallbackInfo<Value>& args) {
642642
str_obj,
643643
enc,
644644
nullptr);
645-
// This check is also needed in case Write() returns that no bytes could
646-
// be written. If no bytes could be written, then return -1 because the
647-
// string is invalid. This will trigger a throw in JavaScript. Silently
648-
// failing should be avoided because it can lead to buffers with unexpected
649-
// contents.
650-
if (str_length == 0)
651-
return args.GetReturnValue().Set(-1);
652645
}
653646

654647
start_fill:
655648

656649
if (str_length >= fill_length)
657650
return;
658651

652+
// If str_length is zero, then either an empty buffer was provided, or Write()
653+
// indicated that no bytes could be written. If no bytes could be written,
654+
// then return -1 because the fill value is invalid. This will trigger a throw
655+
// in JavaScript. Silently failing should be avoided because it can lead to
656+
// buffers with unexpected contents.
657+
if (str_length == 0)
658+
return args.GetReturnValue().Set(-1);
659659

660660
size_t in_there = str_length;
661661
char* ptr = ts_obj_data + start + str_length;

‎test/parallel/test-buffer-alloc.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,3 +1024,10 @@ common.expectsError(() => {
10241024
code: 'ERR_INVALID_ARG_VALUE',
10251025
type: TypeError
10261026
});
1027+
1028+
common.expectsError(() => {
1029+
Buffer.alloc(1, Buffer.alloc(0));
1030+
}, {
1031+
code: 'ERR_INVALID_ARG_VALUE',
1032+
type: TypeError
1033+
});

0 commit comments

Comments
 (0)