Skip to content

Commit ac60740

Browse files
committed
buffer: fix range checking for slowToString
If `start` is not a valid number in the range, then the default value zero will be used. Same way, if `end` is not a valid number in the accepted range, then, by default, the length of the buffer is assumed. Ref: #2668 PR-URL: #2919
1 parent f32a606 commit ac60740

File tree

3 files changed

+99
-12
lines changed

3 files changed

+99
-12
lines changed

lib/buffer.js

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -322,13 +322,31 @@ Object.defineProperty(Buffer.prototype, 'offset', {
322322
function slowToString(encoding, start, end) {
323323
var loweredCase = false;
324324

325-
start = start >>> 0;
326-
end = end === undefined || end === Infinity ? this.length : end >>> 0;
325+
if (start >= this.length || end <= 0)
326+
return '';
327+
328+
/*
329+
* if unsigned 32 bit value (>>> 0) of `start` is not the same as `start`,
330+
* use `0` as the default value. Similarly, if the unsigned 32 bit value of
331+
* `end` is not the same as `end`, then use the length of the buffer as the
332+
* end value.
333+
*
334+
* Note that, the comparison is done with abstract equality operator (!=) to
335+
* allow stringified numbers ('1') and Number objects (new Number(5)).
336+
*/
337+
if (start < 0 || start >>> 0 != parseInt(+start))
338+
start = 0;
339+
340+
if (end > this.length || end >>> 0 != parseInt(+end))
341+
end = this.length;
342+
343+
start >>>= 0;
344+
end >>>= 0;
345+
346+
if (end <= start)
347+
return '';
327348

328349
if (!encoding) encoding = 'utf8';
329-
if (start < 0) start = 0;
330-
if (end > this.length) end = this.length;
331-
if (end <= start) return '';
332350

333351
while (true) {
334352
switch (encoding) {

src/node_internals.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ inline MUST_USE_RESULT bool ParseArrayIndex(v8::Local<v8::Value> arg,
172172
return true;
173173
}
174174

175-
int32_t tmp_i = arg->Int32Value();
175+
int32_t tmp_i = arg->Uint32Value();
176176

177177
if (tmp_i < 0)
178178
return false;

test/parallel/test-buffer.js

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -248,14 +248,83 @@ b.copy(new Buffer(1), 1, 1, 1);
248248
// try to copy 0 bytes from past the end of the source buffer
249249
b.copy(new Buffer(1), 0, 2048, 2048);
250250

251-
// try to toString() a 0-length slice of a buffer, both within and without the
252-
// valid buffer range
253-
assert.equal(new Buffer('abc').toString('ascii', 0, 0), '');
254-
assert.equal(new Buffer('abc').toString('ascii', -100, -100), '');
255-
assert.equal(new Buffer('abc').toString('ascii', 100, 100), '');
251+
const rangeBuffer = new Buffer('abc');
252+
253+
// if start >= buffer's length, empty string will be returned
254+
assert.equal(rangeBuffer.toString('ascii', 3), '');
255+
assert.equal(rangeBuffer.toString('ascii', +Infinity), '');
256+
assert.equal(rangeBuffer.toString('ascii', 3.14, 3), '');
257+
assert.equal(rangeBuffer.toString('ascii', 'Infinity', 3), '');
258+
259+
// if end <= 0, empty string will be returned
260+
assert.equal(rangeBuffer.toString('ascii', 1, 0), '');
261+
assert.equal(rangeBuffer.toString('ascii', 1, -1.2), '');
262+
assert.equal(rangeBuffer.toString('ascii', 1, -100), '');
263+
assert.equal(rangeBuffer.toString('ascii', 1, -Infinity), '');
264+
265+
// if start < 0, start will be taken as zero
266+
assert.equal(rangeBuffer.toString('ascii', -1, 3), 'abc');
267+
assert.equal(rangeBuffer.toString('ascii', -1.99, 3), 'abc');
268+
assert.equal(rangeBuffer.toString('ascii', -Infinity, 3), 'abc');
269+
assert.equal(rangeBuffer.toString('ascii', '-1', 3), 'abc');
270+
assert.equal(rangeBuffer.toString('ascii', '-1.99', 3), 'abc');
271+
assert.equal(rangeBuffer.toString('ascii', '-Infinity', 3), 'abc');
272+
273+
// if start is an invalid integer, start will be taken as zero
274+
assert.equal(rangeBuffer.toString('ascii', 'node.js', 3), 'abc');
275+
assert.equal(rangeBuffer.toString('ascii', {}, 3), 'abc');
276+
assert.equal(rangeBuffer.toString('ascii', [], 3), 'abc');
277+
assert.equal(rangeBuffer.toString('ascii', NaN, 3), 'abc');
278+
assert.equal(rangeBuffer.toString('ascii', null, 3), 'abc');
279+
assert.equal(rangeBuffer.toString('ascii', undefined, 3), 'abc');
280+
assert.equal(rangeBuffer.toString('ascii', false, 3), 'abc');
281+
assert.equal(rangeBuffer.toString('ascii', '', 3), 'abc');
282+
283+
// but, if start is an integer when coerced, then it will be coerced and used.
284+
assert.equal(rangeBuffer.toString('ascii', '-1', 3), 'abc');
285+
assert.equal(rangeBuffer.toString('ascii', '1', 3), 'bc');
286+
assert.equal(rangeBuffer.toString('ascii', '-Infinity', 3), 'abc');
287+
assert.equal(rangeBuffer.toString('ascii', '3', 3), '');
288+
assert.equal(rangeBuffer.toString('ascii', Number(3), 3), '');
289+
assert.equal(rangeBuffer.toString('ascii', '3.14', 3), '');
290+
assert.equal(rangeBuffer.toString('ascii', '1.99', 3), 'bc');
291+
assert.equal(rangeBuffer.toString('ascii', '-1.99', 3), 'abc');
292+
assert.equal(rangeBuffer.toString('ascii', 1.99, 3), 'bc');
293+
assert.equal(rangeBuffer.toString('ascii', true, 3), 'bc');
294+
295+
// if end > buffer's length, end will be taken as buffer's length
296+
assert.equal(rangeBuffer.toString('ascii', 0, 5), 'abc');
297+
assert.equal(rangeBuffer.toString('ascii', 0, 6.99), 'abc');
298+
assert.equal(rangeBuffer.toString('ascii', 0, Infinity), 'abc');
299+
assert.equal(rangeBuffer.toString('ascii', 0, '5'), 'abc');
300+
assert.equal(rangeBuffer.toString('ascii', 0, '6.99'), 'abc');
301+
assert.equal(rangeBuffer.toString('ascii', 0, 'Infinity'), 'abc');
302+
303+
// if end is an invalid integer, end will be taken as buffer's length
304+
assert.equal(rangeBuffer.toString('ascii', 0, 'node.js'), 'abc');
305+
assert.equal(rangeBuffer.toString('ascii', 0, {}), 'abc');
306+
assert.equal(rangeBuffer.toString('ascii', 0, NaN), 'abc');
307+
assert.equal(rangeBuffer.toString('ascii', 0, undefined), 'abc');
308+
assert.equal(rangeBuffer.toString('ascii', 0), 'abc');
309+
assert.equal(rangeBuffer.toString('ascii', 0, null), '');
310+
assert.equal(rangeBuffer.toString('ascii', 0, []), '');
311+
assert.equal(rangeBuffer.toString('ascii', 0, false), '');
312+
assert.equal(rangeBuffer.toString('ascii', 0, ''), '');
313+
314+
// but, if end is an integer when coerced, then it will be coerced and used.
315+
assert.equal(rangeBuffer.toString('ascii', 0, '-1'), '');
316+
assert.equal(rangeBuffer.toString('ascii', 0, '1'), 'a');
317+
assert.equal(rangeBuffer.toString('ascii', 0, '-Infinity'), '');
318+
assert.equal(rangeBuffer.toString('ascii', 0, '3'), 'abc');
319+
assert.equal(rangeBuffer.toString('ascii', 0, Number(3)), 'abc');
320+
assert.equal(rangeBuffer.toString('ascii', 0, '3.14'), 'abc');
321+
assert.equal(rangeBuffer.toString('ascii', 0, '1.99'), 'a');
322+
assert.equal(rangeBuffer.toString('ascii', 0, '-1.99'), '');
323+
assert.equal(rangeBuffer.toString('ascii', 0, 1.99), 'a');
324+
assert.equal(rangeBuffer.toString('ascii', 0, true), 'a');
256325

257326
// try toString() with a object as a encoding
258-
assert.equal(new Buffer('abc').toString({toString: function() {
327+
assert.equal(rangeBuffer.toString({toString: function() {
259328
return 'ascii';
260329
}}), 'abc');
261330

0 commit comments

Comments
 (0)