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.

Sign up for GitHub

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

Jump to bottom

buffer: don't abort on prototype getters #3302

Closed
wants to merge 1 commit into from
Closed

buffer: don't abort on prototype getters #3302

wants to merge 1 commit into from

Conversation

trevnorris
Copy link
Contributor

Accessing prototype properties directly on a typed array will throw. So
do an extra check in Buffer's own getters to verify it is being called
on an instance.

Fixes: #3297

R=@Fishrock123 ?

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

@trevnorris
buffer: don't abort on prototype getters
63aaab1 
Accessing prototype properties directly on a typed array will throw. So
do an extra check in Buffer's own getters to verify it is being called
on an instance.

Fixes: nodejs#3297
@trevnorris trevnorris added the buffer Issues and PRs related to the buffer subsystem. label Oct 9, 2015
@bnoordhuis
Copy link
Member

LGTM

1 similar comment
@Fishrock123
Copy link
Contributor

LGTM

trevnorris added a commit that referenced this pull request Oct 9, 2015
@trevnorris
buffer: don't abort on prototype getters
e97dae5 
Accessing prototype properties directly on a typed array will throw. So
do an extra check in Buffer's own getters to verify it is being called
on an instance.

Fixes: #3297
PR-URL: #3302
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@trevnorris
Copy link
Contributor Author

Thanks. Landed on e97dae5

@trevnorris trevnorris closed this Oct 9, 2015
@thefourtheye
Copy link
Contributor

Isn't this a major change? Behaviour is not backward compatible, right? Earlier we used to throw and we don't now.

@trevnorris
Copy link
Contributor Author

@thefourtheye This was a bug from the change to using typed arrays.

@trevnorris
Copy link
Contributor Author

If you take notice of the change, we didn't remove an exception. It was throwing because of bad access to a typed array property.

@jasnell
Copy link
Member

jasnell commented Oct 10, 2015

@trevnorris ... should this land in v4.x? ... nm... looking at it, there's no reason why not :-)

trevnorris added a commit that referenced this pull request Oct 10, 2015
@trevnorris @jasnell
buffer: don't abort on prototype getters
5479562 
Accessing prototype properties directly on a typed array will throw. So
do an extra check in Buffer's own getters to verify it is being called
on an instance.

Fixes: #3297
PR-URL: #3302
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@jasnell
Copy link
Member

jasnell commented Oct 10, 2015

landed in v4.x in 5479562

@jasnell jasnell mentioned this pull request Oct 10, 2015
Closed
@thefourtheye
Copy link
Contributor

If you take notice of the change, we didn't remove an exception. It was throwing because of bad access to a typed array property.

Ah, fine then. Thanks for clarifying :-)

@trevnorris trevnorris deleted the invalid-buf-prop-access branch March 28, 2016 22:25
@snyk-bot snyk-bot mentioned this pull request Apr 24, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@trevnorris @bnoordhuis @Fishrock123 @thefourtheye @jasnell @MylesBorins