Skip to content

fs : update validateOffsetLengthRead in utils.js#32896

Closed
daemon1024 wants to merge 3 commits intonodejs:masterfrom
daemon1024:fs-error
Closed

fs : update validateOffsetLengthRead in utils.js#32896
daemon1024 wants to merge 3 commits intonodejs:masterfrom
daemon1024:fs-error

Conversation

@daemon1024
Copy link
Contributor

@daemon1024 daemon1024 commented Apr 17, 2020 •
edited
Loading

Fixes : #32871

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the fs Issues and PRs related to the fs subsystem / file system. label Apr 17, 2020
@daemon1024
Copy link
Contributor Author

daemon1024 commented Apr 17, 2020 •
edited
Loading

node/test/parallel/test-fs-read-type.js

Line 47 in 5a4f24e

message: 'The value of "offset" is out of range. It must be >= 0 && <= 4. ' +

node/test/parallel/test-fs-read-type.js

Line 76 in 5a4f24e

'It must be >= 0 && <= 4. Received -1'

node/test/parallel/test-fs-read-type.js

Line 113 in 5a4f24e

'It must be >= 0 && <= 4. Received -1'

node/test/parallel/test-fs-read-type.js

Line 139 in 5a4f24e

'It must be >= 0 && <= 4. Received -1'

Updating these in test/parallel/test-fs-read-type.js accordingly

himself65
himself65 reviewed Apr 17, 2020
View reviewed changes
Copy link
Member

@himself65 himself65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test case for <= ${bufferLength - offset}

@daemon1024 daemon1024 requested a review from himself65 April 17, 2020 16:50
juanarbol
juanarbol approved these changes Apr 18, 2020
View reviewed changes
Copy link
Member

@juanarbol juanarbol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 22, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30911/

@puzpuzpuz
Copy link
Member

puzpuzpuz commented Apr 23, 2020 •
edited
Loading

Landed in 1cc1ca4

Thanks for this improvement!

@puzpuzpuz puzpuzpuz closed this Apr 23, 2020
puzpuzpuz pushed a commit that referenced this pull request Apr 23, 2020
@daemon1024 @puzpuzpuz
fs: update validateOffsetLengthRead in utils.js
1cc1ca4 
PR-URL: #32896
Fixes: #32871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 27, 2020
@daemon1024 @BethGriggs
fs: update validateOffsetLengthRead in utils.js
4c643c0 
PR-URL: #32896
Fixes: #32871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Apr 27, 2020
Merged
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
@daemon1024 @BridgeAR
fs: update validateOffsetLengthRead in utils.js
27837fe 
PR-URL: #32896
Fixes: #32871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Apr 28, 2020
Merged
targos pushed a commit that referenced this pull request Apr 30, 2020
@daemon1024 @targos
fs: update validateOffsetLengthRead in utils.js
fbab5db 
PR-URL: #32896
Fixes: #32871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
@targos targos mentioned this pull request May 2, 2020
Merged
targos pushed a commit that referenced this pull request May 13, 2020
@daemon1024 @targos
fs: update validateOffsetLengthRead in utils.js
2093f13 
PR-URL: #32896
Fixes: #32871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes

@himself65 himself65 himself65 approved these changes

@juanarbol juanarbol juanarbol approved these changes

+1 more reviewer

@puzpuzpuz puzpuzpuz puzpuzpuz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

fs Issues and PRs related to the fs subsystem / file system.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Confusing error message in fs.utils

6 participants

@daemon1024 @nodejs-github-bot @puzpuzpuz @addaleax @himself65 @juanarbol