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: don't abort on prototype getters #3302

Closed
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/

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
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 <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@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
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 <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@jasnell
Copy link
Member

jasnell commented Oct 10, 2015

landed in v4.x in 5479562

@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 :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants