Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v10.x backport] fs: improve truncate length validation #21171

Closed
wants to merge 3 commits into from
Closed

[v10.x backport] fs: improve truncate length validation #21171

wants to merge 3 commits into from

Conversation

codebytere
Copy link
Member

Original PR: #20851

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

This allows validation of integers that are not int32 or uint32.

PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit adds validation to the length parameter of
fs.truncate(). Prior to this commit, passing a non-number would
trigger a CHECK() in the binding layer.

PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
The length used by ftruncate() is 64 bits in the binding layer.
This commit removes the 32 bit restriction in the JS layer.

PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. v10.x labels Jun 6, 2018
@targos
Copy link
Member

targos commented Jun 6, 2018

@codebytere Thanks for the PR :) These commits don't apply cleanly to v10.x because of other previous PRs that need a backport. I opened #21172 for that and it includes the commits that this PR attempts to backport.

@addaleax
Copy link
Member

addaleax commented Jun 6, 2018

@targos These commits to seem to apply cleanly to me? Would there be any issue with landing these first and then rebasing your larger backport PR against it?

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

@targos
Copy link
Member

targos commented Jun 6, 2018

What I meant is that these commits from master don't apply cleanly (that's why they have the backport-requested label).

Would there be any issue with landing these first and then rebasing your larger backport PR against it?

No. There won't be many conflicts to solve.

@addaleax
Copy link
Member

addaleax commented Jun 6, 2018

Oh, that makes more sense… I guess in that case it might be easier to do it your way

targos pushed a commit that referenced this pull request Jun 6, 2018
This allows validation of integers that are not int32 or uint32.

PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Backport-PR-URL: #21171
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
targos pushed a commit that referenced this pull request Jun 6, 2018
This commit adds validation to the length parameter of
fs.truncate(). Prior to this commit, passing a non-number would
trigger a CHECK() in the binding layer.

Backport-PR-URL: #21171
PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Backport-PR-URL: #21171
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
targos pushed a commit that referenced this pull request Jun 6, 2018
The length used by ftruncate() is 64 bits in the binding layer.
This commit removes the 32 bit restriction in the JS layer.

Backport-PR-URL: #21171
PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Backport-PR-URL: #21171
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
@targos
Copy link
Member

targos commented Jun 6, 2018

Landed in 16ef09f...0577312 🎉

@targos targos closed this Jun 6, 2018
targos pushed a commit that referenced this pull request Jun 7, 2018
The length used by ftruncate() is 64 bits in the binding layer.
This commit removes the 32 bit restriction in the JS layer.

Backport-PR-URL: #21171
PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Backport-PR-URL: #21171
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
targos pushed a commit that referenced this pull request Jun 13, 2018
This allows validation of integers that are not int32 or uint32.

PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Backport-PR-URL: #21171
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
targos pushed a commit that referenced this pull request Jun 13, 2018
This commit adds validation to the length parameter of
fs.truncate(). Prior to this commit, passing a non-number would
trigger a CHECK() in the binding layer.

Backport-PR-URL: #21171
PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Backport-PR-URL: #21171
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
targos pushed a commit that referenced this pull request Jun 13, 2018
The length used by ftruncate() is 64 bits in the binding layer.
This commit removes the 32 bit restriction in the JS layer.

Backport-PR-URL: #21171
PR-URL: #20851
Fixes: #20844
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Backport-PR-URL: #21171
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
@targos targos added this to Backports done in v10.x Sep 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
No open projects
v10.x
  
Closed PRs
Development

Successfully merging this pull request may close these issues.

None yet

5 participants