Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 44fe78b

Browse filesBrowse files
kodemillMylesBorins
authored andcommitted
stream: inline needMoreData function
Inline the needMoreData function since it has only one call place. Update the related comment. Add a test for the edge case where HWM=0 and state.length=0. Add a test for ReadableStream.read(n) method's edge case where n, HWM and state.length are all zero. This proves that there is no easy way to simplify the check at https://github.com/nodejs/node/blob/master/lib/_stream_readable.js#L440 Fixes: #19893 Refs: #19896 PR-URL: #21009 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Lance Ball <lball@redhat.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 9ada68b commit 44fe78b
Copy full SHA for 44fe78b

File tree

Expand file treeCollapse file tree

2 files changed

+55
-37
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+55
-37
lines changed
Open diff view settings
Collapse file

‎lib/_stream_readable.js‎

Copy file name to clipboardExpand all lines: lib/_stream_readable.js
+5-14Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,11 @@ function readableAddChunk(stream, chunk, encoding, addToFront, skipChunkCheck) {
270270
}
271271
}
272272

273-
return needMoreData(state);
273+
// We can push more data if we are below the highWaterMark.
274+
// Also, if we have no data yet, we can stand some more bytes.
275+
// This is to work around cases where hwm=0, such as the repl.
276+
return !state.ended &&
277+
(state.length < state.highWaterMark || state.length === 0);
274278
}
275279

276280
function addChunk(stream, state, chunk, addToFront) {
@@ -304,19 +308,6 @@ function chunkInvalid(state, chunk) {
304308
}
305309

306310

307-
// We can push more data if we are below the highWaterMark.
308-
// Also, if we have no data yet, we can stand some
309-
// more bytes. This is to work around cases where hwm=0,
310-
// such as the repl. Also, if the push() triggered a
311-
// readable event, and the user called read(largeNumber) such that
312-
// needReadable was set, then we ought to push more, so that another
313-
// 'readable' event will be triggered.
314-
function needMoreData(state) {
315-
return !state.ended &&
316-
(state.length < state.highWaterMark ||
317-
state.length === 0);
318-
}
319-
320311
Readable.prototype.isPaused = function() {
321312
return this._readableState.flowing === false;
322313
};
Collapse file
+50-23Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,58 @@
11
'use strict';
22
const common = require('../common');
33

4-
// This test ensures that the stream implementation correctly handles values
5-
// for highWaterMark which exceed the range of signed 32 bit integers and
6-
// rejects invalid values.
7-
84
const assert = require('assert');
95
const stream = require('stream');
106

11-
// This number exceeds the range of 32 bit integer arithmetic but should still
12-
// be handled correctly.
13-
const ovfl = Number.MAX_SAFE_INTEGER;
14-
15-
const readable = stream.Readable({ highWaterMark: ovfl });
16-
assert.strictEqual(readable._readableState.highWaterMark, ovfl);
17-
18-
const writable = stream.Writable({ highWaterMark: ovfl });
19-
assert.strictEqual(writable._writableState.highWaterMark, ovfl);
20-
21-
for (const invalidHwm of [true, false, '5', {}, -5, NaN]) {
22-
for (const type of [stream.Readable, stream.Writable]) {
23-
common.expectsError(() => {
24-
type({ highWaterMark: invalidHwm });
25-
}, {
26-
type: TypeError,
27-
code: 'ERR_INVALID_OPT_VALUE',
28-
message: `The value "${invalidHwm}" is invalid for option "highWaterMark"`
29-
});
7+
{
8+
// This test ensures that the stream implementation correctly handles values
9+
// for highWaterMark which exceed the range of signed 32 bit integers and
10+
// rejects invalid values.
11+
12+
// This number exceeds the range of 32 bit integer arithmetic but should still
13+
// be handled correctly.
14+
const ovfl = Number.MAX_SAFE_INTEGER;
15+
16+
const readable = stream.Readable({ highWaterMark: ovfl });
17+
assert.strictEqual(readable._readableState.highWaterMark, ovfl);
18+
19+
const writable = stream.Writable({ highWaterMark: ovfl });
20+
assert.strictEqual(writable._writableState.highWaterMark, ovfl);
21+
22+
for (const invalidHwm of [true, false, '5', {}, -5, NaN]) {
23+
for (const type of [stream.Readable, stream.Writable]) {
24+
common.expectsError(() => {
25+
type({ highWaterMark: invalidHwm });
26+
}, {
27+
type: TypeError,
28+
code: 'ERR_INVALID_OPT_VALUE',
29+
message:
30+
`The value "${invalidHwm}" is invalid for option "highWaterMark"`
31+
});
32+
}
33+
}
34+
}
35+
36+
{
37+
// This test ensures that the push method's implementation
38+
// correctly handles the edge case where the highWaterMark and
39+
// the state.length are both zero
40+
41+
const readable = stream.Readable({ highWaterMark: 0 });
42+
43+
for (let i = 0; i < 3; i++) {
44+
const needMoreData = readable.push();
45+
assert.strictEqual(needMoreData, true);
3046
}
3147
}
48+
49+
{
50+
// This test ensures that the read(n) method's implementation
51+
// correctly handles the edge case where the highWaterMark, state.length
52+
// and n are all zero
53+
54+
const readable = stream.Readable({ highWaterMark: 0 });
55+
56+
readable._read = common.mustCall();
57+
readable.read(0);
58+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.