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

buffer: stricter `from()` input validation #26825

Closed
wants to merge 2 commits into from

Conversation

@BridgeAR
Copy link
Member

commented Mar 20, 2019

So far we did not throw an error for all types of invalid input.
Functions do not return a buffer anymore and number and symbol
validation is also improved.

@cjihrig you opened your PR (#26743) to address the function part first. Please feel free to reopen your PR and I'll either rebase on top of your or just close mine if you would like to pull in my changes.

@ChALkeR you also said that you have a PR almost ready, I am happy to close this one.

@nodejs/tsc is this semver-major because of the function validation (if a function is passed in, it seems like a bug, so I am not sure)? The message part is AFAIK not semver-major.

Fixes: #26741

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
buffer: stricter `from()` input validation
So far we did not throw an error for all types of invalid input.
Functions do not return a buffer anymore and `number` and `symbol`
validation is also improved.
@BridgeAR

This comment has been minimized.

Copy link
Member Author

commented Mar 24, 2019

@mcollina
Copy link
Member

left a comment

LGTM

fixup! buffer: stricter `from()` input validation
Recent code changes require this change.
@BridgeAR

This comment has been minimized.

Copy link
Member Author

commented Mar 24, 2019

@Fishrock123

This comment has been minimized.

Copy link
Member

commented Mar 25, 2019

Yeah I guess the function thing is technically semver-major but it is quite certainly an error and trap.

@BridgeAR BridgeAR requested review from jasnell, ChALkeR, cjihrig and bnoordhuis Mar 27, 2019

@BridgeAR

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

lib/buffer.js Show resolved Hide resolved

@BridgeAR BridgeAR requested a review from Trott Mar 28, 2019

@ZYSzys
ZYSzys approved these changes Mar 29, 2019
@targos
targos approved these changes Mar 29, 2019
@BridgeAR

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

@mcollina

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

I’m fine as patch.

@targos

This comment has been minimized.

Copy link
Member

commented Mar 30, 2019

Landed in 75eaf25

@targos targos closed this Mar 30, 2019

targos added a commit that referenced this pull request Mar 30, 2019
buffer: use stricter `from()` input validation
So far we did not throw an error for all types of invalid input.
Functions do not return a buffer anymore and `number` and `symbol`
validation is also improved.

PR-URL: #26825
Fixes: #26741
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BethGriggs added a commit that referenced this pull request Apr 5, 2019
buffer: use stricter `from()` input validation
So far we did not throw an error for all types of invalid input.
Functions do not return a buffer anymore and `number` and `symbol`
validation is also improved.

PR-URL: #26825
Fixes: #26741
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BethGriggs added a commit that referenced this pull request Apr 9, 2019
buffer: use stricter `from()` input validation
So far we did not throw an error for all types of invalid input.
Functions do not return a buffer anymore and `number` and `symbol`
validation is also improved.

PR-URL: #26825
Fixes: #26741
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.