From cca557cdd97bf45a2620b959d9142b17774837f8 Mon Sep 17 00:00:00 2001 From: Deokjin Kim Date: Mon, 8 May 2023 08:58:41 +0900 Subject: [PATCH] buffer: combine checking range of sourceStart in `buf.copy` Merging 2 checking range of sourceStart into 1. Plus, add test case to increase coverage if sourceStart is greater than length of source. PR-URL: https://github.com/nodejs/node/pull/47758 Reviewed-By: Luigi Pinca --- lib/buffer.js | 10 ++-------- test/parallel/test-buffer-alloc.js | 2 +- test/parallel/test-buffer-copy.js | 11 +++++++++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 3c20253d580121..704b0369a53ffd 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -231,8 +231,8 @@ function _copy(source, target, targetStart, sourceStart, sourceEnd) { sourceStart = 0; } else { sourceStart = toInteger(sourceStart, 0); - if (sourceStart < 0) - throw new ERR_OUT_OF_RANGE('sourceStart', '>= 0', sourceStart); + if (sourceStart < 0 || sourceStart > source.length) + throw new ERR_OUT_OF_RANGE('sourceStart', `>= 0 && <= ${source.length}`, sourceStart); } if (sourceEnd === undefined) { @@ -246,12 +246,6 @@ function _copy(source, target, targetStart, sourceStart, sourceEnd) { if (targetStart >= target.length || sourceStart >= sourceEnd) return 0; - if (sourceStart > source.length) { - throw new ERR_OUT_OF_RANGE('sourceStart', - `<= ${source.length}`, - sourceStart); - } - return _copyActual(source, target, targetStart, sourceStart, sourceEnd); } diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index d2085d11802aee..c6b728027057ec 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -124,7 +124,7 @@ b.copy(Buffer.alloc(0), 1, 1, 1); b.copy(Buffer.alloc(1), 1, 1, 1); // Try to copy 0 bytes from past the end of the source buffer -b.copy(Buffer.alloc(1), 0, 2048, 2048); +b.copy(Buffer.alloc(1), 0, 1024, 1024); // Testing for smart defaults and ability to pass string values as offset { diff --git a/test/parallel/test-buffer-copy.js b/test/parallel/test-buffer-copy.js index f7ccfff9d0cbbe..fef20578cfd396 100644 --- a/test/parallel/test-buffer-copy.js +++ b/test/parallel/test-buffer-copy.js @@ -155,8 +155,15 @@ assert.throws( { code: 'ERR_OUT_OF_RANGE', name: 'RangeError', - message: 'The value of "sourceStart" is out of range. ' + - 'It must be >= 0. Received -1' + } +); + +// Copy throws if sourceStart is greater than length of source +assert.throws( + () => Buffer.allocUnsafe(5).copy(Buffer.allocUnsafe(5), 0, 100), + { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', } );