Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upfs: add validation for fd and path #35187
Open
Conversation
lib/internal/fs/streams.js
Outdated
| // If fd has been set, validate, otherwise validate path. | ||
| if (this.fd !== null) { | ||
| validateInteger(this.fd, 'fd', 0); | ||
| } else if (this.fd === null) { |
lib/internal/fs/streams.js
Outdated
| // If fd has been set, validate, otherwise validate path. | ||
| if (this.fd !== null) { | ||
| validateInteger(this.fd, 'fd', 0); | ||
| } else if (this.fd === null) { |
|
|
||
| assert.throws( | ||
| () => { | ||
| fs.createReadStream(null, { fd: fd }); |
mscdex
Sep 14, 2020
Contributor
Suggested change
| fs.createReadStream(null, { fd: fd }); | |
| fs.createReadStream(null, { fd }); |
|
|
||
| assert.throws( | ||
| () => { | ||
| fs.createWriteStream(null, { fd: fd }); |
mscdex
Sep 14, 2020
Contributor
Suggested change
| fs.createWriteStream(null, { fd: fd }); | |
| fs.createWriteStream(null, { fd }); |
dylanelliott27
Sep 14, 2020
Author
Hi mscdex, thanks so much for your suggestions to improve my code. I have modified the original commit message as well as implemented the changes. I had some trouble with ESLint which unfortunately caused me to change/commit an unnecessary file. I have removed this file, and I am hoping things are in a good state right now, if not, apologies as I am still getting used to things. Let me know and I can make any changes asap.
|
|
dylanelliott27
force-pushed the
dylanelliott27:stream-arg-validation
branch
from
a9b57e3
to
b5eb951
adds type validation to options fd & opts in createWriteStream and createReadStream. Fixes: #35178
dylanelliott27
force-pushed the
dylanelliott27:stream-arg-validation
branch
from
b5eb951
to
3bdb8c8
|
@nodejs/fs This could use another review. |
| @@ -131,6 +132,13 @@ function ReadStream(path, options) { | |||
| this.pos = this.start; | |||
| } | |||
|
|
|||
| // If fd has been set, validate, otherwise validate path. | |||
| if (this.fd !== null) { | |||
| @@ -296,6 +304,13 @@ function WriteStream(path, options) { | |||
| this.closed = false; | |||
| this[kIsPerformingIO] = false; | |||
|
|
|||
| // If fd has been set, validate, otherwise validate path. | |||
| if (this.fd !== null) { | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Adding validation to options fd & path in createWriteStream and createReadStream.
Fixes: #35178
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes